feat(iota-framework): storage fund's ability earn staking rewards #959

Dkwcs · 2024-07-02T17:54:59Z

Description of change

Removed storage_fund_reinvest_rate and leftover_staking_rewards effect on storage_fund.
Storage fund doesn't earn from leftover or reinvest rewards.

Links to any relevant issues

fixes #937 .

Type of change

Enhancement (a non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

How the change has been tested

Run tests - kinesis/crates/iota-framework/packages/iota-system

…lity-earn-staking-rewards

PhilippGackstatter · 2024-07-03T09:24:24Z

crates/iota-framework/packages/iota-system/sources/iota_system_state_inner.move

@@ -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,
-                storage_fund_reinvestment: storage_fund_reinvestment_amount as u64,


Needs to also be removed in iota_types::event::SystemEpochInfoEvent.

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

PhilippGackstatter · 2024-07-03T09:26:39Z

crates/iota-framework/packages/iota-system/sources/iota_system.move

-        storage_fund_reinvest_rate: u64, // share of storage fund's rewards that's reinvested
-                                         // into storage fund, in basis point.
        reward_slashing_rate: u64, // how much rewards are slashed to punish a validator, in bps.


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).

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

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_rate from the Rust side because our main branch will be broken in this case.

Issue - #970

I think we should not merge this branch without removing storage_fund_reinvest_rate from the Rust side because our main branch will be broken in this case.

Exactly why I suggested these changes. Without this change and the SystemEpochInfoEvent update, 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_fund name and those should be updated to storage_deposit(s). So 970 should only be a renaming task, imho, not a logic change.

Okay, I'll fix storage_fund_reinvest_rate in Rust code

Check it out please

…f github.com:iotaledger/kinesis into scl1/rid-storage-fund's-ability-earn-staking-rewards

valeriyr · 2024-07-03T10:04:38Z

crates/iota-framework/packages/iota-system/sources/iota_system_state_inner.move

@@ -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);


We need to clarify what to do with the leftovers and make a final implementation or add a TODO.

In progress

This line fails to compile now since IotaTreasuryCap must be used differently than TreasuryCap<IOTA>.

valeriyr · 2024-07-03T10:06:03Z

crates/iota-framework/packages/iota-system/sources/storage_fund.move

-        non_refundable_storage_fee_amount: u64,
+        //TODO: try the way to configure
+        _non_refundable_storage_fee_amount: u64,


should not be removed?

Reverted those changes.

…nt usage changes. (review comments fix)

…einvest_rate variable

…lity-earn-staking-rewards

PhilippGackstatter

Looks good to me now (after you deal with the IotaTreasuryCap changes). Move tests all pass, iota-test-validator compiles and runs and can successfully advance epochs 👍 .

lzpap · 2024-07-04T12:01:12Z

crates/iota-framework/packages/iota-system/sources/iota_system_state_inner.move

@@ -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;


@Dkwcs This is the line where the storage fund is earning rewards, this and the following shall be removed.

So we have to completely remove the storage_fund_rewards from validators.advance_epoch or replace it with something?

In SUI the storage fund earns rewards itself from the stake susibidies, i.e. it is treated as stake. It goes back to the validators as additional reward.

In IOTA the storage deposit does not earn staking rewards, it is not considered staked. It doesn't generate rewards. So answering your question:

So we have to completely remove the storage_fund_rewards from validators.advance_epoch or replace it with something?

Yes, remove it completely.

feat(iota-framework): removing storage_fund ability to earn

2809508

Dkwcs requested review from valeriyr and PhilippGackstatter July 2, 2024 17:54

Dkwcs requested review from kodemartin, miker83z and a team as code owners July 2, 2024 17:55

Dkwcs added the sc-platform Issues related to the Smart Contract Platform group. label Jul 2, 2024

Dkwcs self-assigned this Jul 2, 2024

Dkwcs changed the base branch from sc-platform/issue-153 to sc-platform/issue-946-retain-iota-treasury-cap July 3, 2024 08:20

Base automatically changed from sc-platform/issue-946-retain-iota-treasury-cap to sc-platform/issue-153 July 3, 2024 08:25

Dkwcs added 2 commits July 3, 2024 12:19

Merge branch 'sc-platform/issue-153' into scl1/rid-storage-fund's-abi…

70dfa51

…lity-earn-staking-rewards

feat(iota-framework): removing storage_fund ability to earn

7abeb42

PhilippGackstatter reviewed Jul 3, 2024

View reviewed changes

Merge branch 'scl1/rid-storage-fund's-ability-earn-staking-rewards' o…

c9a118c

…f github.com:iotaledger/kinesis into scl1/rid-storage-fund's-ability-earn-staking-rewards

valeriyr reviewed Jul 3, 2024

View reviewed changes

refactor(iota-framework): revert fund non_refundable_storage_fee_amou…

b2fa6ca

…nt usage changes. (review comments fix)

PhilippGackstatter mentioned this pull request Jul 3, 2024

feat(sc-platform): Introduce validator target reward with minting and burning #971

Merged

Dkwcs added 3 commits July 3, 2024 16:08

Docs minor changes

19162d0

refactor(iota-protocol-config, iota-execution): remove storage_fund_r…

08a4efd

…einvest_rate variable

Merge branch 'sc-platform/issue-153' into scl1/rid-storage-fund's-abi…

8ce8447

…lity-earn-staking-rewards

PhilippGackstatter approved these changes Jul 4, 2024

View reviewed changes

Dkwcs added 2 commits July 4, 2024 11:09

refactor(iota-framework): iota_treasury_cap wrapper usage

e09aa98

feat(iota-protocol-config): updating protocol config snapshots

66c9573

lzpap reviewed Jul 4, 2024

View reviewed changes

lzpap mentioned this pull request Jul 4, 2024

[Task (L1SC)]: Staking reward generation through inflation #153

Closed

fefactor(iota-framework): minor comments have been added. Test refactor

98e1ab7

Dkwcs marked this pull request as draft July 4, 2024 15:01

Dkwcs closed this Jul 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iota-framework): storage fund's ability earn staking rewards #959

feat(iota-framework): storage fund's ability earn staking rewards #959

Dkwcs commented Jul 2, 2024 •

edited

Loading

PhilippGackstatter Jul 3, 2024

Dkwcs Jul 3, 2024

PhilippGackstatter Jul 3, 2024

Dkwcs Jul 3, 2024

valeriyr Jul 3, 2024

Dkwcs Jul 3, 2024 •

edited

Loading

PhilippGackstatter Jul 3, 2024

Dkwcs Jul 3, 2024

Dkwcs Jul 3, 2024

valeriyr Jul 3, 2024

Dkwcs Jul 3, 2024

PhilippGackstatter Jul 4, 2024

Dkwcs Jul 4, 2024

valeriyr Jul 3, 2024

Dkwcs Jul 3, 2024 •

edited

Loading

PhilippGackstatter left a comment

lzpap Jul 4, 2024

Dkwcs Jul 4, 2024

lzpap Jul 4, 2024

		@@ -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;

feat(iota-framework): storage fund's ability earn staking rewards #959

feat(iota-framework): storage fund's ability earn staking rewards #959

Conversation

Dkwcs commented Jul 2, 2024 • edited Loading

Description of change

Links to any relevant issues

Type of change

How the change has been tested

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dkwcs Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dkwcs Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dkwcs commented Jul 2, 2024 •

edited

Loading

Dkwcs Jul 3, 2024 •

edited

Loading

Dkwcs Jul 3, 2024 •

edited

Loading