From 4662399884e955cfb51c114bb8868cd05b850997 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Fri, 21 Jan 2022 14:29:51 -0600 Subject: [PATCH 01/18] 10461 Reject close of vote accounts unless it earned no credits in the previous epoch --- programs/vote/src/vote_processor.rs | 5 +- programs/vote/src/vote_state/mod.rs | 299 +++++++++++++++++- sdk/program/src/instruction.rs | 5 + sdk/program/src/program_error.rs | 8 + sdk/src/feature_set.rs | 4 + storage-proto/proto/transaction_by_addr.proto | 1 + storage-proto/src/convert.rs | 4 + 7 files changed, 311 insertions(+), 15 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 2e84131c54ea83..95eb3c47a501fb 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -153,7 +153,10 @@ pub fn process_instruction( } else { None }; - vote_state::withdraw(me, lamports, to, &signers, rent_sysvar.as_deref()) + let reject_vote_account_close_feature_active = + invoke_context.feature_set.is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); + + vote_state::withdraw(me, lamports, to, &signers, rent_sysvar.as_deref(), reject_vote_account_close_feature_active) } VoteInstruction::AuthorizeChecked(vote_authorize) => { if invoke_context diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index a67749fef0d91b..e29ccfd9720708 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1151,6 +1151,7 @@ pub fn withdraw( to_account: &KeyedAccount, signers: &HashSet, rent_sysvar: Option<&Rent>, + reject_vote_account_close_feature_active: bool, ) -> Result<(), InstructionError> { let vote_state: VoteState = State::::state(vote_account)?.convert_to_current(); @@ -1161,10 +1162,25 @@ pub fn withdraw( .lamports()? .checked_sub(lamports) .ok_or(InstructionError::InsufficientFunds)?; + let received_credits_previous_epoch = match vote_state.epoch_credits.len() { + 0 => false, + _ => { + let maybe_last_epoch_credits = vote_state.epoch_credits.last(); + match maybe_last_epoch_credits { + None => false, + Some(last_epoch_credits) => (last_epoch_credits.1 - last_epoch_credits.2) > 0 + } + } + }; if remaining_balance == 0 { - // Deinitialize upon zero-balance - vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; + if reject_vote_account_close_feature_active && received_credits_previous_epoch { + return Err(InstructionError::ActiveVoteAccountClose); + } + else { + // Deinitialize upon zero-balance + vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; + } } else if let Some(rent_sysvar) = rent_sysvar { let min_rent_exempt_balance = rent_sysvar.minimum_balance(vote_account.data_len()?); if remaining_balance < min_rent_exempt_balance { @@ -1438,6 +1454,31 @@ mod tests { ) } + fn create_test_account_with_epoch_credits(last_epoch_zero_credit: bool) -> (Pubkey, RefCell) { + let (vote_pubkey, vote_account) = create_test_account(); + let vote_account_space = vote_account.borrow().data().len(); + + let mut vote_state = VoteState::from(&*vote_account.borrow_mut()).unwrap(); + vote_state.authorized_withdrawer = vote_pubkey; + + vote_state.increment_credits(1); + vote_state.increment_credits(1); + vote_state.increment_credits(2); + + if last_epoch_zero_credit { + let last_epoch_credits = vote_state.epoch_credits.last().unwrap(); + let zero_credit_epoch = (last_epoch_credits.0+1, last_epoch_credits.1, last_epoch_credits.1); + vote_state.epoch_credits.push(zero_credit_epoch); + } + let lamports = vote_account.borrow().lamports(); + let mut vote_account_with_epoch_credits = AccountSharedData::new(lamports, vote_account_space, &vote_pubkey); + let versioned = VoteStateVersions::new_current(vote_state); + VoteState::to(&versioned, &mut vote_account_with_epoch_credits); + let ref_vote_account_with_epoch_credits = RefCell::new(vote_account_with_epoch_credits); + + (vote_pubkey, ref_vote_account_with_epoch_credits) + } + fn simulate_process_vote( vote_pubkey: &Pubkey, vote_account: &RefCell, @@ -2236,6 +2277,7 @@ mod tests { ), &signers, None, + false, ); assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); @@ -2253,17 +2295,19 @@ mod tests { ), &signers, None, + false, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); - // non rent exempt withdraw, before feature activation + // non rent exempt withdraw, before 7txXZZD6 feature activation + // without 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account) = create_test_account(); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; - let lamports = vote_account.borrow().lamports(); + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar - .minimum_balance(vote_account.borrow().data().len()) + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) .max(1); assert!(minimum_balance <= lamports); let signers: HashSet = get_signers(keyed_accounts); @@ -2277,18 +2321,101 @@ mod tests { ), &signers, None, + false, ); assert_eq!(res, Ok(())); } - // non rent exempt withdraw, after feature activation + // non rent exempt withdraw, before 7txXZZD6 feature activation + // with 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account) = create_test_account(); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; - let lamports = vote_account.borrow().lamports(); + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar - .minimum_balance(vote_account.borrow().data().len()) + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + None, + false, + ); + assert_eq!(res, Ok(())); + } + + // non rent exempt withdraw, before 7txXZZD6 feature activation + // without 0 credit epoch, after ALBk3EWd feature activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + None, + true, + ); + assert_eq!(res, Ok(())); + } + + // non rent exempt withdraw, before 7txXZZD6 feature activation + // with 0 credit epoch, after ALBk3EWd activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + None, + true, + ); + assert_eq!(res, Ok(())); + } + + // non rent exempt withdraw, after 7txXZZD6 feature activation + // with 0 credit epoch, before ALBk3EWd feature activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) .max(1); assert!(minimum_balance <= lamports); let signers: HashSet = get_signers(keyed_accounts); @@ -2302,6 +2429,88 @@ mod tests { ), &signers, Some(&rent_sysvar), + false, + ); + assert_eq!(res, Err(InstructionError::InsufficientFunds)); + } + + // non rent exempt withdraw, after 7txXZZD6 feature activation + // without 0 credit epoch, before ALBk3EWd feature activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + Some(&rent_sysvar), + false, + ); + assert_eq!(res, Err(InstructionError::InsufficientFunds)); + } + + // non rent exempt withdraw, after 7txXZZD6 feature activation + // with 0 credit epoch, after ALBk3EWd feature activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + Some(&rent_sysvar), + true, + ); + assert_eq!(res, Err(InstructionError::InsufficientFunds)); + } + + // non rent exempt withdraw, after 7txXZZD6 feature activation + // without 0 credit epoch, after ALBk3EWd feature activation + { + let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let lamports = vote_account_with_epoch_credits.borrow().lamports(); + let rent_sysvar = Rent::default(); + let minimum_balance = rent_sysvar + .minimum_balance(vote_account_with_epoch_credits.borrow().data().len()) + .max(1); + assert!(minimum_balance <= lamports); + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports - minimum_balance + 1, + &KeyedAccount::new( + &solana_sdk::pubkey::new_rand(), + false, + &RefCell::new(AccountSharedData::default()), + ), + &signers, + Some(&rent_sysvar), + true, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2325,6 +2534,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, Some(&rent_sysvar), + false, ); assert_eq!(res, Ok(())); assert_eq!( @@ -2334,12 +2544,43 @@ mod tests { assert_eq!(to_account.borrow().lamports(), withdraw_lamports); } - // full withdraw, before/after activation + // full withdraw, before/after 7txXZZD6 feature activation + // with/without 0 credit epoch, before ALBk3EWd feature activation + { + let rent_sysvar = Rent::default(); + for rent_sysvar in [None, Some(&rent_sysvar)] { + for last_epoch_zero_credits in [false, true] { + let to_account = RefCell::new(AccountSharedData::default()); + let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(last_epoch_zero_credits); + let lamports = vote_account.borrow().lamports(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports, + &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), + &signers, + rent_sysvar, + false, + ); + assert_eq!(res, Ok(())); + assert_eq!(vote_account.borrow().lamports(), 0); + assert_eq!(to_account.borrow().lamports(), lamports); + let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); + // State has been deinitialized since balance is zero + assert!(post_state.is_uninitialized()); + } + } + } + + // full withdraw, before/after 7txXZZD6 feature activation + // with 0 credit epoch, after ALBk3EWd feature activation { let rent_sysvar = Rent::default(); for rent_sysvar in [None, Some(&rent_sysvar)] { let to_account = RefCell::new(AccountSharedData::default()); - let (vote_pubkey, vote_account) = create_test_account(); + // let (vote_pubkey, vote_account) = create_test_account(); + let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(true); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); @@ -2349,6 +2590,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, rent_sysvar, + true, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); @@ -2359,6 +2601,34 @@ mod tests { } } + // full withdraw, before/after 7txXZZD6 feature activation + // without 0 credit epoch, after ALBk3EWd feature activation + { + let rent_sysvar = Rent::default(); + for rent_sysvar in [None, Some(&rent_sysvar)] { + let to_account = RefCell::new(AccountSharedData::default()); + // let (vote_pubkey, vote_account) = create_test_account(); + let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(false); + let lamports = vote_account.borrow().lamports(); + let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; + let signers: HashSet = get_signers(keyed_accounts); + let res = withdraw( + &keyed_accounts[0], + lamports, + &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), + &signers, + rent_sysvar, + true, + ); + assert_eq!(res, Err(InstructionError::ActiveVoteAccountClose)); + assert_eq!(vote_account.borrow().lamports(), lamports); + assert_eq!(to_account.borrow().lamports(), 0); + let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); + // State has been deinitialized since balance is zero + assert!(!post_state.is_uninitialized()); + } + } + // authorize authorized_withdrawer let authorized_withdrawer_pubkey = solana_sdk::pubkey::new_rand(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; @@ -2388,6 +2658,7 @@ mod tests { withdrawer_keyed_account, &signers, None, + false, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index e1fbb1825a845a..6e3e2f57b4c990 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -252,6 +252,11 @@ pub enum InstructionError { /// Accounts data budget exceeded #[error("Requested account data allocation exceeded the accounts data budget")] AccountsDataBudgetExceeded, + + /// Active vote account close + #[error("Cannot close vote account unless it received 0 credits in the most recent epoch")] + ActiveVoteAccountClose + // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added } diff --git a/sdk/program/src/program_error.rs b/sdk/program/src/program_error.rs index c47638dddb29b9..c4eac10be99807 100644 --- a/sdk/program/src/program_error.rs +++ b/sdk/program/src/program_error.rs @@ -51,6 +51,8 @@ pub enum ProgramError { IllegalOwner, #[error("Requested account data allocation exceeded the accounts data budget")] AccountsDataBudgetExceeded, + #[error("Cannot close vote account unless it received 0 credits in the most recent epoch")] + ActiveVoteAccountClose, } pub trait PrintProgramError { @@ -90,6 +92,7 @@ impl PrintProgramError for ProgramError { Self::UnsupportedSysvar => msg!("Error: UnsupportedSysvar"), Self::IllegalOwner => msg!("Error: IllegalOwner"), Self::AccountsDataBudgetExceeded => msg!("Error: AccountsDataBudgetExceeded"), + Self::ActiveVoteAccountClose => msg!("Error: ActiveVoteAccountClose"), } } } @@ -121,6 +124,7 @@ pub const ACCOUNT_NOT_RENT_EXEMPT: u64 = to_builtin!(16); pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17); pub const ILLEGAL_OWNER: u64 = to_builtin!(18); pub const ACCOUNTS_DATA_BUDGET_EXCEEDED: u64 = to_builtin!(19); +pub const ACTIVE_VOTE_ACCOUNT_CLOSE: u64 = to_builtin!(20); // Warning: Any new program errors added here must also be: // - Added to the below conversions // - Added as an equivilent to InstructionError @@ -148,6 +152,7 @@ impl From for u64 { ProgramError::UnsupportedSysvar => UNSUPPORTED_SYSVAR, ProgramError::IllegalOwner => ILLEGAL_OWNER, ProgramError::AccountsDataBudgetExceeded => ACCOUNTS_DATA_BUDGET_EXCEEDED, + ProgramError::ActiveVoteAccountClose => ACTIVE_VOTE_ACCOUNT_CLOSE, ProgramError::Custom(error) => { if error == 0 { CUSTOM_ZERO @@ -181,6 +186,7 @@ impl From for ProgramError { UNSUPPORTED_SYSVAR => Self::UnsupportedSysvar, ILLEGAL_OWNER => Self::IllegalOwner, ACCOUNTS_DATA_BUDGET_EXCEEDED => Self::AccountsDataBudgetExceeded, + ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, _ => Self::Custom(error as u32), } } @@ -210,6 +216,7 @@ impl TryFrom for ProgramError { Self::Error::UnsupportedSysvar => Ok(Self::UnsupportedSysvar), Self::Error::IllegalOwner => Ok(Self::IllegalOwner), Self::Error::AccountsDataBudgetExceeded => Ok(Self::AccountsDataBudgetExceeded), + Self::Error::ActiveVoteAccountClose => Ok(Self::ActiveVoteAccountClose), _ => Err(error), } } @@ -241,6 +248,7 @@ where UNSUPPORTED_SYSVAR => Self::UnsupportedSysvar, ILLEGAL_OWNER => Self::IllegalOwner, ACCOUNTS_DATA_BUDGET_EXCEEDED => Self::AccountsDataBudgetExceeded, + ACTIVE_VOTE_ACCOUNT_CLOSE => Self::ActiveVoteAccountClose, _ => { // A valid custom error has no bits set in the upper 32 if error >> BUILTIN_BIT_SHIFT == 0 { diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index b8a954f7967721..53f5adf37f40d1 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -305,6 +305,9 @@ pub mod vote_withdraw_authority_may_change_authorized_voter { pub mod spl_associated_token_account_v1_0_4 { solana_sdk::declare_id!("FaTa4SpiaSNH44PGC4z8bnGVTkSRYaWvrBs3KTu8XQQq"); + +pub mod reject_vote_account_close_unless_zero_credit_epoch { + solana_sdk::declare_id!("ALBk3EWdeAg2WAGf6GPDUf1nynyNqCdEVmgouG7rpuCj"); } lazy_static! { @@ -378,6 +381,7 @@ lazy_static! { (update_syscall_base_costs::id(), "Update syscall base costs"), (vote_withdraw_authority_may_change_authorized_voter::id(), "vote account withdraw authority may change the authorized voter #22521"), (spl_associated_token_account_v1_0_4::id(), "SPL Associated Token Account Program release version 1.0.4, tied to token 3.3.0 #22648"), + (reject_vote_account_close_unless_zero_credit_epoch::id(), "fail vote account withdraw to 0 unless account earned 0 credits in last completed epoch"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index ee88455e66c65f..c12cdd06173ce2 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -113,6 +113,7 @@ enum InstructionErrorType { UNSUPPORTED_SYSVAR = 48; ILLEGAL_OWNER = 49; ACCOUNTS_DATA_BUDGET_EXCEEDED = 50; + ACTIVE_VOTE_ACCOUNT_CLOSE = 51; } message UnixTimestamp { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 02698cb6194ca6..f3ad0395d46287 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -689,6 +689,7 @@ impl TryFrom for TransactionError { 48 => InstructionError::UnsupportedSysvar, 49 => InstructionError::IllegalOwner, 50 => InstructionError::AccountsDataBudgetExceeded, + 51 => InstructionError::ActiveVoteAccountClose, _ => return Err("Invalid InstructionError"), }; @@ -979,6 +980,9 @@ impl From for tx_by_addr::TransactionError { InstructionError::AccountsDataBudgetExceeded => { tx_by_addr::InstructionErrorType::AccountsDataBudgetExceeded } + InstructionError::ActiveVoteAccountClose => { + tx_by_addr::InstructionErrorType::ActiveVoteAccountClose + } } as i32, custom: match instruction_error { InstructionError::Custom(custom) => { From 6c7c151ef6361028bdbfed3b3a1e16ef574b645b Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Fri, 21 Jan 2022 15:30:00 -0600 Subject: [PATCH 02/18] lint --- programs/vote/src/vote_processor.rs | 2 +- programs/vote/src/vote_state/mod.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 95eb3c47a501fb..35d6608ace2875 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -153,7 +153,7 @@ pub fn process_instruction( } else { None }; - let reject_vote_account_close_feature_active = + let reject_vote_account_close_feature_active = invoke_context.feature_set.is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); vote_state::withdraw(me, lamports, to, &signers, rent_sysvar.as_deref(), reject_vote_account_close_feature_active) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index e29ccfd9720708..1eea33c8cbe76d 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -2408,7 +2408,7 @@ mod tests { } // non rent exempt withdraw, after 7txXZZD6 feature activation - // with 0 credit epoch, before ALBk3EWd feature activation + // with 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; @@ -2435,7 +2435,7 @@ mod tests { } // non rent exempt withdraw, after 7txXZZD6 feature activation - // without 0 credit epoch, before ALBk3EWd feature activation + // without 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; @@ -2462,7 +2462,7 @@ mod tests { } // non rent exempt withdraw, after 7txXZZD6 feature activation - // with 0 credit epoch, after ALBk3EWd feature activation + // with 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; From f54e0ecca04c63bc66c1526e1e6122a44bb26911 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Mon, 24 Jan 2022 11:00:45 -0600 Subject: [PATCH 03/18] lint --- programs/vote/src/vote_processor.rs | 14 ++++- programs/vote/src/vote_state/mod.rs | 93 ++++++++++++++++++++++------- sdk/program/src/instruction.rs | 3 +- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 35d6608ace2875..8839158f34e9ec 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -153,10 +153,18 @@ pub fn process_instruction( } else { None }; - let reject_vote_account_close_feature_active = - invoke_context.feature_set.is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); + let reject_vote_account_close_feature_active = invoke_context + .feature_set + .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); - vote_state::withdraw(me, lamports, to, &signers, rent_sysvar.as_deref(), reject_vote_account_close_feature_active) + vote_state::withdraw( + me, + lamports, + to, + &signers, + rent_sysvar.as_deref(), + reject_vote_account_close_feature_active, + ) } VoteInstruction::AuthorizeChecked(vote_authorize) => { if invoke_context diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 1eea33c8cbe76d..108c744d83707c 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1168,7 +1168,7 @@ pub fn withdraw( let maybe_last_epoch_credits = vote_state.epoch_credits.last(); match maybe_last_epoch_credits { None => false, - Some(last_epoch_credits) => (last_epoch_credits.1 - last_epoch_credits.2) > 0 + Some(last_epoch_credits) => (last_epoch_credits.1 - last_epoch_credits.2) > 0, } } }; @@ -1176,8 +1176,7 @@ pub fn withdraw( if remaining_balance == 0 { if reject_vote_account_close_feature_active && received_credits_previous_epoch { return Err(InstructionError::ActiveVoteAccountClose); - } - else { + } else { // Deinitialize upon zero-balance vote_account.set_state(&VoteStateVersions::new_current(VoteState::default()))?; } @@ -1454,7 +1453,9 @@ mod tests { ) } - fn create_test_account_with_epoch_credits(last_epoch_zero_credit: bool) -> (Pubkey, RefCell) { + fn create_test_account_with_epoch_credits( + last_epoch_zero_credit: bool, + ) -> (Pubkey, RefCell) { let (vote_pubkey, vote_account) = create_test_account(); let vote_account_space = vote_account.borrow().data().len(); @@ -1467,11 +1468,16 @@ mod tests { if last_epoch_zero_credit { let last_epoch_credits = vote_state.epoch_credits.last().unwrap(); - let zero_credit_epoch = (last_epoch_credits.0+1, last_epoch_credits.1, last_epoch_credits.1); + let zero_credit_epoch = ( + last_epoch_credits.0 + 1, + last_epoch_credits.1, + last_epoch_credits.1, + ); vote_state.epoch_credits.push(zero_credit_epoch); } let lamports = vote_account.borrow().lamports(); - let mut vote_account_with_epoch_credits = AccountSharedData::new(lamports, vote_account_space, &vote_pubkey); + let mut vote_account_with_epoch_credits = + AccountSharedData::new(lamports, vote_account_space, &vote_pubkey); let versioned = VoteStateVersions::new_current(vote_state); VoteState::to(&versioned, &mut vote_account_with_epoch_credits); let ref_vote_account_with_epoch_credits = RefCell::new(vote_account_with_epoch_credits); @@ -2302,8 +2308,13 @@ mod tests { // non rent exempt withdraw, before 7txXZZD6 feature activation // without 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2329,8 +2340,13 @@ mod tests { // non rent exempt withdraw, before 7txXZZD6 feature activation // with 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2356,8 +2372,13 @@ mod tests { // non rent exempt withdraw, before 7txXZZD6 feature activation // without 0 credit epoch, after ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2383,8 +2404,13 @@ mod tests { // non rent exempt withdraw, before 7txXZZD6 feature activation // with 0 credit epoch, after ALBk3EWd activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2410,8 +2436,13 @@ mod tests { // non rent exempt withdraw, after 7txXZZD6 feature activation // with 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2437,8 +2468,13 @@ mod tests { // non rent exempt withdraw, after 7txXZZD6 feature activation // without 0 credit epoch, before ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2464,8 +2500,13 @@ mod tests { // non rent exempt withdraw, after 7txXZZD6 feature activation // with 0 credit epoch, after ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(true); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(true); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2491,8 +2532,13 @@ mod tests { // non rent exempt withdraw, after 7txXZZD6 feature activation // without 0 credit epoch, after ALBk3EWd feature activation { - let (vote_pubkey, vote_account_with_epoch_credits) = create_test_account_with_epoch_credits(false); - let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account_with_epoch_credits)]; + let (vote_pubkey, vote_account_with_epoch_credits) = + create_test_account_with_epoch_credits(false); + let keyed_accounts = &[KeyedAccount::new( + &vote_pubkey, + true, + &vote_account_with_epoch_credits, + )]; let lamports = vote_account_with_epoch_credits.borrow().lamports(); let rent_sysvar = Rent::default(); let minimum_balance = rent_sysvar @@ -2551,7 +2597,8 @@ mod tests { for rent_sysvar in [None, Some(&rent_sysvar)] { for last_epoch_zero_credits in [false, true] { let to_account = RefCell::new(AccountSharedData::default()); - let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(last_epoch_zero_credits); + let (vote_pubkey, vote_account) = + create_test_account_with_epoch_credits(last_epoch_zero_credits); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index 6e3e2f57b4c990..b62f4d4b68aa25 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -255,8 +255,7 @@ pub enum InstructionError { /// Active vote account close #[error("Cannot close vote account unless it received 0 credits in the most recent epoch")] - ActiveVoteAccountClose - + ActiveVoteAccountClose, // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added } From 39350cf1d703a24e9a59438c2256084663ea0c9a Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Mon, 24 Jan 2022 15:19:53 -0600 Subject: [PATCH 04/18] Update bank::BankSlotDelta_frozen_abi::test_abi_digest --- runtime/src/bank.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5595d685876d74..db2d3080baf32c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -214,7 +214,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "FPLuTUU5MjwsijzDubxY6BvBEkWULhYNUyY6Puqejb4g")] +#[frozen_abi(digest = "6XkxpmzmKZguLZMS1KmU7N2dAcv8MmNhyobJCwRLkTdi")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. From bc684d3c14a859a5b0c2c5d3859fd656019436cf Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 09:52:06 -0600 Subject: [PATCH 05/18] Suggestion: name tuple elements. use checked math. Co-authored-by: Tyera Eulberg --- programs/vote/src/vote_state/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 108c744d83707c..ca91ed67bbc1a8 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1168,7 +1168,7 @@ pub fn withdraw( let maybe_last_epoch_credits = vote_state.epoch_credits.last(); match maybe_last_epoch_credits { None => false, - Some(last_epoch_credits) => (last_epoch_credits.1 - last_epoch_credits.2) > 0, + Some((epoch_credits, previous_epoch_credits)) => epoch_credits.saturating_sub(previous_epoch_credits) > 0, } } }; From 0cad77ea74cc54e25882553b374431f76a31cfae Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 09:54:26 -0600 Subject: [PATCH 06/18] Suggestion: Fix incorrect comment. Co-authored-by: Tyera Eulberg --- programs/vote/src/vote_state/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index ca91ed67bbc1a8..7f417d2ed08e2c 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -2671,7 +2671,7 @@ mod tests { assert_eq!(vote_account.borrow().lamports(), lamports); assert_eq!(to_account.borrow().lamports(), 0); let post_state: VoteStateVersions = vote_account.borrow().state().unwrap(); - // State has been deinitialized since balance is zero + // State is still initialized assert!(!post_state.is_uninitialized()); } } From ea2b33f095a1d66ebe28c7c28a0ad42331aa5724 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 17:05:53 -0600 Subject: [PATCH 07/18] Review feedback. Check 2nd to last epoch credits. --- programs/vote/src/vote_state/mod.rs | 65 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 7f417d2ed08e2c..0ac90b2648f345 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1163,13 +1163,11 @@ pub fn withdraw( .checked_sub(lamports) .ok_or(InstructionError::InsufficientFunds)?; let received_credits_previous_epoch = match vote_state.epoch_credits.len() { - 0 => false, + 0..=1 => false, _ => { - let maybe_last_epoch_credits = vote_state.epoch_credits.last(); - match maybe_last_epoch_credits { - None => false, - Some((epoch_credits, previous_epoch_credits)) => epoch_credits.saturating_sub(previous_epoch_credits) > 0, - } + let (_epoch, epoch_credits, previous_epoch_credits) = + vote_state.epoch_credits[vote_state.epoch_credits.len() - 2]; + epoch_credits.saturating_sub(previous_epoch_credits) > 0 } }; @@ -1454,7 +1452,7 @@ mod tests { } fn create_test_account_with_epoch_credits( - last_epoch_zero_credit: bool, + credits_to_append: &Vec, ) -> (Pubkey, RefCell) { let (vote_pubkey, vote_account) = create_test_account(); let vote_account_space = vote_account.borrow().data().len(); @@ -1462,19 +1460,20 @@ mod tests { let mut vote_state = VoteState::from(&*vote_account.borrow_mut()).unwrap(); vote_state.authorized_withdrawer = vote_pubkey; - vote_state.increment_credits(1); - vote_state.increment_credits(1); - vote_state.increment_credits(2); + vote_state.epoch_credits = Vec::new(); - if last_epoch_zero_credit { - let last_epoch_credits = vote_state.epoch_credits.last().unwrap(); - let zero_credit_epoch = ( - last_epoch_credits.0 + 1, - last_epoch_credits.1, - last_epoch_credits.1, - ); - vote_state.epoch_credits.push(zero_credit_epoch); + let mut epoch = 0; + let mut current_epoch_credits = 0; + let mut previous_epoch_credits = 0; + for credits in credits_to_append { + current_epoch_credits = current_epoch_credits + credits; + vote_state + .epoch_credits + .push((epoch, current_epoch_credits, previous_epoch_credits)); + epoch = epoch + 1; + previous_epoch_credits = current_epoch_credits; } + let lamports = vote_account.borrow().lamports(); let mut vote_account_with_epoch_credits = AccountSharedData::new(lamports, vote_account_space, &vote_pubkey); @@ -2269,6 +2268,8 @@ mod tests { #[test] fn test_vote_state_withdraw() { let (vote_pubkey, vote_account) = create_test_account(); + let with_zero_credit_epoch: Vec = vec![2, 0, 3]; + let without_zero_credit_epoch: Vec = vec![2, 1, 3]; // unsigned request let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, false, &vote_account)]; @@ -2309,7 +2310,7 @@ mod tests { // without 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(false); + create_test_account_with_epoch_credits(&without_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2341,7 +2342,7 @@ mod tests { // with 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(true); + create_test_account_with_epoch_credits(&with_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2373,7 +2374,7 @@ mod tests { // without 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(false); + create_test_account_with_epoch_credits(&without_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2405,7 +2406,7 @@ mod tests { // with 0 credit epoch, after ALBk3EWd activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(true); + create_test_account_with_epoch_credits(&with_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2437,7 +2438,7 @@ mod tests { // with 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(true); + create_test_account_with_epoch_credits(&with_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2469,7 +2470,7 @@ mod tests { // without 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(false); + create_test_account_with_epoch_credits(&without_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2501,7 +2502,7 @@ mod tests { // with 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(true); + create_test_account_with_epoch_credits(&with_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2533,7 +2534,7 @@ mod tests { // without 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(false); + create_test_account_with_epoch_credits(&without_zero_credit_epoch); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2561,7 +2562,7 @@ mod tests { assert_eq!(res, Err(InstructionError::InsufficientFunds)); } - // partial valid withdraw, after feature activation + // partial valid withdraw, after 7txXZZD6 feature activation { let to_account = RefCell::new(AccountSharedData::default()); let (vote_pubkey, vote_account) = create_test_account(); @@ -2595,10 +2596,10 @@ mod tests { { let rent_sysvar = Rent::default(); for rent_sysvar in [None, Some(&rent_sysvar)] { - for last_epoch_zero_credits in [false, true] { + for credits in [&with_zero_credit_epoch, &without_zero_credit_epoch] { let to_account = RefCell::new(AccountSharedData::default()); let (vote_pubkey, vote_account) = - create_test_account_with_epoch_credits(last_epoch_zero_credits); + create_test_account_with_epoch_credits(credits); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); @@ -2627,7 +2628,8 @@ mod tests { for rent_sysvar in [None, Some(&rent_sysvar)] { let to_account = RefCell::new(AccountSharedData::default()); // let (vote_pubkey, vote_account) = create_test_account(); - let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(true); + let (vote_pubkey, vote_account) = + create_test_account_with_epoch_credits(&with_zero_credit_epoch); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); @@ -2655,7 +2657,8 @@ mod tests { for rent_sysvar in [None, Some(&rent_sysvar)] { let to_account = RefCell::new(AccountSharedData::default()); // let (vote_pubkey, vote_account) = create_test_account(); - let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(false); + let (vote_pubkey, vote_account) = + create_test_account_with_epoch_credits(&without_zero_credit_epoch); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); From f430854da6edef2466c1c6d647829dddac65e2e5 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 19:34:29 -0600 Subject: [PATCH 08/18] clippy lint --- programs/vote/src/vote_state/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 0ac90b2648f345..34353a9d0ef428 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1452,7 +1452,7 @@ mod tests { } fn create_test_account_with_epoch_credits( - credits_to_append: &Vec, + credits_to_append: &[u64], ) -> (Pubkey, RefCell) { let (vote_pubkey, vote_account) = create_test_account(); let vote_account_space = vote_account.borrow().data().len(); @@ -1466,11 +1466,11 @@ mod tests { let mut current_epoch_credits = 0; let mut previous_epoch_credits = 0; for credits in credits_to_append { - current_epoch_credits = current_epoch_credits + credits; + current_epoch_credits += credits; vote_state .epoch_credits .push((epoch, current_epoch_credits, previous_epoch_credits)); - epoch = epoch + 1; + epoch += 1; previous_epoch_credits = current_epoch_credits; } From 4461c496ee4cff7b3ce8e7a20f182fdfcbbf4819 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 20:14:31 -0600 Subject: [PATCH 09/18] lint --- programs/vote/src/vote_state/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 34353a9d0ef428..5d3a8e5f7e5bac 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1462,15 +1462,15 @@ mod tests { vote_state.epoch_credits = Vec::new(); - let mut epoch = 0; + // let mut epoch = 0; let mut current_epoch_credits = 0; let mut previous_epoch_credits = 0; - for credits in credits_to_append { + for (epoch, credits) in credits_to_append.into_iter().enumerate() { current_epoch_credits += credits; vote_state .epoch_credits - .push((epoch, current_epoch_credits, previous_epoch_credits)); - epoch += 1; + .push((u64::try_from(epoch).unwrap(), current_epoch_credits, previous_epoch_credits)); + // epoch += 1; previous_epoch_credits = current_epoch_credits; } From 5d04b2a608b0fa81c663f84e35b866dbd635f7b9 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 20:25:37 -0600 Subject: [PATCH 10/18] lint --- programs/vote/src/vote_state/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 5d3a8e5f7e5bac..9d048e555ff945 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1465,7 +1465,7 @@ mod tests { // let mut epoch = 0; let mut current_epoch_credits = 0; let mut previous_epoch_credits = 0; - for (epoch, credits) in credits_to_append.into_iter().enumerate() { + for (epoch, credits) in credits_to_append.iter().enumerate() { current_epoch_credits += credits; vote_state .epoch_credits From e5de27e8942361379b4688f5daea576a23a5c4ef Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 20:31:55 -0600 Subject: [PATCH 11/18] lint --- programs/vote/src/vote_state/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 9d048e555ff945..a94e43faea459d 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1467,9 +1467,11 @@ mod tests { let mut previous_epoch_credits = 0; for (epoch, credits) in credits_to_append.iter().enumerate() { current_epoch_credits += credits; - vote_state - .epoch_credits - .push((u64::try_from(epoch).unwrap(), current_epoch_credits, previous_epoch_credits)); + vote_state.epoch_credits.push(( + u64::try_from(epoch).unwrap(), + current_epoch_credits, + previous_epoch_credits, + )); // epoch += 1; previous_epoch_credits = current_epoch_credits; } From ceb88e5c2b893014fcaef7056d52f219dce7f8a8 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Tue, 25 Jan 2022 23:02:50 -0600 Subject: [PATCH 12/18] fix missing } from merge conflict --- sdk/src/feature_set.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 53f5adf37f40d1..002a32487e49a1 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -305,6 +305,7 @@ pub mod vote_withdraw_authority_may_change_authorized_voter { pub mod spl_associated_token_account_v1_0_4 { solana_sdk::declare_id!("FaTa4SpiaSNH44PGC4z8bnGVTkSRYaWvrBs3KTu8XQQq"); +} pub mod reject_vote_account_close_unless_zero_credit_epoch { solana_sdk::declare_id!("ALBk3EWdeAg2WAGf6GPDUf1nynyNqCdEVmgouG7rpuCj"); From 1feb4aea5b59f6060b357edc64811dbeda3730a3 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Wed, 26 Jan 2022 22:37:39 -0600 Subject: [PATCH 13/18] Use clock to determine current epoch and compare to most recent epoch that has credits in vote_state --- programs/vote/src/vote_processor.rs | 6 +++ programs/vote/src/vote_state/mod.rs | 63 ++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index 8839158f34e9ec..cb7ca3f068ff94 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -27,6 +27,7 @@ use { mod get_sysvar_with_keyed_account_check { use super::*; + // This might work pub fn clock( keyed_account: &KeyedAccount, invoke_context: &InvokeContext, @@ -157,6 +158,10 @@ pub fn process_instruction( .feature_set .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); + let clock = &from_keyed_account::(keyed_account_at_index( + keyed_accounts, + first_instruction_account + 1, + )?)?; vote_state::withdraw( me, lamports, @@ -164,6 +169,7 @@ pub fn process_instruction( &signers, rent_sysvar.as_deref(), reject_vote_account_close_feature_active, + clock, ) } VoteInstruction::AuthorizeChecked(vote_authorize) => { diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index a94e43faea459d..a04c78ccfac369 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1152,6 +1152,7 @@ pub fn withdraw( signers: &HashSet, rent_sysvar: Option<&Rent>, reject_vote_account_close_feature_active: bool, + clock: &Clock, ) -> Result<(), InstructionError> { let vote_state: VoteState = State::::state(vote_account)?.convert_to_current(); @@ -1162,12 +1163,16 @@ pub fn withdraw( .lamports()? .checked_sub(lamports) .ok_or(InstructionError::InsufficientFunds)?; - let received_credits_previous_epoch = match vote_state.epoch_credits.len() { - 0..=1 => false, - _ => { - let (_epoch, epoch_credits, previous_epoch_credits) = - vote_state.epoch_credits[vote_state.epoch_credits.len() - 2]; - epoch_credits.saturating_sub(previous_epoch_credits) > 0 + + let received_credits_previous_epoch = if vote_state.epoch_credits.is_empty() { + false + } else { + let maybe_last_epoch_with_credits = vote_state.epoch_credits.last(); + match maybe_last_epoch_with_credits { + None => false, + Some(last_epoch_with_credits) => { + clock.epoch.saturating_sub(last_epoch_with_credits.0) < 2 + } } }; @@ -2270,8 +2275,13 @@ mod tests { #[test] fn test_vote_state_withdraw() { let (vote_pubkey, vote_account) = create_test_account(); - let with_zero_credit_epoch: Vec = vec![2, 0, 3]; - let without_zero_credit_epoch: Vec = vec![2, 1, 3]; + let credits_through_epoch_1: Vec = vec![2, 1]; + let credits_through_epoch_2: Vec = vec![2, 1, 3]; + + let clock_epoch_3 = &Clock { + epoch: 3, + ..Clock::default() + }; // unsigned request let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, false, &vote_account)]; @@ -2287,6 +2297,7 @@ mod tests { &signers, None, false, + &Clock::default(), ); assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); @@ -2305,6 +2316,7 @@ mod tests { &signers, None, false, + &Clock::default(), ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); @@ -2312,7 +2324,7 @@ mod tests { // without 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&without_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_2); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2336,6 +2348,7 @@ mod tests { &signers, None, false, + clock_epoch_3, ); assert_eq!(res, Ok(())); } @@ -2344,7 +2357,7 @@ mod tests { // with 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&with_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_1); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2368,6 +2381,7 @@ mod tests { &signers, None, false, + clock_epoch_3, ); assert_eq!(res, Ok(())); } @@ -2376,7 +2390,7 @@ mod tests { // without 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&without_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_2); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2400,6 +2414,7 @@ mod tests { &signers, None, true, + clock_epoch_3, ); assert_eq!(res, Ok(())); } @@ -2408,7 +2423,7 @@ mod tests { // with 0 credit epoch, after ALBk3EWd activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&with_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_1); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2432,6 +2447,7 @@ mod tests { &signers, None, true, + clock_epoch_3, ); assert_eq!(res, Ok(())); } @@ -2440,7 +2456,7 @@ mod tests { // with 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&with_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_1); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2464,6 +2480,7 @@ mod tests { &signers, Some(&rent_sysvar), false, + clock_epoch_3, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2472,7 +2489,7 @@ mod tests { // without 0 credit epoch, before ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&without_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_2); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2496,6 +2513,7 @@ mod tests { &signers, Some(&rent_sysvar), false, + clock_epoch_3, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2504,7 +2522,7 @@ mod tests { // with 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&with_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_1); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2528,6 +2546,7 @@ mod tests { &signers, Some(&rent_sysvar), true, + clock_epoch_3, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2536,7 +2555,7 @@ mod tests { // without 0 credit epoch, after ALBk3EWd feature activation { let (vote_pubkey, vote_account_with_epoch_credits) = - create_test_account_with_epoch_credits(&without_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_2); let keyed_accounts = &[KeyedAccount::new( &vote_pubkey, true, @@ -2560,6 +2579,7 @@ mod tests { &signers, Some(&rent_sysvar), true, + clock_epoch_3, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2584,6 +2604,7 @@ mod tests { &signers, Some(&rent_sysvar), false, + &Clock::default(), ); assert_eq!(res, Ok(())); assert_eq!( @@ -2598,7 +2619,7 @@ mod tests { { let rent_sysvar = Rent::default(); for rent_sysvar in [None, Some(&rent_sysvar)] { - for credits in [&with_zero_credit_epoch, &without_zero_credit_epoch] { + for credits in [&credits_through_epoch_1, &credits_through_epoch_2] { let to_account = RefCell::new(AccountSharedData::default()); let (vote_pubkey, vote_account) = create_test_account_with_epoch_credits(credits); @@ -2612,6 +2633,7 @@ mod tests { &signers, rent_sysvar, false, + clock_epoch_3, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); @@ -2631,7 +2653,7 @@ mod tests { let to_account = RefCell::new(AccountSharedData::default()); // let (vote_pubkey, vote_account) = create_test_account(); let (vote_pubkey, vote_account) = - create_test_account_with_epoch_credits(&with_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_1); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); @@ -2642,6 +2664,7 @@ mod tests { &signers, rent_sysvar, true, + clock_epoch_3, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); @@ -2660,7 +2683,7 @@ mod tests { let to_account = RefCell::new(AccountSharedData::default()); // let (vote_pubkey, vote_account) = create_test_account(); let (vote_pubkey, vote_account) = - create_test_account_with_epoch_credits(&without_zero_credit_epoch); + create_test_account_with_epoch_credits(&credits_through_epoch_2); let lamports = vote_account.borrow().lamports(); let keyed_accounts = &[KeyedAccount::new(&vote_pubkey, true, &vote_account)]; let signers: HashSet = get_signers(keyed_accounts); @@ -2671,6 +2694,7 @@ mod tests { &signers, rent_sysvar, true, + clock_epoch_3, ); assert_eq!(res, Err(InstructionError::ActiveVoteAccountClose)); assert_eq!(vote_account.borrow().lamports(), lamports); @@ -2711,6 +2735,7 @@ mod tests { &signers, None, false, + &Clock::default(), ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); From 4f812a8f6ea172046a8bc212ebc6c120eb7601c3 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Thu, 27 Jan 2022 22:09:40 -0600 Subject: [PATCH 14/18] Sysvar read and use Option<&Clock> param --- programs/vote/src/vote_processor.rs | 18 +++---- programs/vote/src/vote_state/mod.rs | 74 ++++++++++++----------------- 2 files changed, 40 insertions(+), 52 deletions(-) diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index cb7ca3f068ff94..c0b5320e5b9f22 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -27,7 +27,6 @@ use { mod get_sysvar_with_keyed_account_check { use super::*; - // This might work pub fn clock( keyed_account: &KeyedAccount, invoke_context: &InvokeContext, @@ -154,22 +153,23 @@ pub fn process_instruction( } else { None }; - let reject_vote_account_close_feature_active = invoke_context + + let clock_if_feature_active = if invoke_context .feature_set - .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()); + .is_active(&feature_set::reject_vote_account_close_unless_zero_credit_epoch::id()) + { + Some(invoke_context.get_sysvar_cache().get_clock()?) + } else { + None + }; - let clock = &from_keyed_account::(keyed_account_at_index( - keyed_accounts, - first_instruction_account + 1, - )?)?; vote_state::withdraw( me, lamports, to, &signers, rent_sysvar.as_deref(), - reject_vote_account_close_feature_active, - clock, + clock_if_feature_active.as_deref(), ) } VoteInstruction::AuthorizeChecked(vote_authorize) => { diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index a04c78ccfac369..949127f4401c04 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1151,8 +1151,7 @@ pub fn withdraw( to_account: &KeyedAccount, signers: &HashSet, rent_sysvar: Option<&Rent>, - reject_vote_account_close_feature_active: bool, - clock: &Clock, + clock: Option<&Clock>, ) -> Result<(), InstructionError> { let vote_state: VoteState = State::::state(vote_account)?.convert_to_current(); @@ -1164,20 +1163,24 @@ pub fn withdraw( .checked_sub(lamports) .ok_or(InstructionError::InsufficientFunds)?; - let received_credits_previous_epoch = if vote_state.epoch_credits.is_empty() { - false - } else { - let maybe_last_epoch_with_credits = vote_state.epoch_credits.last(); - match maybe_last_epoch_with_credits { + if remaining_balance == 0 { + let reject_active_vote_account_close = match clock { None => false, - Some(last_epoch_with_credits) => { - clock.epoch.saturating_sub(last_epoch_with_credits.0) < 2 + Some(clock) => { + if vote_state.epoch_credits.is_empty() { + false + } else { + let (last_epoch_with_credits, _, _) = vote_state.epoch_credits.last().unwrap(); + let current_epoch = clock.epoch; + // if current_epoch - last_epoch_with_credits < 2 then the validator has received credits + // either in the current epoch or the previous epoch. If it's >= 2 then it has been at least + // one full epoch since the validator has received credits. + current_epoch.saturating_sub(*last_epoch_with_credits) < 2 + } } - } - }; + }; - if remaining_balance == 0 { - if reject_vote_account_close_feature_active && received_credits_previous_epoch { + if reject_active_vote_account_close { return Err(InstructionError::ActiveVoteAccountClose); } else { // Deinitialize upon zero-balance @@ -2296,8 +2299,7 @@ mod tests { ), &signers, None, - false, - &Clock::default(), + None, ); assert_eq!(res, Err(InstructionError::MissingRequiredSignature)); @@ -2315,8 +2317,7 @@ mod tests { ), &signers, None, - false, - &Clock::default(), + Some(&Clock::default()), ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); @@ -2347,8 +2348,7 @@ mod tests { ), &signers, None, - false, - clock_epoch_3, + None, ); assert_eq!(res, Ok(())); } @@ -2380,8 +2380,7 @@ mod tests { ), &signers, None, - false, - clock_epoch_3, + None, ); assert_eq!(res, Ok(())); } @@ -2413,8 +2412,7 @@ mod tests { ), &signers, None, - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Ok(())); } @@ -2446,8 +2444,7 @@ mod tests { ), &signers, None, - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Ok(())); } @@ -2479,8 +2476,7 @@ mod tests { ), &signers, Some(&rent_sysvar), - false, - clock_epoch_3, + None, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2512,8 +2508,7 @@ mod tests { ), &signers, Some(&rent_sysvar), - false, - clock_epoch_3, + None, ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2545,8 +2540,7 @@ mod tests { ), &signers, Some(&rent_sysvar), - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2578,8 +2572,7 @@ mod tests { ), &signers, Some(&rent_sysvar), - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Err(InstructionError::InsufficientFunds)); } @@ -2603,8 +2596,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, Some(&rent_sysvar), - false, - &Clock::default(), + Some(&Clock::default()), ); assert_eq!(res, Ok(())); assert_eq!( @@ -2632,8 +2624,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, rent_sysvar, - false, - clock_epoch_3, + None, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); @@ -2663,8 +2654,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, rent_sysvar, - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); @@ -2693,8 +2683,7 @@ mod tests { &KeyedAccount::new(&solana_sdk::pubkey::new_rand(), false, &to_account), &signers, rent_sysvar, - true, - clock_epoch_3, + Some(clock_epoch_3), ); assert_eq!(res, Err(InstructionError::ActiveVoteAccountClose)); assert_eq!(vote_account.borrow().lamports(), lamports); @@ -2734,8 +2723,7 @@ mod tests { withdrawer_keyed_account, &signers, None, - false, - &Clock::default(), + None, ); assert_eq!(res, Ok(())); assert_eq!(vote_account.borrow().lamports(), 0); From abae8f687076022b6199e116184a02e2964ced3f Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Fri, 28 Jan 2022 10:05:45 -0600 Subject: [PATCH 15/18] Cleanup. Remove commented code --- programs/vote/src/vote_state/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 949127f4401c04..5f914e07e3de35 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1470,7 +1470,6 @@ mod tests { vote_state.epoch_credits = Vec::new(); - // let mut epoch = 0; let mut current_epoch_credits = 0; let mut previous_epoch_credits = 0; for (epoch, credits) in credits_to_append.iter().enumerate() { @@ -1480,7 +1479,6 @@ mod tests { current_epoch_credits, previous_epoch_credits, )); - // epoch += 1; previous_epoch_credits = current_epoch_credits; } From ffea66da6860ebac8f6b8b3032a2ec3cdc15fe11 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Fri, 28 Jan 2022 13:43:48 -0600 Subject: [PATCH 16/18] Review suggestion: Update error message Co-authored-by: Tyera Eulberg --- sdk/program/src/instruction.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index b62f4d4b68aa25..587ecee5ffcb5c 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -254,7 +254,7 @@ pub enum InstructionError { AccountsDataBudgetExceeded, /// Active vote account close - #[error("Cannot close vote account unless it received 0 credits in the most recent epoch")] + #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added From 6c9295ec5c22d614d77c2166aaf53e8f8aa692dd Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Fri, 28 Jan 2022 13:43:58 -0600 Subject: [PATCH 17/18] Review suggestion: Update error message Co-authored-by: Tyera Eulberg --- sdk/program/src/program_error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/program/src/program_error.rs b/sdk/program/src/program_error.rs index c4eac10be99807..0f8e02dab1cf82 100644 --- a/sdk/program/src/program_error.rs +++ b/sdk/program/src/program_error.rs @@ -51,7 +51,7 @@ pub enum ProgramError { IllegalOwner, #[error("Requested account data allocation exceeded the accounts data budget")] AccountsDataBudgetExceeded, - #[error("Cannot close vote account unless it received 0 credits in the most recent epoch")] + #[error("Cannot close vote account unless it stopped voting at least one full epoch ago")] ActiveVoteAccountClose, } From 88b95e39d7066a854e07cbf3eb141aa3976b76d7 Mon Sep 17 00:00:00 2001 From: Will Hickey Date: Wed, 2 Feb 2022 11:06:54 -0600 Subject: [PATCH 18/18] Review suggestion: logic simplification. Co-authored-by: Trent Nelson --- programs/vote/src/vote_state/mod.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 5f914e07e3de35..ceaebe326992e8 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1164,21 +1164,16 @@ pub fn withdraw( .ok_or(InstructionError::InsufficientFunds)?; if remaining_balance == 0 { - let reject_active_vote_account_close = match clock { - None => false, - Some(clock) => { - if vote_state.epoch_credits.is_empty() { - false - } else { - let (last_epoch_with_credits, _, _) = vote_state.epoch_credits.last().unwrap(); - let current_epoch = clock.epoch; - // if current_epoch - last_epoch_with_credits < 2 then the validator has received credits - // either in the current epoch or the previous epoch. If it's >= 2 then it has been at least - // one full epoch since the validator has received credits. - current_epoch.saturating_sub(*last_epoch_with_credits) < 2 - } - } - }; + let reject_active_vote_account_close = clock + .zip(vote_state.epoch_credits.last()) + .map(|(clock, (last_epoch_with_credits, _, _))| { + let current_epoch = clock.epoch; + // if current_epoch - last_epoch_with_credits < 2 then the validator has received credits + // either in the current epoch or the previous epoch. If it's >= 2 then it has been at least + // one full epoch since the validator has received credits. + current_epoch.saturating_sub(*last_epoch_with_credits) < 2 + }) + .unwrap_or(false); if reject_active_vote_account_close { return Err(InstructionError::ActiveVoteAccountClose);