Skip to content

program: use Rent over rent_exempt_reserve#542

Merged
2501babe merged 1 commit intosolana-program:mainfrom
2501babe:20260311_rent
Mar 11, 2026
Merged

program: use Rent over rent_exempt_reserve#542
2501babe merged 1 commit intosolana-program:mainfrom
2501babe:20260311_rent

Conversation

@2501babe
Copy link
Member

@2501babe 2501babe commented Mar 11, 2026

downstream changes to support solana-program/stake#303

also fix a mistake in the cli using the wrong size_of()

note i had to bump quinn-proto in the lockfile for audit, it doesnt affect anything in here though

@2501babe 2501babe self-assigned this Mar 11, 2026
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great!

let user_excess_lamports = post_pool_lamports
.checked_sub(pool_stake_state.delegation.stake)
.and_then(|amount| amount.checked_sub(pool_stake_meta.rent_exempt_reserve))
.and_then(|amount| amount.checked_sub(pool_rent_exempt_reserve))
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting that this is assuming a stake account will be the same size post-merge, which makes sense to me

Copy link
Member Author

@2501babe 2501babe Mar 11, 2026

Choose a reason for hiding this comment

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

yep, it isnt articulated by the stake program but this would be in line with the general principle of current behavior. if you Split, the new account has the currently enforced size asserted and both accounts must meet minimum delegation. if you Merge, we dont enforce size and we dont enforce minimum delegation

the principle i read into this is that an old Merge destination is allowed to continue as it was before, since closing the source is good, and the destination would have kept on existing as it was if there was no merge. so i dont think we would ever change this if we made larger stake accounts

@2501babe 2501babe merged commit c2e7545 into solana-program:main Mar 11, 2026
22 checks passed
@2501babe 2501babe deleted the 20260311_rent branch March 11, 2026 14:49
@2501babe 2501babe requested a review from grod220 March 18, 2026 14:05
@2501babe
Copy link
Member Author

@grod220 would you mind giving program/src/processor.rs a second review if you get a chance? the rest of the changes are just tests and cli and not important. i may have to do a short-notice singlepool release/deploy, and this is the only nontrivial (though still pretty straightforward) commit in it

Copy link
Member

@grod220 grod220 left a comment

Choose a reason for hiding this comment

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

The work in program/src/processor.rs looks good to me 👍 . A minor take-or-leave comment for the CLI.

Comment on lines +82 to +86
assert_eq!(
account.data.len(),
StakeStateV2::size_of(),
"StakeStateV2 is no longer canonical, or StakeStateV2::size_of() is no longer 200."
);
Copy link
Member

Choose a reason for hiding this comment

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

If you brought the get_minimum_balance_for_rent_exemption() down, invoked it with account.data.len() and then memoized the result (keyed by size)---this could automatically adjust to different future stake account sizings. Just an idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

yea that was what i meant by "we may be fetching hundreds of accounts here so memoize the rents"

@2501babe
Copy link
Member Author

ty!

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