Update calculation for automated staking rewards

[joshuahannan]

Automated Staking Rewards

In preparation for enabling automated staking rewards on Flow mainnet, this PR adds a few lines to change how rewards are calculated. There was a batch of tokens from Flow Genesis that were minted as a necessary incentive structure for proper decentralization. These tokens are not circulating and were never meant to be included in the true total supply calculation. Because FLOW has hit its decentralization milestones, these tokens are meant to be burned. Most of them have been burnt and will be burnt in the future, but unfortunately, it has been difficult to access all of them at once to burn them, so there are still some remaining.

This change adds a section when calculating the reward amount for the next epoch that subtracts the current amount of bonus tokens remaining from the expected total supply so that the inflation amount is calculated from the expected supply not including bonus tokens instead of the total supply. This will result in less tokens being minted per epoch than there would be if the bonus tokens were included.

The process for updating this value will go through the service account committee. When bonus tokens are burnt, a proposal will be posted as a PR in the service account repo where the committee will review it and schedule a multisign to update the value. These changes will be very infrequent because we would prefer to burn large batches of bonus tokens instead of little amounts every time.

The initial value is expected to be something around 20M

Update Rewards Events

Rewards payment is now happening automatically in the system chunk transaction, so events will not be queryable in the same way as before. Any projects that rely on rewards events will need to update to be compatible. Instructions are here: https://forum.onflow.org/t/breaking-change-coming-to-flow-epoch-smart-contracts-for-automatic-rewards/4516We will make sure to broadcast this change very widely to all affected users if it is approved.

[jordanschalm]

Depending on how the bonus tokens are structured, I suggested a possible alternative which would avoid needing service account approval for each bonus token burn. Not sure if it is feasible though.

[bluesign]

Why don't we 'just' burn them ?

[joshuahannan]

@bluesign Most of the bonus tokens are controlled by accounts that require a multisig between the Flow team and a third party, so getting them back to burn them has been a little bit difficult. We're going to be able to get them all back eventually, hopefully this year, but we really want to enable automatic rewards in the meantime, so we need this workaround in the interim.

[bluesign]

@joshuahannan ah makes sense, thanks

[joshuahannan]

@jordanschalm We discussed the rewards event emissions and decided that we still wanted it to be in a different transaction than the move tokens transaction, so I moved it to happen during the staking auction phase only when the currentBlock.view == (currentEpochMetadata.startView + UInt64(1)). That way, rewards are paid in the block that is immediately after the start block of the epoch. It is in the latest commit.

Is this a safe way to do this? Is it possible for a view to get skipped or for a view to not change at all between blocks?
There is an additional check in the payRewardsForPreviousEpoch method that ensures that rewards can only be paid once even if the method is called multiple times, but it still felt good to be safe. I primarily want to make sure this view won't get skipped because then it would skip paying rewards.

[bluesign]

Is this a safe way to do this?

I don't think this works ( view can be skipped )

  pub fun main():UInt64 {
   var b1 =  getCurrentBlock()
   var dif1 = (b1.height - b1.view)

   var b2 = getBlock(at: b1.height-1440*60)!
   var dif2 = (b2.height - b2.view)

   return dif2 - dif1
  }

PS: considering comment on Heartbeat is correct ( and called in every block )

[jordanschalm]

Is this a safe way to do this? Is it possible for a view to get skipped or for a view to not change at all between blocks?

No it is not safe. @bluesign is right, views can be skipped.

[joshuahannan]

okay that makes sense. So do we think we're good with just having the staking auction part of advanceBlock() be:

                case EpochPhase.STAKINGAUCTION:
                    let currentBlock = getCurrentBlock()
                    let currentEpochMetadata = FlowEpoch.getEpochMetadata(FlowEpoch.currentEpochCounter)!
                    // Pay rewards only if automatic rewards are enabled
                    // payRewardsForPreviousEpoch will only pay rewards once because it sets a rewards paid flag
                    // the first time it pays and won't pay rewards any more for that epoch once the flag is set
                    // even though the method is called every time
                    if FlowEpoch.automaticRewardsEnabled() {
                        self.payRewardsForPreviousEpoch()
                    }

