From 7c62127b2a4cca7dea5785ad3bca1269904bec83 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 24 Oct 2024 14:58:24 -0700 Subject: [PATCH] ugly, but simple fix for stake subsidies --- .../docs/sui-system/stake_subsidy.md | 26 +++++++ .../docs/sui-system/sui_system_state_inner.md | 25 ++++--- .../sui-system/sources/stake_subsidy.move | 6 +- .../sui-system/sources/sui_system.move | 11 +++ .../sources/sui_system_state_inner.move | 30 +++++--- .../tests/rewards_distribution_tests.move | 66 ++++++++++++++++++ .../packages_compiled/sui-system | Bin 44238 -> 44434 bytes crates/sui-framework/published_api.txt | 3 + 8 files changed, 150 insertions(+), 17 deletions(-) diff --git a/crates/sui-framework/docs/sui-system/stake_subsidy.md b/crates/sui-framework/docs/sui-system/stake_subsidy.md index f66978db04aec..61fd4cefe7efe 100644 --- a/crates/sui-framework/docs/sui-system/stake_subsidy.md +++ b/crates/sui-framework/docs/sui-system/stake_subsidy.md @@ -9,6 +9,7 @@ title: Module `0x3::stake_subsidy` - [Function `create`](#0x3_stake_subsidy_create) - [Function `advance_epoch`](#0x3_stake_subsidy_advance_epoch) - [Function `current_epoch_subsidy_amount`](#0x3_stake_subsidy_current_epoch_subsidy_amount) +- [Function `get_distribution_counter`](#0x3_stake_subsidy_get_distribution_counter)
use 0x1::u64;
@@ -210,4 +211,29 @@ Returns the amount of stake subsidy to be added at the end of the current epoch.
 
 
 
+
+
+
+
+## Function `get_distribution_counter`
+
+Returns the number of distributions that have occurred.
+
+
+
public(friend) fun get_distribution_counter(self: &stake_subsidy::StakeSubsidy): u64
+
+ + + +
+Implementation + + +
public(package) fun get_distribution_counter(self: &StakeSubsidy): u64 {
+    self.distribution_counter
+}
+
+ + +
diff --git a/crates/sui-framework/docs/sui-system/sui_system_state_inner.md b/crates/sui-framework/docs/sui-system/sui_system_state_inner.md index f4357743de41d..54fba5c2d65d5 100644 --- a/crates/sui-framework/docs/sui-system/sui_system_state_inner.md +++ b/crates/sui-framework/docs/sui-system/sui_system_state_inner.md @@ -2163,18 +2163,27 @@ gas coins. let storage_charge = storage_reward.value(); let computation_charge = computation_reward.value(); - + let mut stake_subsidy = balance::zero(); // Include stake subsidy in the rewards given out to validators and stakers. // Delay distributing any stake subsidies until after `stake_subsidy_start_epoch`. // And if this epoch is shorter than the regular epoch duration, don't distribute any stake subsidy. - let stake_subsidy = - if (ctx.epoch() >= self.parameters.stake_subsidy_start_epoch && - epoch_start_timestamp_ms >= prev_epoch_start_timestamp + self.parameters.epoch_duration_ms) - { - self.stake_subsidy.advance_epoch() - } else { - balance::zero() + if (ctx.epoch() >= self.parameters.stake_subsidy_start_epoch && + epoch_start_timestamp_ms >= prev_epoch_start_timestamp + self.parameters.epoch_duration_ms) + { + // special case for epoch 560 -> 561 change bug. add extra subsidies for "safe mode" + // where reward distribution was skipped. use distribution counter and epoch check to + // avoiding affecting devnet and testnet + if (self.stake_subsidy.get_distribution_counter() == 540 && ctx.epoch() > 560) { + let first_safe_mode_epoch = 561; + let safe_mode_epoch_count = ctx.epoch() - first_safe_mode_epoch; + safe_mode_epoch_count.do!(|_| { + stake_subsidy.join(self.stake_subsidy.advance_epoch()); + }); + // done with catchup for safe mode epochs. distribution counter is now >540, we won't hit this again + // fall through to the normal logic, which will add subsidies for the current epoch }; + stake_subsidy.join(self.stake_subsidy.advance_epoch()); + }; let stake_subsidy_amount = stake_subsidy.value(); computation_reward.join(stake_subsidy); diff --git a/crates/sui-framework/packages/sui-system/sources/stake_subsidy.move b/crates/sui-framework/packages/sui-system/sources/stake_subsidy.move index d0760d97a88c0..5a91ebc7d54fd 100644 --- a/crates/sui-framework/packages/sui-system/sources/stake_subsidy.move +++ b/crates/sui-framework/packages/sui-system/sources/stake_subsidy.move @@ -83,9 +83,13 @@ module sui_system::stake_subsidy { self.current_distribution_amount.min(self.balance.value()) } - #[test_only] /// Returns the number of distributions that have occurred. public(package) fun get_distribution_counter(self: &StakeSubsidy): u64 { self.distribution_counter } + + #[test_only] + public(package) fun set_distribution_counter(self: &mut StakeSubsidy, distribution_counter: u64) { + self.distribution_counter = distribution_counter; + } } diff --git a/crates/sui-framework/packages/sui-system/sources/sui_system.move b/crates/sui-framework/packages/sui-system/sources/sui_system.move index 916c2cd55b33b..641cc3f9dd86e 100644 --- a/crates/sui-framework/packages/sui-system/sources/sui_system.move +++ b/crates/sui-framework/packages/sui-system/sources/sui_system.move @@ -726,6 +726,17 @@ module sui_system::sui_system { self.get_stake_subsidy_distribution_counter() } + #[test_only] + public fun set_stake_subsidy_distribution_counter(wrapper: &mut SuiSystemState, counter: u64) { + let self = load_system_state_mut(wrapper); + self.set_stake_subsidy_distribution_counter(counter) + } + + #[test_only] + public fun inner_mut_for_testing(wrapper: &mut SuiSystemState): &mut SuiSystemStateInnerV2 { + wrapper.load_system_state_mut() + } + // CAUTION: THIS CODE IS ONLY FOR TESTING AND THIS MACRO MUST NEVER EVER BE REMOVED. Creates a // candidate validator - bypassing the proof of possession check and other metadata validation // in the process. diff --git a/crates/sui-framework/packages/sui-system/sources/sui_system_state_inner.move b/crates/sui-framework/packages/sui-system/sources/sui_system_state_inner.move index 121a12fc75b94..b2137264c5505 100644 --- a/crates/sui-framework/packages/sui-system/sources/sui_system_state_inner.move +++ b/crates/sui-framework/packages/sui-system/sources/sui_system_state_inner.move @@ -865,18 +865,27 @@ module sui_system::sui_system_state_inner { let storage_charge = storage_reward.value(); let computation_charge = computation_reward.value(); - + let mut stake_subsidy = balance::zero(); // Include stake subsidy in the rewards given out to validators and stakers. // Delay distributing any stake subsidies until after `stake_subsidy_start_epoch`. // And if this epoch is shorter than the regular epoch duration, don't distribute any stake subsidy. - let stake_subsidy = - if (ctx.epoch() >= self.parameters.stake_subsidy_start_epoch && - epoch_start_timestamp_ms >= prev_epoch_start_timestamp + self.parameters.epoch_duration_ms) - { - self.stake_subsidy.advance_epoch() - } else { - balance::zero() + if (ctx.epoch() >= self.parameters.stake_subsidy_start_epoch && + epoch_start_timestamp_ms >= prev_epoch_start_timestamp + self.parameters.epoch_duration_ms) + { + // special case for epoch 560 -> 561 change bug. add extra subsidies for "safe mode" + // where reward distribution was skipped. use distribution counter and epoch check to + // avoiding affecting devnet and testnet + if (self.stake_subsidy.get_distribution_counter() == 540 && ctx.epoch() > 560) { + let first_safe_mode_epoch = 561; + let safe_mode_epoch_count = ctx.epoch() - first_safe_mode_epoch; + safe_mode_epoch_count.do!(|_| { + stake_subsidy.join(self.stake_subsidy.advance_epoch()); + }); + // done with catchup for safe mode epochs. distribution counter is now >540, we won't hit this again + // fall through to the normal logic, which will add subsidies for the current epoch }; + stake_subsidy.join(self.stake_subsidy.advance_epoch()); + }; let stake_subsidy_amount = stake_subsidy.value(); computation_reward.join(stake_subsidy); @@ -1127,6 +1136,11 @@ module sui_system::sui_system_state_inner { self.validators.request_add_validator(min_joining_stake_for_testing, ctx); } + #[test_only] + public(package) fun set_stake_subsidy_distribution_counter(self: &mut SuiSystemStateInnerV2, counter: u64) { + self.stake_subsidy.set_distribution_counter(counter) + } + // CAUTION: THIS CODE IS ONLY FOR TESTING AND THIS MACRO MUST NEVER EVER BE REMOVED. Creates a // candidate validator - bypassing the proof of possession check and other metadata validation // in the process. diff --git a/crates/sui-framework/packages/sui-system/tests/rewards_distribution_tests.move b/crates/sui-framework/packages/sui-system/tests/rewards_distribution_tests.move index 7c04d28e61aca..2cb3c0a9fd634 100644 --- a/crates/sui-framework/packages/sui-system/tests/rewards_distribution_tests.move +++ b/crates/sui-framework/packages/sui-system/tests/rewards_distribution_tests.move @@ -3,6 +3,7 @@ #[test_only] module sui_system::rewards_distribution_tests { + use sui::balance; use sui::test_scenario::{Self, Scenario}; use sui_system::sui_system::SuiSystemState; use sui_system::validator_cap::UnverifiedValidatorOperationCap; @@ -491,4 +492,69 @@ module sui_system::rewards_distribution_tests { scenario.return_to_sender(cap); test_scenario::return_shared(system_state); } + + #[test] + fun test_stake_subsidy_with_safe_mode() { + use std::unit_test::assert_eq; + + set_up_sui_system_state_with_big_amounts(); + + let mut test = test_scenario::begin(VALIDATOR_ADDR_1); + let mut sui_system = test.take_shared(); + + let ctx = test.ctx(); + + // increment epoch number (safe mode emulation) + 562u64.do!(|_| ctx.increment_epoch_number()); + sui_system.set_epoch_for_testing(562); + sui_system.set_stake_subsidy_distribution_counter(540); + + assert!(ctx.epoch() == 562); + assert!(sui_system.get_stake_subsidy_distribution_counter() == 540); + + // perform advance epoch + sui_system + .inner_mut_for_testing() + .advance_epoch(563, 65, balance::zero(), balance::zero(), 0, 0, 0, 0, 100000000000, ctx) + .destroy_for_testing(); // balance returned from `advance_epoch` + + assert_eq!(sui_system.get_stake_subsidy_distribution_counter(), 542); + + test_scenario::return_shared(sui_system); + test.end(); + } + + #[test] + // Test that the fix for the subsidy distribution doesn't affect testnet, + // where the distribution has no epoch delay, and the condition could result + // in arithmetic error. + fun test_stake_subsidy_with_safe_mode_testnet() { + use std::unit_test::assert_eq; + + set_up_sui_system_state_with_big_amounts(); + + let mut test = test_scenario::begin(VALIDATOR_ADDR_1); + let mut sui_system = test.take_shared(); + + let ctx = test.ctx(); + + // increment epoch number (safe mode emulation) + 540u64.do!(|_| ctx.increment_epoch_number()); + sui_system.set_epoch_for_testing(540); + sui_system.set_stake_subsidy_distribution_counter(540); + + assert!(ctx.epoch() == 540); + assert!(sui_system.get_stake_subsidy_distribution_counter() == 540); + + // perform advance epoch + sui_system + .inner_mut_for_testing() + .advance_epoch(541, 65, balance::zero(), balance::zero(), 0, 0, 0, 0, 100000000000, ctx) + .destroy_for_testing(); // balance returned from `advance_epoch` + + assert_eq!(sui_system.get_stake_subsidy_distribution_counter(), 541); + + test_scenario::return_shared(sui_system); + test.end(); + } } diff --git a/crates/sui-framework/packages_compiled/sui-system b/crates/sui-framework/packages_compiled/sui-system index 69ed46cf2174ca124ba738d1779a506a0d78fd1c..579bcc4606828af1704b8be4b13a3823653e74e6 100644 GIT binary patch delta 3033 zcmaKuS#T8B6^75bx4UPi@9pVnsb@4ZT4%Hjl3>sv4H_0>Hi?LRG5aPK0Rkj}5Qx=d zY&HQFfv!{~&SHoY$0UwJOm^ck}MB_#uP(hpzX z``>d;pVPPdbpPh}?9Lz9gO83b{ikuS>WMG_Kq5#cV?5oWte?{ zfq$gp2m3~L@9#e`GTguS1xCkIFRhWSAMARp`-^?KiF^M(e0$6a3TEgQ;K z=w-l=1BB+75CNBf1#mR)=Fst?#yG7Qnt%v%889k=E(jV}5QF!+OUlZz+%bL+-!!zx zfr9=t@Y_J(6Z?K`@Nd~(YViB(S zlY2g|`~0@g8$Q3|^L3wZVT=OZ^!Z(18TWbD=a|pR3QDFqo&Z>_a9SZ}5L(sfN0=lg z;*|goFj)og<08hZ0rJTjfKFvGTj4&|)&Z=lO##H?X@G3K4cK_J2LWF^vJ` zn{x?}`@L6&-Qod;V0ElqrB3r!80xfs<$5}W>sBvC9-d|_C(c_#F~SShDP{dyHSX^r z-LBkagWbL}0Z!!}8=P)?8aQXS)AT%{tb0})lr7s}K-rG@guWLD2h`{_e2I98=s?+J ze{#j=Rov_~p2!!~@>g*)@oT>RhR?4nmj4R7mw_A4q`^&QW`kSVYk*s|iI7_4fZLEs zfLrLC2ksn&xpN6GELjR}cViQ{Us9{94yM3eyVwSI1@8N}gp{B7{J>))jnTc!_s^|cwSPtP+PbdRIf+7I zacg(~!}%kRSuD$u`v>CWfOCy|?r7S}kIv#o(0+Je^fFKU&zJwhLX)%-O7DF!Q!2g_FkVIBV=3XAC?jQWwFy&Lz500zxo zpL_Wp-l`(<&pG0hWAFd>)Rt2nuwzcs*mEwQiRY0XB$;u$|YCX{vwezJktWWV)IsRHt)Z_V!$_FTRS* zP+G08jVx+Y4KSj)O=A73IT~3y+WhP4nXWV;q zBM}{^>rzI#Q_jk0L%LeR;0$|zy>SSKy$v@P7FFguSX9L;EvBEvo|g`?isz+6EdIQ7 zoFz!w81d6!&Kb!~PM0_=cvhsK}9H|4gMRhLR< zDAE9{DV0?4eXO=rQdUE(PD#bmqWf5iw2h9RJEz<=Q&aoq3<@o)Hjc8_Df_hYoY-fs zk|&G(tX{c3U-q$Q({E6YrB^&qWQ||scjQ^Wrsw_kunb9Cu5o$<3|BUpASGv^Q4!VNo>(gyxyi)}>HnYkt}-oz zoNlJg>GjGmjaCaK9(FcT1tLwK!HFX#)B0CR*TOB7b1T)>q6}x}Wv5x4mo*}r7d3Iw znUu3E&79Ryuw_on5`la?V2Pw`vvOve5@&k1Z&j!Ng!*4_@BkJv5eP z<-AXC)f7)sx!}<4YxGTOeUo-kCu%EXN_2?!3Yn*V0CsuQw>(WlH!5m)IlPl zQc0HU7pafZcKvrC%C$>ezrq8djL6)%y)1YWS(cvCecqbeDSX{Ke!I@TfvinE%YG+t zu6cKFcXVxK?NgF;wy_R1nffAJN0u+07uoD7TX0UWPTJ~2Gc4%p_Y!x;@YKV%?zATV E4ZH_~egFUf delta 2765 zcmaKu*>@Dj6^HMw>Sd;@x_f%2CC!Ys83}2$pg}?!G+={Sv;YDFHe%mb0TKwa2pjNV z$B7Y(ZPf8OG1+ip$Bq*lCvrABiywG&oRgE~Wc>?r@{l~_Ik#HqoRP>$KYVrTSNGn! zRa0F(Q-5Wj|CxRGtI@t+8*kS9Ll^)c5hRn9yrD@O)byeC=IeN$MIP`2##>sA^{M8| z-)kSCeciC!DMJGftC+z|=CjIxk2R&xkvcHfi~x_>4gyaK9l@~lU|{S&)5E82nl40isJSh=EVQB6vJ_@66c}XTYc!iU$pBsDLl`mlfE_I>-4vylZG5n??QK z=4Yn)OZT<<$mjV3#s`Z|S#Nay0_7jOeiOIf?#sKs>(k&OuEU=qt7Ubpo-Jf+*$_L= zF0toXiTxPaPY_PR7>vU^2;W6`AA@(sFPFZ9r~z!(X;*fdgl!SR4u#!O!f}ON7U7t} zRv`g~ZNeVM1<1xcfK73-o=VW@HB@}sC%k})dsW7-V+HZ6A-@*#?U3J4EWyiR=U&Lk zkZ*>(5%Sw1Z-x98R#KvyA>Rs>+abRh@_NYXDk`QmnF83TFrm<85ZaaX0j7zmWHrEj zOxFN>Si)p2Ku@|3V2Sc5D7=gH8GsG-S%74+0U+P#0ydpmhkz?i_93w4M~J+?BM(~h zLIXj2VpocIV&6jIk8qFbe1bbw@8@Vxqc0j=L40fl*+y=ifjqME1Ou8#bzD*^r@{EgPPq#>qU{humo~))SX)qLj|^ElSz4 zS=l4&NO!CEy5LnGAoRN};61xL1>R*synk^g#0Q=>e2DLU+<^Fz<%FZkdJ(q>=zTt<~SNl6+3* zYQevv;@-t$8SwApni}wbfNMyZ4*7Ag_E6pFyEMvI^FIDPUqp+DS9Mn%n+QKf}-Qv-}i)j$h@gINc_&pjv=scvob^iFgyiwMieh2d_{5vwfG0 z32;HggzLSCOe0@I4;A!YLZ+*JO5If6)!_XbjoBBF8A_`UwR5XF)dd#Q{AMlv(Cc3} zYD!-~W|CLYwN!32(YuY3tF#V}dd~?in74;T%3WBQOXQ);J{DDT88h`z%omYaN-;x) zdCv;;_A;TQ$c!YgNPX^I3tqihTe^nKo}1xI$eg*#Mdr>GN-U;?O0^z~d6zXqmjX~A znp*DRf`F4sHpOVjIhf2 z(lJ&wUpmf`^QCc?B561lR=~VVl3SdnI3ajmYJyfFH17gS&xVzNc2d=_pH=^_xKnfE zYG&hR!w$0Aa_JIf8fA6ml1hGv)t5`k>om(KsZ_q`BP>hWuou?OsdmlM1Z}N1CbYby zlStL;RDFYrocWEpDxN$JvqlvY6=_tMqDNL+rd8df`Bp?!H%XyaMx_uUc<@TEpR>8h6>TKnXi>^$TNBNJ z`)aXt9c6(sNpFX2x7tOaQg%DM=*TulBi2|}-W#NGjG6Q@oflEDAne(GWwT7^7o##E z653Tt>v+S`*Gol|zVKy>(?a2`G))W5D!L1@$EqpFTG3t*wMo&FmWyo7TGX?>=*qlU zB+Nq66lvLE=dF$&(iYMzu1}wynTJlj>sAOVpVTaUyYx>`9exW{SJ_R&pLgq{;*`V}HCvvyW!RJW6* zoDHN(x*HaXtfdRX+a{aBGNcMh_)@)*zAvjpqfCl6nGzj6GA(o|yJb*e)ZzJuP&!I7z+ y{vE7)R+8RM)}uPJ_w6LILV5prwq({9yk}T1m3X|vhW?S@?^Ba_VfuK`p8hXVgIX*A diff --git a/crates/sui-framework/published_api.txt b/crates/sui-framework/published_api.txt index e6d0acfbb736b..5b4e3a9ff2eb5 100644 --- a/crates/sui-framework/published_api.txt +++ b/crates/sui-framework/published_api.txt @@ -691,6 +691,9 @@ advance_epoch current_epoch_subsidy_amount public fun 0x3::stake_subsidy +get_distribution_counter + public(package) fun + 0x3::stake_subsidy SystemParameters public struct 0x3::sui_system_state_inner