Merge pull request #4828 from fizyk20/fix-gsug-unbonds-2

Fix handling of unbonds in global-state-update-gen

utils/global-state-update-gen/src/generic.rs

@@ -120,6 +120,17 @@ fn update_auction_state<T: StateReader>(
 
     let validators_diff = validators_diff(&old_snapshot, &new_snapshot);
 
+    let bids = state.get_bids();
+    if slash {
+        // zero the unbonds for the removed validators independently of set_bid; set_bid will take
+        // care of zeroing the delegators if necessary
+        for pub_key in &validators_diff.removed {
+            if let Some(bid) = bids.get(pub_key) {
+                state.remove_withdraws_and_unbonds_with_bonding_purse(bid.bonding_purse());
+            }
+        }
+    }
+
     add_and_remove_bids(
         state,
         &validators_diff,
@@ -128,10 +139,6 @@ fn update_auction_state<T: StateReader>(
         slash,
     );
 
-    if slash {
-        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(

utils/global-state-update-gen/src/generic/state_reader.rs

@@ -2,7 +2,10 @@ use casper_engine_test_support::LmdbWasmTestBuilder;
 use casper_types::{
     account::{Account, AccountHash},
     system::{
-        auction::{Bids, UnbondingPurses, WithdrawPurses, SEIGNIORAGE_RECIPIENTS_SNAPSHOT_KEY},
+        auction::{
+            Bids, UnbondingPurses, WithdrawPurses, SEIGNIORAGE_RECIPIENTS_SNAPSHOT_KEY,
+            UNBONDING_DELAY_KEY,
+        },
         mint::TOTAL_SUPPLY_KEY,
     },
     Key, StoredValue,
@@ -22,6 +25,8 @@ pub trait StateReader {
     fn get_withdraws(&mut self) -> WithdrawPurses;
 
     fn get_unbonds(&mut self) -> UnbondingPurses;
+
+    fn get_unbonding_delay(&mut self) -> u64;
 }
 
 impl<'a, T> StateReader for &'a mut T
@@ -55,6 +60,10 @@ where
     fn get_unbonds(&mut self) -> UnbondingPurses {
         T::get_unbonds(self)
     }
+
+    fn get_unbonding_delay(&mut self) -> u64 {
+        T::get_unbonding_delay(self)
+    }
 }
 
 impl StateReader for LmdbWasmTestBuilder {
@@ -95,4 +104,22 @@ impl StateReader for LmdbWasmTestBuilder {
     fn get_unbonds(&mut self) -> UnbondingPurses {
         LmdbWasmTestBuilder::get_unbonds(self)
     }
+
+    fn get_unbonding_delay(&mut self) -> u64 {
+        // Find the hash of the auction contract.
+        let auction_contract_hash = self.get_system_auction_hash();
+
+        let unbonding_delay_key = self
+            .get_contract(auction_contract_hash)
+            .expect("auction should exist")
+            .named_keys()[UNBONDING_DELAY_KEY];
+
+        self.query(unbonding_delay_key)
+            .expect("should query")
+            .as_cl_value()
+            .cloned()
+            .expect("should be cl value")
+            .into_t()
+            .expect("should be u64")
+    }
 }

utils/global-state-update-gen/src/generic/state_tracker.rs

@@ -1,14 +1,17 @@
 use std::{
     cmp::Ordering,
-    collections::{btree_map::Entry, BTreeMap, BTreeSet},
+    collections::{btree_map::Entry, BTreeMap},
     convert::TryFrom,
 };
 
 use rand::Rng;
 
 use casper_types::{
     account::{Account, AccountHash},
-    system::auction::{Bid, Bids, SeigniorageRecipientsSnapshot, UnbondingPurse},
+    system::auction::{
+        Bid, Bids, SeigniorageRecipientsSnapshot, UnbondingPurse, UnbondingPurses, WithdrawPurse,
+        WithdrawPurses,
+    },
     AccessRights, CLValue, Key, PublicKey, StoredValue, URef, U512,
 };
 
@@ -21,6 +24,7 @@ pub struct StateTracker<T> {
     total_supply: U512,
     total_supply_key: Key,
     accounts_cache: BTreeMap<AccountHash, Account>,
+    withdraws_cache: BTreeMap<AccountHash, Vec<WithdrawPurse>>,
     unbonds_cache: BTreeMap<AccountHash, Vec<UnbondingPurse>>,
     purses_cache: BTreeMap<URef, U512>,
     bids_cache: Option<Bids>,
@@ -46,6 +50,7 @@ impl<T: StateReader> StateTracker<T> {
             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(),
             bids_cache: None,
@@ -222,7 +227,12 @@ impl<T: StateReader> StateTracker<T> {
     pub fn set_bid(&mut self, public_key: PublicKey, bid: Bid, slash: bool) {
         let maybe_current_bid = self.get_bids().get(&public_key).cloned();
 
-        let new_amount = *bid.staked_amount();
+        let account_hash = public_key.to_account_hash();
+        let already_unbonded = self.already_unbonding_amount(&account_hash, &public_key);
+
+        // we need to put enough funds in the bonding purse to cover the staked amount and the
+        // amount that will be getting unbonded in the future
+        let new_amount = *bid.staked_amount() + already_unbonded;
         let old_amount = maybe_current_bid
             .as_ref()
             .map(|bid| self.get_purse_balance(*bid.bonding_purse()))
@@ -234,8 +244,6 @@ impl<T: StateReader> StateTracker<T> {
             .unwrap()
             .insert(public_key.clone(), bid.clone());
 
-        let account_hash = public_key.to_account_hash();
-
         // Replace the bid (overwrite the previous bid, if any):
         self.write_entry(Key::Bid(account_hash), bid.clone().into());
 
@@ -246,6 +254,8 @@ impl<T: StateReader> StateTracker<T> {
             if old_bid.bonding_purse() != bid.bonding_purse() {
                 self.set_purse_balance(*old_bid.bonding_purse(), U512::zero());
                 self.set_purse_balance(*bid.bonding_purse(), old_amount);
+                // the old bonding purse gets zeroed - the unbonds will get invalid, anyway
+                self.remove_withdraws_and_unbonds_with_bonding_purse(old_bid.bonding_purse());
             }
 
             for (delegator_pub_key, delegator) in old_bid
@@ -271,6 +281,8 @@ impl<T: StateReader> StateTracker<T> {
                         self.set_purse_balance(*new_delegator.bonding_purse(), old_amount);
                     }
                     self.set_purse_balance(*delegator.bonding_purse(), U512::zero());
+                    // the old bonding purse gets zeroed - remove the delegator's unbonds
+                    self.remove_withdraws_and_unbonds_with_bonding_purse(delegator.bonding_purse());
                 } else {
                     let amount = self.get_purse_balance(*delegator.bonding_purse());
                     let already_unbonding =
@@ -288,48 +300,78 @@ impl<T: StateReader> StateTracker<T> {
         if (slash && new_amount != old_amount) || new_amount > old_amount {
             self.set_purse_balance(*bid.bonding_purse(), new_amount);
         } else if new_amount < old_amount {
-            let already_unbonded = self.already_unbonding_amount(&account_hash, &public_key);
             self.create_unbonding_purse(
                 *bid.bonding_purse(),
                 &public_key,
                 &public_key,
-                old_amount - new_amount - already_unbonded,
+                old_amount - new_amount,
             );
         }
 
         for (delegator_public_key, delegator) in bid.delegators() {
             let old_amount = self.get_purse_balance(*delegator.bonding_purse());
-            let new_amount = *delegator.staked_amount();
+            let already_unbonded =
+                self.already_unbonding_amount(&account_hash, delegator_public_key);
+            let new_amount = *delegator.staked_amount() + already_unbonded;
             if (slash && new_amount != old_amount) || new_amount > old_amount {
-                self.set_purse_balance(*delegator.bonding_purse(), *delegator.staked_amount());
+                self.set_purse_balance(*delegator.bonding_purse(), new_amount);
             } else if new_amount < old_amount {
-                let already_unbonded = self.already_unbonding_amount(&account_hash, &public_key);
                 self.create_unbonding_purse(
                     *delegator.bonding_purse(),
                     &public_key,
                     delegator_public_key,
-                    old_amount - new_amount - already_unbonded,
+                    old_amount - new_amount,
                 );
             }
         }
     }
 
+    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<WithdrawPurse>) {
+        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<UnbondingPurse>) {
+        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, account: &AccountHash, unbonder: &PublicKey) -> U512 {
-        let existing_purses = self
-            .reader
-            .get_unbonds()
-            .get(account)
-            .cloned()
-            .unwrap_or_default();
+        let current_era = *self.read_snapshot().1.keys().next().unwrap();
+        let unbonding_delay = self.reader.get_unbonding_delay();
+        let limit_era = current_era.saturating_sub(unbonding_delay);
+
+        let existing_purses = self.get_unbonds().get(account).cloned().unwrap_or_default();
         let existing_purses_legacy = self
-            .reader
             .get_withdraws()
             .get(account)
             .cloned()
             .unwrap_or_default()
             .into_iter()
-            .map(UnbondingPurse::from);
+            .map(UnbondingPurse::from)
+            // There may be some leftover old legacy purses that haven't been purged - this is to
+            // make sure that we don't accidentally take them into account.
+            .filter(|purse| purse.era_of_creation() >= limit_era);
 
         existing_purses_legacy
             .chain(existing_purses)
@@ -338,26 +380,23 @@ impl<T: StateReader> StateTracker<T> {
             .sum()
     }
 
-    /// 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<PublicKey>) {
-        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(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![]));
+    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(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![]));
+        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);
             }
         }
     }