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

stake-pool: Wait at least two epoch boundaries to set fee #3979

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

joncinque
Copy link
Contributor

Problem

If a malicious manager wants to hike a fee, they can still do so right at the end of an epoch and have it take into account immediately at the start of the next epoch, giving users no time to get their funds out if they want.

Solution

Force a wait of at least two epoch boundaries before applying a new fee. This solution needs to be backwards compatible with existing stake pools, which use Option<Fee>. In Borsh, this is encoded as:

if x.is_some() {
  repr(1 as u8)
  repr(x.unwrap() as ident)
} else {
  repr(0 as u8)
}

This introduces a new enum FutureEpoch, which encodes 0 as None (same as option), 1 as "wait one epoch" (same as option), and then 2 as "wait two epochs" (the new state).

The biggest issue with this approach is that it requires calling "update" in two different epochs before applying the fee, rather than having the fee apply at a particular future epoch. Meaning, you can do "set fee", wait two epochs, then call "update", and you'll only move forward one epoch. You must call "set fee", wait at least one epoch boundary, call "update", wait at least one more epoch boundary, and call "update" again. Thankfully, at worst, it just takes longer for the new fee to kick in, which is acceptable (to me).

Comment on lines +821 to +841
pub fn update_epoch(&mut self) {
match self {
Self::None => {}
Self::One(_) => {
// The value has waited its last epoch
*self = Self::None;
}
// The value still has to wait one more epoch after this
Self::Two(v) => {
*self = Self::One(v.clone());
}
}
}

/// Get the value if it's ready, which is only at `One` epoch remaining
pub fn get(&self) -> Option<&T> {
match self {
Self::None | Self::Two(_) => None,
Self::One(v) => Some(v),
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this is cute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I certainly had fun writing it 😄

@joncinque joncinque merged commit c4269cb into solana-labs:master Jan 27, 2023
@joncinque joncinque deleted the sp-two branch January 27, 2023 01:10
HaoranYi pushed a commit to HaoranYi/solana-program-library that referenced this pull request Jul 19, 2023
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.

None yet

2 participants