Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
ochaloup committed Dec 18, 2023
1 parent 3702488 commit e452e2f
Show file tree
Hide file tree
Showing 32 changed files with 729 additions and 490 deletions.
606 changes: 378 additions & 228 deletions packages/validator-bonds-sdk/generated/validator_bonds.ts

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion programs/validator-bonds/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "validator-bonds"
version = "0.1.0"
description = "Marinade validator bonds program to insure validator behavior"
description = "Marinade validator bonds program protecting validator behavior"
edition = "2021"
license = "Apache-2.0"
authors = ["Marinade.Finance"]
Expand Down
114 changes: 69 additions & 45 deletions programs/validator-bonds/src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,49 @@ use anchor_lang::require_keys_eq;
use anchor_lang::solana_program::stake::state::{Delegation, Meta, Stake};
use anchor_lang::solana_program::stake_history::{Epoch, StakeHistoryEntry};
use anchor_lang::solana_program::vote::program::id as vote_program_id;
use anchor_lang::solana_program::vote::state::VoteState;
use anchor_spl::stake::StakeAccount;
use std::ops::Deref;

/// Verification the account is a vote account + matching owner (withdrawer authority)
pub fn check_validator_vote_account_owner(
/// Verification the account is owned by vote program + matching withdrawer authority (owner)
pub fn check_validator_vote_account_withdrawer_authority(
validator_vote_account: &UncheckedAccount,
expected_owner: &Pubkey,
) -> Result<VoteState> {
) -> Result<()> {
require!(
validator_vote_account.owner == &vote_program_id(),
ErrorCode::InvalidVoteAccountProgramId
);
let validator_vote_data = &validator_vote_account.data.borrow()[..];
let vote_account = VoteState::deserialize(validator_vote_data).map_err(|err| {
msg!("Cannot deserialize vote account: {:?}", err);
error!(ErrorCode::FailedToDeserializeVoteAccount)
.with_values(("validator_vote_account", validator_vote_account.key()))
})?;
// let's find position of the authorized withdrawer within the vote state account data
// https://github.com/solana-labs/solana/pull/30515
// https://github.com/solana-labs/solana/blob/v1.17.10/sdk/program/src/vote/state/mod.rs#L290
let pos = 36;
if validator_vote_data.len() < pos + 32 {
msg!(
"Cannot get withdrawer authority from vote account {} data",
validator_vote_account.key
);
return Err(ErrorCode::FailedToDeserializeVoteAccount.into());
}
let withdrawer_slice: [u8; 32] =
validator_vote_data[pos..pos + 32]
.try_into()
.map_err(|err| {
msg!(
"Cannot get withdrawer authority from vote account {} data: {:?}",
validator_vote_account.key,
err
);
error!(ErrorCode::FailedToDeserializeVoteAccount)
.with_values(("validator_vote_account", validator_vote_account.key()))
})?;
let authorized_withdrawer = Pubkey::from(withdrawer_slice);
require_keys_eq!(
*expected_owner,
vote_account.authorized_withdrawer,
authorized_withdrawer,
ErrorCode::ValidatorVoteAccountOwnerMismatch
);
Ok(vote_account)
Ok(())
}

/// Bond account change is permitted to bond authority or validator vote account owner
Expand All @@ -42,7 +60,7 @@ pub fn check_bond_change_permitted(
if authority == &bond_account.authority.key() {
true
} else {
check_validator_vote_account_owner(validator_vote_account, authority)
check_validator_vote_account_withdrawer_authority(validator_vote_account, authority)
.map_or(false, |_| true)
}
}
Expand All @@ -69,7 +87,7 @@ pub fn check_stake_valid_delegation(
}
}

