Skip to content

Conversation

@Johnnycus
Copy link
Contributor

@Johnnycus Johnnycus commented Oct 30, 2025

This PR contains the StartRebalance and EndRebalance instructions reimplementation with jiminy.

@Johnnycus Johnnycus marked this pull request as ready for review November 3, 2025 21:59
Copy link
Contributor

@billythedummy billythedummy left a comment

Choose a reason for hiding this comment

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

a lot of clutter

Copy link
Contributor

@billythedummy billythedummy left a comment

Choose a reason for hiding this comment

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

i think theres also some confusion between Pubkey and [u8; 32]

  • Pubkey is just a simple newtype of [u8; 32] and you can always convert from one to the other easily
  • we're only using Pubkey because the solana dev deps (mollusk-svm, solana-instruction etc) use it. Otherwise, just stick with [u8; 32]

Comment on lines +380 to +385
// Filter out executable accounts (programs and sysvars) which are not
// relevant for rent-exemption checks - only user accounts matter
let mut result_for_check = result.clone();
result_for_check
.resulting_accounts
.retain(|(_, acc)| !acc.executable);
Copy link
Contributor

Choose a reason for hiding this comment

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

which program account is not rent-exempt tho?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the filter i would get this #89 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

#89 (comment) seems like it affects the balanced check, not the rent exemption check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Token Program starts with 934M lamports from fixtures in accs_bef, but after Mollusk executes the tx it appears in resulting_accounts with 0 lamports (token program account is reset in tests here? is that expected?). so we need to filter it out here before we do rent-exempt check

Copy link
Contributor

Choose a reason for hiding this comment

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

no this isnt rly expected lol wtf, guess we can check some other simpler tests that involve the token program to see whats going on

Copy link
Contributor

@billythedummy billythedummy Nov 7, 2025

Choose a reason for hiding this comment

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

this doesnt happen in withdraw_protocol_fees_test_correct_basic. Im guessing theres somewhere where you've mutated bef or aft and youre not checking the actual input/output to svm.process_instruction_(chain). Or maybe mollusk is weird and only outputs the accounts that the last instruction involves in InstructionResult

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resulting_accounts: The resulting accounts after the last instruction.
https://docs.rs/mollusk-svm/latest/mollusk_svm/struct.Mollusk.html#method.process_instruction_chain

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah i thought so too but if you look at the impl for process_instruction (_chain just calls this in a loop): the resulting accounts is always whatever was input to the function, not just the ones that are involved in the instruction (theres no filter in sight): https://github.com/anza-xyz/mollusk/blob/8029da5304e78fcf1d4396515f1e43422ad550a9/harness/src/lib.rs#L776-L806

Copy link
Contributor

Choose a reason for hiding this comment

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

seems like this is happening on the second token transfer instruction

Copy link
Contributor

@billythedummy billythedummy left a comment

Choose a reason for hiding this comment

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

lets just do the <acc>_checked functions and then do other cleanups in subsequent PRs

Comment on lines +380 to +385
// Filter out executable accounts (programs and sysvars) which are not
// relevant for rent-exemption checks - only user accounts matter
let mut result_for_check = result.clone();
result_for_check
.resulting_accounts
.retain(|(_, acc)| !acc.executable);
Copy link
Contributor

Choose a reason for hiding this comment

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

no this isnt rly expected lol wtf, guess we can check some other simpler tests that involve the token program to see whats going on


const _ACC_DATA_ALIGN: usize = 8;

const _POOL_STATE_ALIGN_CHECK: () = assert!(core::mem::align_of::<PoolState>() <= _ACC_DATA_ALIGN);
Copy link
Contributor

Choose a reason for hiding this comment

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

whyd you delete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

happy little accident

@Johnnycus Johnnycus merged commit ff3754b into master Nov 7, 2025
5 checks passed
@Johnnycus Johnnycus deleted the lst-343-inf1-jiminy-reimpl-startrebalance-and-endrebalance branch November 7, 2025 11:24
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.

3 participants