feat(iota-framework): storage fund's ability earn staking rewards #959
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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.burn_balance(leftover_staking_rewards, ctx); | ||
| 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, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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