feat(iota-framework): storage fund's ability earn staking rewards #959
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,7 +207,6 @@ module iota_system::iota_system_state_inner { | |
| protocol_version: u64, | ||
| reference_gas_price: u64, | ||
| total_stake: u64, | ||
| storage_charge: u64, | ||
| storage_rebate: u64, | ||
| storage_fund_balance: u64, | ||
|
|
@@ -831,8 +830,6 @@ module iota_system::iota_system_state_inner { | |
| mut computation_reward: Balance<IOTA>, | ||
| mut storage_rebate_amount: u64, | ||
| mut non_refundable_storage_fee_amount: u64, | ||
| reward_slashing_rate: u64, // how much rewards are slashed to punish a validator, in bps. | ||
| epoch_start_timestamp_ms: u64, // Timestamp of the epoch start | ||
| ctx: &mut TxContext, | ||
|
|
@@ -842,11 +839,7 @@ module iota_system::iota_system_state_inner { | |
|
|
||
| let bps_denominator_u64 = BASIS_POINT_DENOMINATOR as u64; | ||
| // Rates can't be higher than 100%. | ||
| assert!(reward_slashing_rate <= bps_denominator_u64, EBpsTooLarge); | ||
|
|
||
| // TODO: remove this in later upgrade. | ||
| if (self.parameters.stake_subsidy_start_epoch > 0) { | ||
|
|
@@ -890,11 +883,6 @@ module iota_system::iota_system_state_inner { | |
|
|
||
| let storage_fund_reward_amount = storage_fund_balance as u128 * computation_charge_u128 / total_stake_u128; | ||
|
There was a problem hiding this comment. @Dkwcs This is the line where the storage fund is earning rewards, this and the following shall be removed. There was a problem hiding this comment. So we have to completely remove the storage_fund_rewards from validators.advance_epoch or replace it with something? There was a problem hiding this comment. In SUI the In IOTA the
Yes, remove it completely. |
||
| let mut storage_fund_reward = computation_reward.split(storage_fund_reward_amount as u64); | ||
|
|
||
| self.epoch = self.epoch + 1; | ||
| // Sanity check to make sure we are advancing to the right epoch. | ||
|
|
@@ -931,12 +919,11 @@ module iota_system::iota_system_state_inner { | |
| let mut leftover_staking_rewards = storage_fund_reward; | ||
| leftover_staking_rewards.join(computation_reward); | ||
| let leftover_storage_fund_inflow = leftover_staking_rewards.value(); | ||
|
|
||
| self.iota_treasury_cap.supply_mut().decrease_supply(leftover_staking_rewards); | ||
|
There was a problem hiding this comment. We need to clarify what to do with the leftovers and make a final implementation or add a TODO. There was a problem hiding this comment. This line fails to compile now since |
||
| let refunded_storage_rebate = | ||
| self.storage_fund.advance_epoch( | ||
| storage_reward, | ||
| storage_rebate_amount, | ||
| non_refundable_storage_fee_amount, | ||
| ); | ||
|
|
@@ -948,7 +935,6 @@ module iota_system::iota_system_state_inner { | |
| reference_gas_price: self.reference_gas_price, | ||
| total_stake: new_total_stake, | ||
| storage_charge, | ||
|
There was a problem hiding this comment. Needs to also be removed in There was a problem hiding this comment. We'll create a separate issue for that |
||
| storage_rebate: storage_rebate_amount, | ||
| storage_fund_balance: self.storage_fund.total_balance(), | ||
| stake_subsidy_amount, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This param must also be removed in the call to advance_epoch in
iota_adapter_latest::execution_engine::checked::construct_advance_epoch_pt(as well as v0, v1, v2).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll create a separate issue for that
Because of removing of storage_fund_reinvest_rate looks dangerous in some places.
We'll do it separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think it is better to do this in a separate issue, please, add a subtask and implement it based on this branch.
I think we should not merge this branch without removing
storage_fund_reinvest_ratefrom the Rust side because our main branch will be broken in this case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue - #970
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly why I suggested these changes. Without this change and the
SystemEpochInfoEventupdate, the iota-test-validator does not work. The required changes are not huge, so we should do it in this branch, imo, otherwise we cannot merge this PR without breaking our issue 153 branch.Issue 970 deals with multiple things now. I think we simply should do the appropriate changes to the Rust types in this PR, and left 970 deal only with the naming changes. There are lots of variables and fields that contain the
storage_fundname and those should be updated tostorage_deposit(s). So 970 should only be a renaming task, imho, not a logic change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll fix
storage_fund_reinvest_ratein Rust codeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check it out please