program: use Rent over rent_exempt_reserve#615
Conversation
| let reserve_stake = try_from_slice_unchecked::<stake::state::StakeStateV2>( | ||
| &reserve_stake_info.data.borrow(), | ||
| )?; | ||
| let reserve_meta = reserve_stake | ||
| .meta() | ||
| .ok_or(StakePoolError::WrongStakeStake)?; |
There was a problem hiding this comment.
i dont believe this parse is necessary to validate the reserve, since the function checks the address, and unlike some of the other areas there is no assertion the state must be StakeStateV2::Initialized, but im not fully confident about this
There was a problem hiding this comment.
This looks correct -- we only did the parse because we wanted to get meta.rent_exempt_reserve
| // check that reserve has enough | ||
| let minimum_reserve_lamports = minimum_reserve_lamports(&meta); | ||
| let minimum_reserve_lamports = minimum_reserve_lamports(split_from_rent); |
There was a problem hiding this comment.
this looks surprising at first glance, but this instruction has no reserve_stake_info, and this is inside a branch if *stake_split_from.key == stake_pool.reserve_stake
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great! Just a few additional parses that we can remove, but that can be done in a future PR if you prefer
| let reserve_stake = try_from_slice_unchecked::<stake::state::StakeStateV2>( | ||
| &reserve_stake_info.data.borrow(), | ||
| )?; | ||
| let reserve_meta = reserve_stake | ||
| .meta() | ||
| .ok_or(StakePoolError::WrongStakeStake)?; |
There was a problem hiding this comment.
This looks correct -- we only did the parse because we wanted to get meta.rent_exempt_reserve
program/src/processor.rs
Outdated
| let reserve_stake = try_from_slice_unchecked::<stake::state::StakeStateV2>( | ||
| &reserve_stake_info.data.borrow(), | ||
| )?; |
There was a problem hiding this comment.
We probably don't need this parse either anymore -- same with the other case, the account address is checked earlier
There was a problem hiding this comment.
ok i think youre right actually. i was wary of messing with the implicit assertion these are Initialized but process_initialize() ensures it is Initialized, it should not be possible to delegate to Stake, and all the rent checks surrounding the reserve are meant to ensure it can never change state to Uninitialized or closed. and in any event it couldnt be reopened anyway so the withdraw would just fail. ill remove them
There was a problem hiding this comment.
Yeah exactly, that's what I was thinking
program/src/processor.rs
Outdated
| let stake_state = try_from_slice_unchecked::<stake::state::StakeStateV2>( | ||
| &reserve_stake_info.data.borrow(), | ||
| )?; |
There was a problem hiding this comment.
We can probably remove this parse too, since it's only used to get the rent_exempt_reserve field
2 participants
downstream changes to support solana-program/stake#303