only add non-empty rewards as delegation when processing delegation s…

…witches (MystenLabs#7581)

Right now when we process delegation switches at epoch changes, we add
the rewards portion of the stake as a delegation to the new validator
directly without checking whether the rewards are empty or not. When the
rewards are empty, it will cause the `request_add_delegation` code to
abort because `request_add_delegation` asserts that the new delegate
stake has balance > 0. This PR adds a check before we add the rewards as
new delegation, and a unit test for switching delegation when rewards ==
0.

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

@@ -4,13 +4,13 @@ expression: common_costs_estimate
 ---
 {
   "MergeCoin": {
-    "computation_cost": 7814,
-    "storage_cost": 11250,
+    "computation_cost": 7820,
+    "storage_cost": 11259,
     "storage_rebate": 0
   },
   "Publish": {
-    "computation_cost": 8674,
-    "storage_cost": 12453,
+    "computation_cost": 8680,
+    "storage_cost": 12462,
     "storage_rebate": 0
   },
   "SharedCounterAssertValue": {
@@ -29,8 +29,8 @@ expression: common_costs_estimate
     "storage_rebate": 0
   },
   "SplitCoin": {
-    "computation_cost": 7793,
-    "storage_cost": 11217,
+    "computation_cost": 7798,
+    "storage_cost": 11226,
     "storage_rebate": 0
   },
   "TransferPortionSuiCoin": {

crates/sui-framework/docs/validator_set.md

@@ -1409,13 +1409,18 @@ the <code>from</code> validator's pool, and deposits it into the <code><b>to</b>
         <b>while</b> (!<a href="_is_empty">vector::is_empty</a>(&rewards)) {
             <b>let</b> delegator = <a href="_pop_back">vector::pop_back</a>(&<b>mut</b> delegators);
             <b>let</b> new_stake = <a href="_pop_back">vector::pop_back</a>(&<b>mut</b> rewards);
-            <a href="validator.md#0x2_validator_request_add_delegation">validator::request_add_delegation</a>(
-                to_validator,
-                new_stake,
-                <a href="_none">option::none</a>(), // no time lock for rewards
-                delegator,
-                ctx
-            );
+            // Only add delegation when the reward is non-empty.
+            <b>if</b> (<a href="balance.md#0x2_balance_value">balance::value</a>(&new_stake) == 0) {
+                <a href="balance.md#0x2_balance_destroy_zero">balance::destroy_zero</a>(new_stake);
+            } <b>else</b> {
+                <a href="validator.md#0x2_validator_request_add_delegation">validator::request_add_delegation</a>(
+                    to_validator,
+                    new_stake,
+                    <a href="_none">option::none</a>(), // no time lock for rewards
+                    delegator,
+                    ctx
+                );
+            }
         };
         <a href="_destroy_empty">vector::destroy_empty</a>(rewards);
     };

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

@@ -604,13 +604,18 @@ module sui::validator_set {
             while (!vector::is_empty(&rewards)) {
                 let delegator = vector::pop_back(&mut delegators);
                 let new_stake = vector::pop_back(&mut rewards);
-                validator::request_add_delegation(
-                    to_validator,
-                    new_stake,
-                    option::none(), // no time lock for rewards
-                    delegator,
-                    ctx
-                );
+                // Only add delegation when the reward is non-empty.
+                if (balance::value(&new_stake) == 0) {
+                    balance::destroy_zero(new_stake);
+                } else {
+                    validator::request_add_delegation(
+                        to_validator,
+                        new_stake,
+                        option::none(), // no time lock for rewards
+                        delegator,
+                        ctx
+                    );
+                }
             };
             vector::destroy_empty(rewards);
         };

crates/sui-framework/tests/delegation_tests.move

@@ -151,6 +151,54 @@ module sui::delegation_tests {
         test_scenario::end(scenario_val);
     }
 
+    #[test]
+    fun test_switch_delegation_zero_rewards() {
+        let scenario_val = test_scenario::begin(VALIDATOR_ADDR_1);
+        let scenario = &mut scenario_val;
+        set_up_sui_system_state(scenario);
+
+        test_scenario::next_tx(scenario, DELEGATOR_ADDR_1);
+        {
+            let system_state = test_scenario::take_shared<SuiSystemState>(scenario);
+            let ctx = test_scenario::ctx(scenario);
+            // Create a delegation to VALIDATOR_ADDR_1.
+            sui_system::request_add_delegation(
+                &mut system_state, coin::mint_for_testing(50, ctx), VALIDATOR_ADDR_1, ctx);
+            test_scenario::return_shared(system_state);
+        };
+
+        // Advance the epoch with no rewards.
+        governance_test_utils::advance_epoch(scenario);
+
+        test_scenario::next_tx(scenario, DELEGATOR_ADDR_1);
+        { 
+            let delegation = test_scenario::take_from_sender<Delegation>(scenario);
+            let staked_sui = test_scenario::take_from_sender<StakedSui>(scenario);
+            let system_state = test_scenario::take_shared<SuiSystemState>(scenario);
+
+            let ctx = test_scenario::ctx(scenario);
+
+            // Switch stake from VALIDATOR_ADDR_1 to VALIDATOR_ADDR_2
+            sui_system::request_switch_delegation(
+                &mut system_state, delegation, staked_sui, VALIDATOR_ADDR_2, ctx);
+            test_scenario::return_shared(system_state);
+        };
+
+        governance_test_utils::advance_epoch(scenario);
+        test_scenario::next_tx(scenario, DELEGATOR_ADDR_1);
+        {
+            let staked_sui_ids = test_scenario::ids_for_sender<StakedSui>(scenario);
+            // the delegator got no rewards from the previous validator so she
+            // didn't get any additional StakedSui object.
+            assert!(vector::length(&staked_sui_ids) == 1, 0);
+            let staked_sui = test_scenario::take_from_sender_by_id(scenario, *vector::borrow(&staked_sui_ids, 0));
+            assert!(staking_pool::staked_sui_amount(&staked_sui) == 50, 0); 
+            assert!(staking_pool::validator_address(&staked_sui) == VALIDATOR_ADDR_2, 0); 
+            test_scenario::return_to_sender(scenario, staked_sui);
+        };
+        test_scenario::end(scenario_val);
+    }
+
     #[test]
     fun test_cancel_delegation_request() {
         let scenario_val = test_scenario::begin(VALIDATOR_ADDR_1);