Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flashloan fee #273

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add flashloan fee #273

wants to merge 5 commits into from

Conversation

dyodx
Copy link

@dyodx dyodx commented Jan 6, 2025

Attempt to close #270. All feedback welcome. No worries if this is too far from the right way to go about it.

I'm also wondering if this could perhaps do with a test similar to this one

assert.equal(feeAccSolAfter - feeAccSolBefore, INIT_POOL_ORIGINATION_FEE);

If so, afaict there aren't ts tests for flashloans so would have to add a new file perhaps?

@dyodx dyodx requested review from jkbpvsc and losman0s as code owners January 6, 2025 14:06
@@ -23,11 +23,14 @@ pub struct FeeState {
/// Flat fee assessed when a new bank is initialized, in lamports.
/// * In SOL, in native decimals.
pub bank_init_flat_sol_fee: u32,
/// Flat fee assessed at flashloan end, in lamports.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct is live on mainnet now, so new fields need to go at the end (just before padding) to avoid breaking existing serialization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, let me know if it's preferred that I move the flashloan_flat_sol_feeto be the last arg in all the methods as well. I had put it next to the other "flat_sol" fee arbitrarily.

@@ -144,4 +156,29 @@ pub struct LendingAccountEndFlashloan<'info> {
pub marginfi_account: AccountLoader<'info, MarginfiAccount>,
#[account(address = marginfi_account.load()?.authority)]
pub signer: Signer<'info>,
// Note: there is just one FeeState per program, so no further check is required.
#[account(
seeds = [FEE_STATE_SEED.as_bytes()],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we don't need the seed check, this account is unique per-program deployment.

@jgur-psyops
Copy link
Contributor

Almost LGTM, just needs updates to the tests in tests/user_actions/flash_loan.rs (there are currently no flashloan TS tests but that would be a nice-to-have as well).

@jgur-psyops
Copy link
Contributor

The TS side tests also need to be updated for initGlobalFeeState and editGlobalFeeState in group-instructions.ts.

current CI pipeline failure is from roothooks.ts:164 not having the extra argument)

@dyodx
Copy link
Author

dyodx commented Jan 8, 2025

@jgur-psyops all that's left is the tests but been having a tough time getting the rust tests to work. The closest I've gotten is e.g. running ./scripts/single-test.sh marginfi flashloan_success_1op --verbose which errors with:

...
[2025-01-08T06:50:53.810207000Z DEBUG solana_runtime::message_processor::stable_log] Program TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb success
[2025-01-08T06:50:53.813467000Z DEBUG solana_runtime::message_processor::stable_log] Program 11111111111111111111111111111111 invoke [1]                               
[2025-01-08T06:50:53.813598000Z DEBUG solana_runtime::message_processor::stable_log] Program 11111111111111111111111111111111 success                                  
[2025-01-08T06:50:53.816228000Z DEBUG solana_runtime::message_processor::stable_log] Program MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA invoke [1]
[2025-01-08T06:50:53.816258000Z DEBUG solana_runtime::message_processor::stable_log] Program is not deployed
[2025-01-08T06:50:53.816266000Z DEBUG solana_runtime::message_processor::stable_log] Program MFv2hWf31Z9kbCa1snEPYctwafyhdvnV7FZnsebVacA failed: invalid account data f
or instruction                                                                                                                                                         
thread 'user_actions::flash_loan::flashloan_success_1op' panicked at /Users/superuzer/Documents/marginfi-v2/test-utils/src/marginfi_group.rs:128:64: 
called `Result::unwrap()` on an `Err` value: TransactionError(InstructionError(0, InvalidAccountData))
...

Best guess atm is I still haven't built something or set some flag correctly. I built and have .so for brick, liquidity_incentive_program, marginfi, mocks, test_transfer_hook.

When I run ./scripts/test-program.sh marginfi 27 tests pass (the instructions and state and I think 2 others) and all the others fail with the same error. Any pointers? I'll also give it another fresh try tomorrow.

EDIT: nvm got it. reinstalled everything following CI. its a new day.

@dyodx
Copy link
Author

dyodx commented Jan 9, 2025

Thanks for the feedback @jgur-psyops. Ready for another review if you'd like to get this in ASAP--gonna see if I can come back to the TS nice to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flashloan baby fee
2 participants