new flashloan entrypoint #30
Conversation
|
this is a new pr with a whole new entry point that wraps the |
|
Note that this is just an MVP, if we choose that this is the right approach then I'll further tidy the code. |
There was a problem hiding this comment.
Looks good so far @heytdep!
We should add an integration test in test-suites/tests that actually invokes a flash loan contract. We could add a test moderc3156 contract in mocks that just returns some additional tokens to the invoker, to validate we can successfully process a flash loan.
Let me know if any help is required on test requirements, happy to assist.
Thanks for the contribution!
pool/src/pool/submit.rs
Outdated
| // similarly to usual borrows, we want the flash loan to be debited before | ||
| // checking the health factor. TODO: We should decide whether the execution ordering | ||
| // matters here (I'm pretty sure it doesn't but haven't looked much into it). |
There was a problem hiding this comment.
This would only be an issue if the function ever was re-entrant.
maybe make a comment that re-entrancy is unsafe here since the user state is not written before the flash loan contract was invoked, to warn any future developer.
pool/src/pool/submit.rs
Outdated
| if use_allowance { | ||
| handle_transfer_with_allowance(e, &actions, from, from); | ||
| } else { | ||
| handle_transfers(e, &actions, from, from); | ||
| } |
There was a problem hiding this comment.
we should only use allowances here.
When executing transfers with flash loans, the odds the transfer requests will be accurate across blocks is unlikely, making simulation difficult.
There was a problem hiding this comment.
good point, switched on the use allowance flag.
pool/src/contract.rs
Outdated
| /// Submit a set of requests to the pool where 'from' takes on the position, 'sender' sends any | ||
| /// required tokens to the pool and 'to' receives any tokens sent from the pool | ||
| /// | ||
| /// Returns the new positions for 'from' | ||
| /// | ||
| /// ### Arguments | ||
| /// * `from` - The address of the user whose positions are being modified | ||
| /// * `spender` - The address of the user who is sending tokens to the pool | ||
| /// * `to` - The address of the user who is receiving tokens from the pool | ||
| /// * `requests` - A vec of requests to be processed |
There was a problem hiding this comment.
we should update docs for flash_loan args
pool/src/pool/submit.rs
Outdated
| from: &Address, | ||
| flash_loan: FlashLoan, | ||
| requests: Vec<Request>, | ||
| use_allowance: bool, |
There was a problem hiding this comment.
we could drop this argument, given it is fixed as true
test-suites/tests/test_flashloan.rs
Outdated
| use test_suites::{create_fixture_with_data, moderc3156::create_flashloan_receiver, test_fixture::{TokenIndex, SCALAR_7}}; | ||
|
|
||
| #[test] | ||
| fn test_liquidations() { |
test-suites/tests/test_flashloan.rs
Outdated
| // reset frodo's health. | ||
| // TODO: there probably is a user in the fixture that doesn't require this due to low hf? haven't really looked into it. |
There was a problem hiding this comment.
you can just create a new user, see wasm_happy_path for example, we create user samwise to start from scratch.
frodo mainly exists to seed the test pool with capital.
test-suites/tests/test_flashloan.rs
Outdated
| // this request rebalances the flash loan borrow. | ||
| // we need to account for the overcollateralization too, so it cannot be the same | ||
| // exact amount borrowed. | ||
| let requests: Vec<Request> = vec![ | ||
| &fixture.env, | ||
| Request { | ||
| request_type: RequestType::SupplyCollateral as u32, | ||
| address: fixture.tokens[TokenIndex::XLM].address.clone(), | ||
| amount: 2_000 * SCALAR_7, | ||
| }, | ||
|
|
||
| ]; |
There was a problem hiding this comment.
we should includes a separate test that processes a repayment as well, to validate the issue found in execute_submit_with_flash_loan
| // perform operations here | ||
| // ... | ||
|
|
There was a problem hiding this comment.
should this test contract just return amount to caller?
That way, we can test more common cases of flash loans like:
- Flash loan 1000 USDC to contract that does arb are returns 1000 USDC
- Repay 1000 USDC
And that the tokens are being properly transferred to the flash loan contract by the pool
pool/src/pool/submit.rs
Outdated
| // note: check_health is omitted since we always will want to check the health | ||
| // if a flash loan is involved. | ||
| let (actions, mut new_from_state, _) = | ||
| build_actions_from_request(e, &mut pool, from, requests); | ||
|
|
||
| // similarly to usual borrows, we want the flash loan to be debited before | ||
| // checking the health factor. | ||
|
|
||
| // Warning: at this point user state is not written yet before invoking the | ||
| // flash loan contract, so was this function ever reentrant this code would become | ||
| // unsafe. | ||
| { | ||
| let mut reserve = pool.load_reserve(e, &flash_loan.asset, true); | ||
| let d_tokens_minted = reserve.to_d_token_up(flash_loan.amount); | ||
| new_from_state.add_liabilities(e, &mut reserve, d_tokens_minted); | ||
| reserve.require_utilization_below_max(e); | ||
|
|
||
| e.events().publish( | ||
| ( | ||
| Symbol::new(e, "flash_borrow"), | ||
| &flash_loan.asset, | ||
| &flash_loan.contract, | ||
| ), | ||
| (&flash_loan.amount, d_tokens_minted), | ||
| ); | ||
| } |
There was a problem hiding this comment.
The flash loan liabilities will need to be added before build_actions_from_request, right?
Reason being, it will be quite common for a repay request to be included of the flash loaned asset in the requests array.
repay has logic that reduces the repayment amount based on the actual liabilities on the block this is being processed on. So, the flash_loan liability needs to exist on the Positions object before repay is processed, that way the liability will accurately be reduced.
test-suites/src/moderc3156.rs
Outdated
| @@ -0,0 +1,17 @@ | |||
| use soroban_sdk::{testutils::Address as _, Address, Env}; | |||
|
|
|||
| mod emitter_contract { | |||
test-suites/src/moderc3156.rs
Outdated
| use soroban_sdk::{testutils::Address as _, Address, Env}; | ||
|
|
||
| mod emitter_contract { | ||
| soroban_sdk::contractimport!(file = "../target/wasm32-unknown-unknown/optimized/moderc3156_example.wasm"); |
There was a problem hiding this comment.
we need to add this to the Makefile so that the moderc3156_example.wasm is built during make build, such that things like the github test pipeline can access it.
continuation of #10