diff --git a/Makefile b/Makefile index 61596b9a80..db90778235 100644 --- a/Makefile +++ b/Makefile @@ -152,7 +152,7 @@ lint-smart-contracts: .PHONY: audit-rs audit-rs: - $(CARGO) audit --ignore RUSTSEC-2024-0344 --ignore RUSTSEC-2024-0367 + $(CARGO) audit --ignore RUSTSEC-2024-0344 --ignore RUSTSEC-2024-0367 --ignore RUSTSEC-2024-0371 .PHONY: audit-as audit-as: diff --git a/utils/global-state-update-gen/src/generic.rs b/utils/global-state-update-gen/src/generic.rs index 2ac3363966..dd2fa76056 100644 --- a/utils/global-state-update-gen/src/generic.rs +++ b/utils/global-state-update-gen/src/generic.rs @@ -131,6 +131,22 @@ fn update_auction_state( let validators_diff = validators_diff(&old_snapshot, &new_snapshot); + let bids = state.get_bids(); + if slash_instead_of_unbonding { + // zero the unbonds for the removed validators independently of set_bid; set_bid will take + // care of zeroing the delegators if necessary + for bid_kind in bids { + if validators_diff + .removed + .contains(&bid_kind.validator_public_key()) + { + if let Some(bonding_purse) = bid_kind.bonding_purse() { + state.remove_withdraws_and_unbonds_with_bonding_purse(&bonding_purse); + } + } + } + } + add_and_remove_bids( state, &validators_diff, @@ -139,10 +155,6 @@ fn update_auction_state( slash_instead_of_unbonding, ); - if slash_instead_of_unbonding { - state.remove_withdraws_and_unbonds(&validators_diff.removed); - } - // We need to output the validators for the next era, which are contained in the first entry // in the snapshot. Some( diff --git a/utils/global-state-update-gen/src/generic/state_tracker.rs b/utils/global-state-update-gen/src/generic/state_tracker.rs index 18000a48b8..c2de88a199 100644 --- a/utils/global-state-update-gen/src/generic/state_tracker.rs +++ b/utils/global-state-update-gen/src/generic/state_tracker.rs @@ -9,7 +9,10 @@ use rand::Rng; use casper_types::{ account::AccountHash, addressable_entity::{ActionThresholds, AssociatedKeys, MessageTopics, Weight}, - system::auction::{BidAddr, BidKind, BidsExt, SeigniorageRecipientsSnapshot, UnbondingPurse}, + system::auction::{ + BidAddr, BidKind, BidsExt, SeigniorageRecipientsSnapshot, UnbondingPurse, UnbondingPurses, + WithdrawPurse, WithdrawPurses, + }, AccessRights, AddressableEntity, AddressableEntityHash, ByteCodeHash, CLValue, EntityKind, EntityVersions, Groups, Key, Package, PackageHash, PackageStatus, ProtocolVersion, PublicKey, StoredValue, URef, U512, @@ -24,6 +27,7 @@ pub struct StateTracker { total_supply: U512, total_supply_key: Key, accounts_cache: BTreeMap, + withdraws_cache: BTreeMap>, unbonds_cache: BTreeMap>, purses_cache: BTreeMap, staking: Option>, @@ -47,6 +51,7 @@ impl StateTracker { total_supply_key, total_supply: total_supply.into_t().expect("should be U512"), accounts_cache: BTreeMap::new(), + withdraws_cache: BTreeMap::new(), unbonds_cache: BTreeMap::new(), purses_cache: BTreeMap::new(), staking: None, @@ -340,29 +345,31 @@ impl StateTracker { return; } + let bids = self.get_bids(); + let maybe_existing_bid = self.existing_bid(&bid_kind, bids); + // since we skip bridge records optional values should be present let new_stake = bid_kind.staked_amount().expect("should have staked amount"); let bonding_purse = bid_kind.bonding_purse().expect("should have bonding purse"); - let bids = self.get_bids(); - - let maybe_existing_bid = self.existing_bid(&bid_kind, bids); let previous_stake = match maybe_existing_bid { None => U512::zero(), Some(existing_bid) => { - //let previous_stake = self.get_purse_balance(existing_bid_kind.bonding_purse()); - let previous_stake = existing_bid - .staked_amount() - .expect("should have staked amount"); + let previously_bonded = + self.get_purse_balance(existing_bid.bonding_purse().unwrap()); if existing_bid .bonding_purse() .expect("should have bonding purse") != bonding_purse { self.set_purse_balance(existing_bid.bonding_purse().unwrap(), U512::zero()); - self.set_purse_balance(bonding_purse, previous_stake); + self.set_purse_balance(bonding_purse, previously_bonded); + // the old bonding purse gets zeroed - the unbonds will get invalid, anyway + self.remove_withdraws_and_unbonds_with_bonding_purse( + &existing_bid.bonding_purse().unwrap(), + ); } - previous_stake + previously_bonded } }; @@ -372,19 +379,28 @@ impl StateTracker { // Replace the bid (overwrite the previous bid, if any): self.write_bid(bid_kind.clone()); + let unbonder_key = match bid_kind.delegator_public_key() { + None => bid_kind.validator_public_key(), + Some(delegator_public_key) => delegator_public_key, + }; + + // Remove all the relevant unbonds if we're slashing + if slash_instead_of_unbonding { + self.remove_withdraws_and_unbonds_with_bonding_purse(&bonding_purse); + } + + // This will be zero if the unbonds got removed above. + let already_unbonded = + self.already_unbonding_amount(&bid_kind.validator_public_key(), &unbonder_key); + + // This is the amount that should be in the bonding purse. + let new_stake = new_stake + already_unbonded; + if (slash_instead_of_unbonding && new_stake != previous_stake) || new_stake > previous_stake { self.set_purse_balance(bonding_purse, new_stake); } else if new_stake < previous_stake { - let unbonder_key = match bid_kind.delegator_public_key() { - None => bid_kind.validator_public_key(), - Some(delegator_public_key) => delegator_public_key, - }; - - let already_unbonded = - self.already_unbonding_amount(&bid_kind.validator_public_key(), &unbonder_key); - - let amount = previous_stake - new_stake - already_unbonded; + let amount = previous_stake - new_stake; self.create_unbonding_purse( bonding_purse, &bid_kind.validator_public_key(), @@ -394,13 +410,42 @@ impl StateTracker { } } + fn get_withdraws(&mut self) -> WithdrawPurses { + let mut result = self.reader.get_withdraws(); + for (acc, purses) in &self.withdraws_cache { + result.insert(*acc, purses.clone()); + } + result + } + + fn get_unbonds(&mut self) -> UnbondingPurses { + let mut result = self.reader.get_unbonds(); + for (acc, purses) in &self.unbonds_cache { + result.insert(*acc, purses.clone()); + } + result + } + + fn write_withdraw(&mut self, account_hash: AccountHash, withdraws: Vec) { + self.withdraws_cache.insert(account_hash, withdraws.clone()); + self.write_entry( + Key::Withdraw(account_hash), + StoredValue::Withdraw(withdraws), + ); + } + + fn write_unbond(&mut self, account_hash: AccountHash, unbonds: Vec) { + self.unbonds_cache.insert(account_hash, unbonds.clone()); + self.write_entry(Key::Unbond(account_hash), StoredValue::Unbonding(unbonds)); + } + /// Returns the sum of already unbonding purses for the given validator account & unbonder. fn already_unbonding_amount( &mut self, validator_public_key: &PublicKey, unbonder_public_key: &PublicKey, ) -> U512 { - let unbonds = self.reader.get_unbonds(); + let unbonds = self.get_unbonds(); let account_hash = AccountHash::from(validator_public_key); if let Some(purses) = unbonds.get(&account_hash) { if let Some(purse) = purses @@ -411,7 +456,7 @@ impl StateTracker { } } - let withdrawals = self.reader.get_withdraws(); + let withdrawals = self.get_withdraws(); if let Some(withdraws) = withdrawals.get(&account_hash) { if let Some(withdraw) = withdraws .iter() @@ -424,25 +469,23 @@ impl StateTracker { U512::zero() } - /// Generates the writes to the global state that will remove the pending withdraws and unbonds - /// of all the old validators that will cease to be validators, and slashes their unbonding - /// purses. - pub fn remove_withdraws_and_unbonds(&mut self, removed: &BTreeSet) { - let withdraws = self.reader.get_withdraws(); - let unbonds = self.reader.get_unbonds(); - for removed_validator in removed { - let acc = removed_validator.to_account_hash(); - if let Some(unbond_set) = unbonds.get(&acc) { - for unbond in unbond_set { - self.set_purse_balance(*unbond.bonding_purse(), U512::zero()); - } - self.write_entry(Key::Unbond(acc), StoredValue::Unbonding(vec![])); + pub fn remove_withdraws_and_unbonds_with_bonding_purse(&mut self, affected_purse: &URef) { + let withdraws = self.get_withdraws(); + let unbonds = self.get_unbonds(); + + for (acc, mut purses) in withdraws { + let old_len = purses.len(); + purses.retain(|purse| purse.bonding_purse().addr() != affected_purse.addr()); + if purses.len() != old_len { + self.write_withdraw(acc, purses); } - if let Some(withdraw_set) = withdraws.get(&acc) { - for withdraw in withdraw_set { - self.set_purse_balance(*withdraw.bonding_purse(), U512::zero()); - } - self.write_entry(Key::Withdraw(acc), StoredValue::Withdraw(vec![])); + } + + for (acc, mut purses) in unbonds { + let old_len = purses.len(); + purses.retain(|purse| purse.bonding_purse().addr() != affected_purse.addr()); + if purses.len() != old_len { + self.write_unbond(acc, purses); } } } diff --git a/utils/global-state-update-gen/src/generic/testing.rs b/utils/global-state-update-gen/src/generic/testing.rs index e099c93286..cd9a1de5a1 100644 --- a/utils/global-state-update-gen/src/generic/testing.rs +++ b/utils/global-state-update-gen/src/generic/testing.rs @@ -246,7 +246,7 @@ impl MockStateReader { None, ); - let account_hash = unbonding_purse.validator_public_key().to_account_hash(); + let account_hash = unbonding_purse.unbonder_public_key().to_account_hash(); match self.unbonds.get_mut(&account_hash) { None => { self.unbonds.insert(account_hash, vec![unbonding_purse]); @@ -1703,7 +1703,7 @@ fn should_slash_a_validator_and_delegator_with_enqueued_withdraws() { ) .with_withdraw( validator2.clone(), - past_delegator2, + past_delegator2.clone(), era_id, amount, &mut rng, @@ -1735,23 +1735,22 @@ fn should_slash_a_validator_and_delegator_with_enqueued_withdraws() { .expect("should have validator2"); update.assert_written_balance(*old_bid2.bonding_purse(), 0); - let delegator2 = old_bids2 + let delegator2_record = old_bids2 .delegator_by_public_keys(&validator2, &delegator2) .expect("should have delegator 2"); // check delegator2 slashed - update.assert_written_balance(*delegator2.bonding_purse(), 0); - // check past_delegator2 slashed - update.assert_written_balance( - *reader - .withdraws - .get(&validator2.to_account_hash()) - .expect("should have withdraws for validator2") - .last() - .expect("should have withdraw purses") - .bonding_purse(), - 0, - ); + update.assert_written_balance(*delegator2_record.bonding_purse(), 0); + // check past_delegator2 untouched + let past_delegator2_bid_purse = reader + .withdraws + .get(&validator2.to_account_hash()) + .expect("should have withdraws for validator2") + .iter() + .find(|withdraw| withdraw.unbonder_public_key() == &past_delegator2) + .expect("should have withdraw purses") + .bonding_purse(); + update.assert_key_absent(&Key::Balance(past_delegator2_bid_purse.addr())); // check validator1 and its delegators not slashed for withdraw in reader @@ -1762,8 +1761,8 @@ fn should_slash_a_validator_and_delegator_with_enqueued_withdraws() { update.assert_key_absent(&Key::Balance(withdraw.bonding_purse().addr())); } - // check the withdraws under validator 2 are cleared - update.assert_withdraws_empty(&validator2); + // check the withdraws under validator 2 still contain the past delegator's withdraw + update.assert_withdraw_purse(*past_delegator2_bid_purse, &validator2, &past_delegator2, 1); // check the withdraws under validator 1 are unchanged update.assert_key_absent(&Key::Withdraw(validator1.to_account_hash())); @@ -1771,8 +1770,8 @@ fn should_slash_a_validator_and_delegator_with_enqueued_withdraws() { // 8 keys should be written: // - seigniorage recipients // - total supply - // - 3 balances, 2 bids, 1 withdraw - assert_eq!(update.len(), 8); + // - 2 balances, 2 bids, 1 withdraw + assert_eq!(update.len(), 7); } #[test] @@ -1829,10 +1828,15 @@ fn should_slash_a_validator_and_delegator_with_enqueued_unbonds() { v1_stake.into(), &mut rng, ) - .with_unbond(validator1.clone(), delegator1, d1_stake.into(), &mut rng) .with_unbond( validator1.clone(), - past_delegator1, + delegator1.clone(), + d1_stake.into(), + &mut rng, + ) + .with_unbond( + validator1.clone(), + past_delegator1.clone(), pd1_stake.into(), &mut rng, ) @@ -1850,7 +1854,7 @@ fn should_slash_a_validator_and_delegator_with_enqueued_unbonds() { ) .with_unbond( validator2.clone(), - past_delegator2, + past_delegator2.clone(), pd2_stake.into(), &mut rng, ); @@ -1890,17 +1894,16 @@ fn should_slash_a_validator_and_delegator_with_enqueued_unbonds() { // check delegator2 slashed update.assert_written_balance(*delegator.bonding_purse(), 0); - // check past_delegator2 slashed - update.assert_written_balance( - *reader - .unbonds - .get(&validator2.to_account_hash()) - .expect("should have unbonds for validator2") - .last() - .expect("should have unbond purses") - .bonding_purse(), - 0, - ); + // check past_delegator2 untouched + let past_delegator2_bid_purse = reader + .unbonds + .get(&past_delegator2.to_account_hash()) + .expect("should have unbonds for validator2") + .iter() + .find(|unbond| unbond.validator_public_key() == &validator2) + .expect("should have unbonding purses") + .bonding_purse(); + update.assert_key_absent(&Key::Balance(past_delegator2_bid_purse.addr())); // check validator1 and its delegators not slashed for unbond in reader @@ -1911,17 +1914,16 @@ fn should_slash_a_validator_and_delegator_with_enqueued_unbonds() { update.assert_key_absent(&Key::Balance(unbond.bonding_purse().addr())); } - // check the unbonds under validator 2 are cleared - update.assert_unbonds_empty(&validator2); - - // check the withdraws under validator 1 are unchanged + // check the unbonds under validator 1 are unchanged update.assert_key_absent(&Key::Unbond(validator1.to_account_hash())); + update.assert_key_absent(&Key::Unbond(delegator1.to_account_hash())); + update.assert_key_absent(&Key::Unbond(past_delegator1.to_account_hash())); // 9 keys should be written: // - seigniorage recipients // - total supply - // - 4 balances, 2 bids, - // - 1 unbond + // - 3 balances, 2 bids, + // - 2 unbonds assert_eq!(update.len(), 9); } @@ -2128,6 +2130,11 @@ fn should_handle_unbonding_to_a_delegator_correctly() { .find(|&purse| purse.unbonder_public_key() == &old_validator) .map(|purse| *purse.bonding_purse()) .expect("A bonding purse for the validator"); + let unbonding_purses = reader + .get_unbonds() + .get(&delegator.to_account_hash()) + .cloned() + .expect("should have unbond purses"); let _ = unbonding_purses .iter() .find(|&purse| purse.unbonder_public_key() == &delegator) diff --git a/utils/global-state-update-gen/src/generic/update.rs b/utils/global-state-update-gen/src/generic/update.rs index 5a4331e69c..6ef080fc28 100644 --- a/utils/global-state-update-gen/src/generic/update.rs +++ b/utils/global-state-update-gen/src/generic/update.rs @@ -187,6 +187,29 @@ impl Update { ); } + #[track_caller] + pub(crate) fn assert_withdraw_purse( + &self, + bid_purse: URef, + validator_key: &PublicKey, + unbonder_key: &PublicKey, + amount: u64, + ) { + let account_hash = validator_key.to_account_hash(); + let withdraws = self + .entries + .get(&Key::Withdraw(account_hash)) + .expect("should have withdraws for the account") + .as_withdraw() + .expect("should be withdraw purses"); + assert!(withdraws.iter().any( + |withdraw_purse| withdraw_purse.bonding_purse() == &bid_purse + && withdraw_purse.validator_public_key() == validator_key + && withdraw_purse.unbonder_public_key() == unbonder_key + && withdraw_purse.amount() == &U512::from(amount) + )) + } + #[track_caller] pub(crate) fn assert_unbonding_purse( &self, @@ -233,7 +256,7 @@ impl Update { .iter() .map(|unbonding_purse| { ( - unbonding_purse.validator_public_key().to_account_hash(), + unbonding_purse.unbonder_public_key().to_account_hash(), *unbonding_purse.bonding_purse(), unbonding_purse.unbonder_public_key(), *unbonding_purse.amount(), @@ -266,26 +289,4 @@ impl Update { pub(crate) fn assert_validators_unchanged(&self) { assert!(self.validators.is_none()); } - - #[track_caller] - pub(crate) fn assert_withdraws_empty(&self, validator_key: &PublicKey) { - let withdraws = self - .entries - .get(&Key::Withdraw(validator_key.to_account_hash())) - .expect("should have withdraw purses") - .as_withdraw() - .expect("should be vec of withdraws"); - assert!(withdraws.is_empty()); - } - - #[track_caller] - pub(crate) fn assert_unbonds_empty(&self, validator_key: &PublicKey) { - let unbonds = self - .entries - .get(&Key::Unbond(validator_key.to_account_hash())) - .expect("should have unbond purses") - .as_unbonding() - .expect("should be vec of unbonds"); - assert!(unbonds.is_empty()); - } }