Merge casper-network#4932

4932: Fix handling zero bids in forced_undelegate r=EdHastingsCasperAssociation a=fizyk20 This PR reverts a change from casper-network#4802 left in by mistake (should have been reverted in casper-network#4827). It also makes sure that we always prune zero bids, even if the minimum delegation amount allowed by a validator is zero. Closes casper-network#4920 Co-authored-by: Bartłomiej Kamiński <[email protected]>
teonite · Oct 25, 2024 · a44f882 · a44f882
2 parents 43209a1 + 653add8
commit a44f882
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 17 deletions.
diff --git a/execution_engine_testing/tests/src/test/system_contracts/auction/bids.rs b/execution_engine_testing/tests/src/test/system_contracts/auction/bids.rs
@@ -823,8 +823,7 @@ fn should_forcibly_undelegate_after_setting_validator_limits() {
     builder.forced_undelegate(None, DEFAULT_PROTOCOL_VERSION, DEFAULT_BLOCK_TIME);
 
     let bids = builder.get_bids();
-    // The undelegation itself doesn't remove bids, only process_unbond does.
-    assert_eq!(bids.len(), 3);
+    assert_eq!(bids.len(), 2);
 
     assert!(builder.get_validator_weights(new_era + 1).is_none());
 

diff --git a/storage/src/system/auction.rs b/storage/src/system/auction.rs
@@ -443,11 +443,16 @@ pub trait Auction:
             for mut delegator in delegators {
                 let staked_amount = delegator.staked_amount();
                 if staked_amount.is_zero() {
-                    // A delegator who has unbonded - nothing to do here, this will be removed when
-                    // unbonding is processed.
-                    continue;
-                }
-                if staked_amount < minimum_delegation_amount
+                    // If the stake is zero, we don't need to unbond anything - we can just prune
+                    // the bid outright. Also, we don't want to keep zero bids even if the minimum
+                    // allowed amount is zero, as they only use up space for no reason.
+                    let delegator_bid_addr = BidAddr::new_from_public_keys(
+                        validator_public_key,
+                        Some(delegator.delegator_public_key()),
+                    );
+                    debug!("pruning delegator bid {}", delegator_bid_addr);
+                    self.prune_bid(delegator_bid_addr);
+                } else if staked_amount < minimum_delegation_amount
                     || staked_amount > maximum_delegation_amount
                 {
                     let amount = if staked_amount < minimum_delegation_amount {
@@ -477,16 +482,19 @@ pub trait Auction:
                         Some(&delegator_public_key),
                     );
 
-                    debug!(
-                        "forced undelegation for {} reducing {} by {} to {}",
-                        delegator_bid_addr, staked_amount, amount, updated_stake
-                    );
-
-                    // Keep the bid for now - it will get pruned when the unbonds are processed.
-                    self.write_bid(
-                        delegator_bid_addr.into(),
-                        BidKind::Delegator(Box::new(delegator)),
-                    )?;
+                    if updated_stake.is_zero() {
+                        debug!("pruning delegator bid {}", delegator_bid_addr);
+                        self.prune_bid(delegator_bid_addr);
+                    } else {
+                        debug!(
+                            "forced undelegation for {} reducing {} by {} to {}",
+                            delegator_bid_addr, staked_amount, amount, updated_stake
+                        );
+                        self.write_bid(
+                            delegator_bid_addr.into(),
+                            BidKind::Delegator(Box::new(delegator)),
+                        )?;
+                    }
                 }
             }
         }