remove another potential abort in staking_pool (#7571)

This showed up when stress unit testing delegation code with random
amounts, random operations, etc. The rewards pool balance is 1 less than
the rewards we are trying to withdraw, possibly due to rounding. The
exact reason why this happened needs to be investigated. This PR removes
the possibility of aborting at `balance::split` when withdrawing rewards
from rewards pool.

UPDATE: switching the order in which we process new delegations and
delegation withdraws seems to fix the issue. It's still not super clear
whether the original order was the only trigger of the abort though.

crates/sui-cost/tests/snapshots/empirical_transaction_cost__good_snapshot-2.snap

@@ -4,13 +4,13 @@ expression: common_costs_estimate
 ---
 {
   "MergeCoin": {
-    "computation_cost": 7824,
-    "storage_cost": 11264,
+    "computation_cost": 7827,
+    "storage_cost": 11269,
     "storage_rebate": 0
   },
   "Publish": {
-    "computation_cost": 8683,
-    "storage_cost": 12467,
+    "computation_cost": 8687,
+    "storage_cost": 12472,
     "storage_rebate": 0
   },
   "SharedCounterAssertValue": {
@@ -29,8 +29,8 @@ expression: common_costs_estimate
     "storage_rebate": 0
   },
   "SplitCoin": {
-    "computation_cost": 7802,
-    "storage_cost": 11231,
+    "computation_cost": 7806,
+    "storage_cost": 11237,
     "storage_rebate": 0
   },
   "TransferPortionSuiCoin": {

crates/sui-framework/docs/staking_pool.md

@@ -48,6 +48,7 @@
 <b>use</b> <a href="epoch_time_lock.md#0x2_epoch_time_lock">0x2::epoch_time_lock</a>;
 <b>use</b> <a href="linked_table.md#0x2_linked_table">0x2::linked_table</a>;
 <b>use</b> <a href="locked_coin.md#0x2_locked_coin">0x2::locked_coin</a>;
+<b>use</b> <a href="math.md#0x2_math">0x2::math</a>;
 <b>use</b> <a href="object.md#0x2_object">0x2::object</a>;
 <b>use</b> <a href="sui.md#0x2_sui">0x2::sui</a>;
 <b>use</b> <a href="table_vec.md#0x2_table_vec">0x2::table_vec</a>;
@@ -960,6 +961,10 @@ time in <code>request_withdraw_stake</code>.
         <b>if</b> (total_sui_withdraw_amount &gt;= principal_withdraw_amount)
             total_sui_withdraw_amount - principal_withdraw_amount
         <b>else</b> 0;
+    // This may happen when we are withdrawing everything from the pool and
+    // the rewards pool <a href="balance.md#0x2_balance">balance</a> may be less than reward_withdraw_amount.
+    // TODO: FIGURE OUT EXACTLY WHY THIS CAN HAPPEN.
+    reward_withdraw_amount = <a href="math.md#0x2_math_min">math::min</a>(reward_withdraw_amount, <a href="balance.md#0x2_balance_value">balance::value</a>(&pool.rewards_pool));
     <a href="balance.md#0x2_balance_decrease_supply">balance::decrease_supply</a>(
         &<b>mut</b> pool.delegation_token_supply,
         withdrawn_pool_tokens

crates/sui-framework/docs/validator.md

@@ -732,10 +732,11 @@ Process pending delegations and withdraws, called at the end of the epoch.
 
 
 <pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="validator.md#0x2_validator_process_pending_delegations_and_withdraws">process_pending_delegations_and_withdraws</a>(self: &<b>mut</b> <a href="validator.md#0x2_validator_Validator">Validator</a>, ctx: &<b>mut</b> TxContext) {
-    <a href="staking_pool.md#0x2_staking_pool_process_pending_delegations">staking_pool::process_pending_delegations</a>(&<b>mut</b> self.delegation_staking_pool, ctx);
     <b>let</b> reward_withdraw_amount = <a href="staking_pool.md#0x2_staking_pool_process_pending_delegation_withdraws">staking_pool::process_pending_delegation_withdraws</a>(
         &<b>mut</b> self.delegation_staking_pool, ctx);
     self.metadata.next_epoch_delegation = self.metadata.next_epoch_delegation - reward_withdraw_amount;
+    <a href="staking_pool.md#0x2_staking_pool_process_pending_delegations">staking_pool::process_pending_delegations</a>(&<b>mut</b> self.delegation_staking_pool, ctx);
+    // TODO: consider bringing this <b>assert</b> back when we are more confident.
     // <b>assert</b>!(<a href="validator.md#0x2_validator_delegate_amount">delegate_amount</a>(self) == self.metadata.next_epoch_delegation, 0);
 }
 </code></pre>

crates/sui-framework/sources/governance/staking_pool.move

@@ -14,6 +14,7 @@ module sui::staking_pool {
     use std::vector;
     use sui::table_vec::{Self, TableVec};
     use sui::linked_table::{Self, LinkedTable};
+    use sui::math;
 
     friend sui::validator;
     friend sui::validator_set;
@@ -352,6 +353,10 @@ module sui::staking_pool {
             if (total_sui_withdraw_amount >= principal_withdraw_amount)
                 total_sui_withdraw_amount - principal_withdraw_amount
             else 0;
+        // This may happen when we are withdrawing everything from the pool and
+        // the rewards pool balance may be less than reward_withdraw_amount.
+        // TODO: FIGURE OUT EXACTLY WHY THIS CAN HAPPEN.
+        reward_withdraw_amount = math::min(reward_withdraw_amount, balance::value(&pool.rewards_pool));
         balance::decrease_supply(
             &mut pool.delegation_token_supply, 
             withdrawn_pool_tokens

crates/sui-framework/sources/governance/validator.move

@@ -284,10 +284,11 @@ module sui::validator {
 
     /// Process pending delegations and withdraws, called at the end of the epoch.
     public(friend) fun process_pending_delegations_and_withdraws(self: &mut Validator, ctx: &mut TxContext) {
-        staking_pool::process_pending_delegations(&mut self.delegation_staking_pool, ctx);
         let reward_withdraw_amount = staking_pool::process_pending_delegation_withdraws(
             &mut self.delegation_staking_pool, ctx);
         self.metadata.next_epoch_delegation = self.metadata.next_epoch_delegation - reward_withdraw_amount;
+        staking_pool::process_pending_delegations(&mut self.delegation_staking_pool, ctx);
+        // TODO: consider bringing this assert back when we are more confident.
         // assert!(delegate_amount(self) == self.metadata.next_epoch_delegation, 0);
     }