pub fn check_stake_is_initialized_with_authority(
pub fn check_stake_is_initialized_with_withdrawer_authority(
stake_account: &StakeAccount,
authority: &Pubkey,
stake_account_attribute_name: &str,
Expand Down Expand Up @@ -98,11 +116,12 @@ pub fn check_stake_is_initialized_with_authority(
pub fn check_stake_is_not_locked(
stake_account: &StakeAccount,
clock: &Clock,
custodian: Option<&Pubkey>,
stake_account_attribute_name: &str,
) -> Result<()> {
if let Some(stake_lockup) = stake_account.lockup() {
if stake_lockup.is_in_force(clock, custodian) {
// TODO: consider working with custodian, would need to be passed from the caller
// and on authorize withdrawer we would need to change the lockup to the new custodian
if stake_lockup.is_in_force(clock, None) {
return Err(error!(ErrorCode::StakeLockedUp)
.with_account_name(stake_account_attribute_name)
.with_values((
Expand Down Expand Up @@ -180,7 +199,10 @@ mod tests {
);
let wrong_owner_account = UncheckedAccount::try_from(&account);
assert_eq!(
check_validator_vote_account_owner(&wrong_owner_account, &vote_init.authorized_voter,),
check_validator_vote_account_withdrawer_authority(
&wrong_owner_account,
&vote_init.authorized_voter,
),
Err(ErrorCode::InvalidVoteAccountProgramId.into())
);

Expand All @@ -197,14 +219,23 @@ mod tests {
);
let unchecked_account = UncheckedAccount::try_from(&account);

check_validator_vote_account_owner(&unchecked_account, &vote_init.authorized_withdrawer)
.unwrap();
check_validator_vote_account_withdrawer_authority(
&unchecked_account,
&vote_init.authorized_withdrawer,
)
.unwrap();
assert_eq!(
check_validator_vote_account_owner(&unchecked_account, &vote_init.authorized_voter,),
check_validator_vote_account_withdrawer_authority(
&unchecked_account,
&vote_init.authorized_voter,
),
Err(ErrorCode::ValidatorVoteAccountOwnerMismatch.into())
);
assert_eq!(
check_validator_vote_account_owner(&unchecked_account, &Pubkey::default(),),
check_validator_vote_account_withdrawer_authority(
&unchecked_account,
&Pubkey::default(),
),
Err(ErrorCode::ValidatorVoteAccountOwnerMismatch.into())
);
}
Expand Down Expand Up @@ -304,7 +335,7 @@ mod tests {
pub fn stake_initialized_with_authority_check() {
let uninitialized_stake_account = get_stake_account(StakeState::Uninitialized);
assert_eq!(
check_stake_is_initialized_with_authority(
check_stake_is_initialized_with_withdrawer_authority(
&uninitialized_stake_account,
&Pubkey::default(),
""
Expand All @@ -313,7 +344,7 @@ mod tests {
);
let rewards_pool_stake_account = get_stake_account(StakeState::RewardsPool);
assert_eq!(
check_stake_is_initialized_with_authority(
check_stake_is_initialized_with_withdrawer_authority(
&rewards_pool_stake_account,
&Pubkey::default(),
""
Expand All @@ -323,7 +354,7 @@ mod tests {

let initialized_stake_account = get_stake_account(StakeState::Initialized(Meta::default()));
assert_eq!(
check_stake_is_initialized_with_authority(
check_stake_is_initialized_with_withdrawer_authority(
&initialized_stake_account,
&Pubkey::default(),
""
Expand All @@ -333,7 +364,7 @@ mod tests {
let default_delegated_stake_account =
get_stake_account(StakeState::Stake(Meta::default(), Stake::default()));
assert_eq!(
check_stake_is_initialized_with_authority(
check_stake_is_initialized_with_withdrawer_authority(
&default_delegated_stake_account,
&Pubkey::default(),
""
Expand All @@ -347,7 +378,11 @@ mod tests {
let delegated_stake_account =
get_delegated_stake_account(None, Some(withdrawer), Some(staker));
assert_eq!(
check_stake_is_initialized_with_authority(&delegated_stake_account, &withdrawer, ""),
check_stake_is_initialized_with_withdrawer_authority(
&delegated_stake_account,
&withdrawer,
""
),
Ok(Meta {
authorized: Authorized { withdrawer, staker },
..Meta::default()
Expand All @@ -359,7 +394,7 @@ mod tests {
let delegated_stake_account =
get_delegated_stake_account(None, Some(withdrawer), Some(staker));
assert_eq!(
check_stake_is_initialized_with_authority(
check_stake_is_initialized_with_withdrawer_authority(
&delegated_stake_account,
&wrong_withdrawer,
""
Expand All @@ -375,24 +410,24 @@ mod tests {
// no lock on default stake account
let unlocked_stake_account = get_stake_account(StakeState::Uninitialized);
assert_eq!(
check_stake_is_not_locked(&unlocked_stake_account, &clock, None, ""),
check_stake_is_not_locked(&unlocked_stake_account, &clock, ""),
Ok(())
);
let rewards_pool_stake_account = get_stake_account(StakeState::RewardsPool);
assert_eq!(
check_stake_is_not_locked(&rewards_pool_stake_account, &clock, None, ""),
check_stake_is_not_locked(&rewards_pool_stake_account, &clock, ""),
Ok(())
);

let initialized_stake_account = get_stake_account(StakeState::Initialized(Meta::default()));
assert_eq!(
check_stake_is_not_locked(&initialized_stake_account, &clock, None, ""),
check_stake_is_not_locked(&initialized_stake_account, &clock, ""),
Ok(())
);
let default_delegated_stake_account =
get_stake_account(StakeState::Stake(Meta::default(), Stake::default()));
assert_eq!(
check_stake_is_not_locked(&default_delegated_stake_account, &clock, None, ""),
check_stake_is_not_locked(&default_delegated_stake_account, &clock, ""),
Ok(())
);

Expand All @@ -412,26 +447,15 @@ mod tests {

assert!(clock.epoch > 0 && clock.unix_timestamp > 0);

// locked, wrong custodian
let wrong_custodian = Pubkey::new_unique();
// locked
assert_eq!(
check_stake_is_not_locked(&epoch_locked_stake_account, &clock, None, ""),
check_stake_is_not_locked(&epoch_locked_stake_account, &clock, ""),
Err(ErrorCode::StakeLockedUp.into())
);
assert_eq!(
check_stake_is_not_locked(
&epoch_locked_stake_account,
&clock,
Some(&wrong_custodian),
""
),
check_stake_is_not_locked(&epoch_locked_stake_account, &clock, ""),
Err(ErrorCode::StakeLockedUp.into())
);
// locked, correct custodian
assert_eq!(
check_stake_is_not_locked(&epoch_locked_stake_account, &clock, Some(&custodian), ""),
Ok(())
);

let unix_timestamp_lockup = Lockup {
epoch: 0,
Expand All @@ -446,7 +470,7 @@ mod tests {
Stake::default(),
));
assert_eq!(
check_stake_is_not_locked(&unix_locked_stake_account, &clock, None, ""),
check_stake_is_not_locked(&unix_locked_stake_account, &clock, ""),
Err(ErrorCode::StakeLockedUp.into())
);
}
Expand Down
2 changes: 1 addition & 1 deletion programs/validator-bonds/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use anchor_lang::prelude::*;

#[constant]
pub const PROGRAM_ID: &str = "vbondsKbsC4QSLQQnn6ngZvkqfywn6KgEeQbkGSpk1V";
pub const PROGRAM_ID: &str = "vBoNdEvzMrSai7is21XgVYik65mqtaKXuSdMBJ1xkW4";

// TODO: anchor-0.29: constants cannot be used in anchor #[Account] when seeds = true
// https://github.com/coral-xyz/anchor/issues/2697
Expand Down
2 changes: 1 addition & 1 deletion programs/validator-bonds/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub enum ErrorCode {
ClaimAmountExceedsMaxTotalClaim, // 6030 0x178e

#[msg("Claim exceeded number of claimable nodes in the merkle tree")]
ClaimAmountExceedsMaxNumNodes, // 6031 0x178f
ClaimCountExceedsMaxNumNodes, // 6031 0x178f

#[msg("Empty merkle tree, nothing to be claimed")]
EmptySettlementMerkleTree, // 6032 0x1790
Expand Down
6 changes: 3 additions & 3 deletions programs/validator-bonds/src/events/bond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@ pub struct InitBondEvent {
pub validator_vote_account: Pubkey,
pub validator_vote_withdrawer: Pubkey,
pub authority: Pubkey,
pub revenue_share_config: HundredthBasisPoint,
pub revenue_share: HundredthBasisPoint,
pub bond_bump: u8,
}

#[event]
pub struct ConfigureBondEvent {
pub bond_authority: Option<PubkeyValueChange>,
pub revenue_share_config: Option<HundrethBasisPointChange>,
pub revenue_share: Option<HundrethBasisPointChange>,
}

#[event]
pub struct CloseBondEvent {
pub config_address: Pubkey,
pub validator_vote_account: Pubkey,
pub authority: Pubkey,
pub revenue_share_config: HundredthBasisPoint,
pub revenue_share: HundredthBasisPoint,
pub bump: u8,
}

Expand Down
2 changes: 2 additions & 0 deletions programs/validator-bonds/src/events/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub struct InitConfigEvent {
pub operator_authority: Pubkey,
pub withdraw_lockup_epochs: u64,
pub epochs_to_claim_settlement: u64,
pub minimum_stake_lamports: u64,
pub bonds_withdrawer_authority: Pubkey,
pub bonds_withdrawer_authority_bump: u8,
}
Expand All @@ -16,5 +17,6 @@ pub struct ConfigureConfigEvent {
pub admin_authority: Option<PubkeyValueChange>,
pub operator_authority: Option<PubkeyValueChange>,
pub epochs_to_claim_settlement: Option<U64ValueChange>,
pub minimum_stake_lamports: Option<U64ValueChange>,
pub withdraw_lockup_epochs: Option<U64ValueChange>,
}
4 changes: 2 additions & 2 deletions programs/validator-bonds/src/events/settlement_claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use anchor_lang::prelude::*;
pub struct ClaimSettlementEvent {
pub settlement: Pubkey,
pub settlement_claim: Pubkey,
pub stake_authority: Pubkey,
pub withdraw_authority: Pubkey,
pub staker_authority: Pubkey,
pub withdrawer_authority: Pubkey,
pub vote_account: Pubkey,
pub claim: u64,
pub rent_collector: Pubkey,
Expand Down
1 change: 1 addition & 0 deletions programs/validator-bonds/src/events/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct ResetEvent {
pub bond: Pubkey,
pub settlement: Pubkey,
pub stake_account: Pubkey,
pub validator_vote_acount: Pubkey,
pub settlement_authority: Pubkey,
pub bonds_withdrawer_authority: Pubkey,
}
12 changes: 6 additions & 6 deletions programs/validator-bonds/src/instructions/bond/configure_bond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use anchor_lang::prelude::*;
#[derive(AnchorDeserialize, AnchorSerialize)]
pub struct ConfigureBondArgs {
pub bond_authority: Option<Pubkey>,
pub revenue_share_config: Option<HundredthBasisPoint>,
pub revenue_share: Option<HundredthBasisPoint>,
}

/// Change parameters of validator bond account
Expand Down Expand Up @@ -40,7 +40,7 @@ impl<'info> ConfigureBond<'info> {
&mut self,
ConfigureBondArgs {
bond_authority,
revenue_share_config,
revenue_share,
}: ConfigureBondArgs,
) -> Result<()> {
require!(
Expand All @@ -60,18 +60,18 @@ impl<'info> ConfigureBond<'info> {
new: authority,
}
});
let revenue_share_config_change = match revenue_share_config {
let revenue_share_change = match revenue_share {
Some(revenue) => {
let old = self.bond.revenue_share_config;
self.bond.revenue_share_config = revenue.check()?;
let old = self.bond.revenue_share;
self.bond.revenue_share = revenue.check()?;
Some(HundrethBasisPointChange { old, new: revenue })
}
None => None,
};

emit!(ConfigureBondEvent {
bond_authority: bond_authority_change,
revenue_share_config: revenue_share_config_change,
revenue_share: revenue_share_change,
});

Ok(())
Expand Down
Loading

0 comments on commit e452e2f

Please sign in to comment.