Added clarification in the comments there too since rewards can only be paid once anyway

[yagop]

Now, the delegator reward amounts are just emitted in the RewardsPaid event instead, which should significantly decrease the number of events emitted in this transaction.

Even though the number of event's will be much lower, the RewardsPaid.delegatorAmounts will be large too.

This will be a breaking change if any apps or delegators are relying on the DelegatorRewardsPaid events for their app, so they will need to update to be compatible. We will make sure to broadcast this change very widely to all affected users if it is approved.

This will cause problems as to process rewards events from previous blocks.

Wouldn't be better to address the large number of events by other way? There are many delegators receiving very small amounts like 0.00000006, probably because they can't pay for the transaction fee or something. It makes sense to limit the minimum amount to have staked as to receive rewards. Or the minimum amount to receive reward (the dust can go to the Node Operator).

Plus there are many events that they are A.1654653399040a61.FlowToken.TokensWithdrawn and A.1654653399040a61.FlowToken.TokensDeposited.

For example, in 84eca4ff612ef70047d60510710cca872c8a17c1bd9f63686e74852b6382cc84 :

curl -s https://rest-mainnet.onflow.org/v1/transaction_results/84eca4ff612ef70047d60510710cca872c8a17c1bd9f63686e74852b6382cc84 | jq '.events[].type' | sort |  uniq -c
16593 "A.1654653399040a61.FlowToken.TokensDeposited"
   1 "A.1654653399040a61.FlowToken.TokensMinted"
16593 "A.1654653399040a61.FlowToken.TokensWithdrawn"
16277 "A.8624b52f9ddcd04a.FlowIDTableStaking.DelegatorRewardsPaid"
   1 "A.8624b52f9ddcd04a.FlowIDTableStaking.EpochTotalRewardsPaid"
   1 "A.8624b52f9ddcd04a.FlowIDTableStaking.NewWeeklyPayout"
 314 "A.8624b52f9ddcd04a.FlowIDTableStaking.RewardsPaid"
   1 "A.f919ee77447b7497.FlowFees.FeesDeducted"
   1 "A.f919ee77447b7497.FlowFees.TokensWithdrawn"

From those 16593, 10152 are greater than 0.01 Flow:

curl -s https://rest-mainnet.onflow.org/v1/transaction_results/84eca4ff612ef70047d60510710cca872c8a17c1bd9f63686e74852b6382cc84 \
| jq '.events | map(select( .type == "A.1654653399040a61.FlowToken.TokensDeposited" )) | map(.payload | @base64d | fromjson) | map(select((.value.fields[0].value.value | tonumber) > 0.01)) | length'
10152

And 3909 greater than 1 Flow :

curl -s https://rest-mainnet.onflow.org/v1/transaction_results/84eca4ff612ef70047d60510710cca872c8a17c1bd9f63686e74852b6382cc84 \
| jq '.events | map(select( .type == "A.1654653399040a61.FlowToken.TokensDeposited" )) | map(.payload | @base64d | fromjson) | map(select((.value.fields[0].value.value | tonumber) > 1)) | length'
3909

[joshuahannan]

@yagop Thank you so much for the feedback! Because of your feedback, we decide that we're going to hold off on changing the delegator rewards events for now and try to find a better solution for the events problem. I'm going to create a FLIP for requiring a minimum stake for delegators to receive rewards and I'm also going to propose a cadence flag that allows the protocol contracts to turn off event emission for the unnecessary events.

We also might work to split up rewards payments into multiple transactions so they aren't as large.

I'll share the FLIPs here after I've proposed them.

Thanks again!

[yagop]

Glad to help, many thanks @joshuahannan ! 🙇