Murisi/non compounding rewards

[murisi]

Describe your changes

Based on #4303 . Implemented a version of MASP rewards where rewards do not compound. More precisely, the NAM received for locking tokens in the MASP does not itself receive NAM rewards. This version was implemented as an alternative to the compounding version in case the rounding errors become significant. This PR was also made to demonstrate that non-compounding rewards are also possible, and hence leave this possibility on the table.

In order to achieve the goal of non-compounding rewards, the following changes were made:

• MASP rewards are denominated in NAMs that have no timestamps
• The reward deflation step that was used to denominate rewards as NAM in epoch 0 is only required when compounding, so this has been removed
• The special logic for rewarding locking NAM in the MASP is no longer required and is now covered by the same logic that rewards other tokens
• Note: the test failures are due to the fact that the rewards in the integration tests need to be recomputed for the new rewards algorithm

The overall outcome of these changes is that while the PD-controller is untouched and the MASP pool's transparent balance still grows at the same rate, the portion of rewards that would have gone to the reward NAM locked in the MASP has now been redistributed to the non-reward NAM locked in the MASP.

Additionally, this PR reduces the amount of dust created by the rewards algorithm - the issue raised at #4372. To see this, note that the dust left at the MASP address after all shielded tokens are unshielded in https://github.com/anoma/namada/pull/4285/files#diff-78668f42dcc983dea532ff370112f41796cfae99510ea10156df37c4e1e4e0d2R2287 has gone down from nam: 0.003216 to nam: 0.

Checklist before merging

• If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
  • This will require 2 reviewers to approve the changes
• If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
  • Relevant PR if applies:
• If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
  • Relevant PR if applies:

[codecov]

Codecov Report

Attention: Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.57%. Comparing base (6e33dcc) to head (6c5662c).
Report is 59 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/sdk/src/tx.rs	50.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4285      +/-   ##
==========================================
- Coverage   74.58%   74.57%   -0.02%     
==========================================
  Files         339      339              
  Lines      111054   111004      -50     
==========================================
- Hits        82834    82776      -58     
- Misses      28220    28228       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[grarco]

I'm very much in favor of this PR as it simplifies the shielded rewards logic but we'll need to wait for someone to review the economic outcomes of it

[grarco]

Actually, one small thing, I believe we need to review and update the shielded rewards estimate function:

namada/crates/shielded_token/src/masp/shielded_wallet.rs

Line 692 in 102adbb

async fn estimate_next_epoch_rewards(

We were handling the rewards with Epoch(0) and also accounting for the compound effect

[murisi]

I'm very much in favor of this PR as it simplifies the shielded rewards logic but we'll need to wait for someone to review the economic outcomes of it

Thanks for reviewing! Maybe @cwgoes can chime in here? I think the key economic change is as follows:

The overall outcome of these changes is that while the PD-controller is untouched and the MASP pool's transparent balance still grows at the same rate, the portion of rewards that would have gone to the reward NAM locked in the MASP has now been redistributed to the non-reward NAM locked in the MASP.

I guess one (maybe less desirable) implication of this change/PR is that users can still get compounding rewards, but only indirectly. This would be done by unshielding reward NAMs (which would be undated) as soon as you receive them, and then immediately reshielding them. These reshielded NAM are no longer reward NAM and are dated, so they would now receive rewards. Unfortunately, I guess this would mean that those willing to do this trick would accumulate more NAM than those who do not or cannot do this. More precisely, those who do this will have exponential growth in their NAM balance/ownership while those who do not will only have a linear growth in their NAM balance/ownership since they are only earning shielded rewards on the principal amount locked in the shielded pool. (I'm not accounting for the gas fees paid when unshielding and reshielding in this line of thought though.) I'm not sure if this is acceptable from an economics/incentives perspective?

Also, if this trick is profitable then it encourages lots of unshielding and reshielding transactions solely for the purposes of claiming rewards. Could this theoretically have a negative impact on other Namada activities or the privacy of those doing it? cc @brentstone

[brentstone]

@murisi I don't think unshielding and reshielding in order to take advantage of compounding rewards is an issue at all. Anyone is free to do this in proof of stake, for example. And I don't think the increased transaction volume is a downside at all (in fact, might be a net-positive).

[murisi]

Actually, one small thing, I believe we need to review and update the shielded rewards estimate function:

namada/crates/shielded_token/src/masp/shielded_wallet.rs

Line 692 in 102adbb

async fn estimate_next_epoch_rewards(

We were handling the rewards with Epoch(0) and also accounting for the compound effect

Good point, thanks! I'm not so familiar with the rewards estimation code and would need some time to go through it. @batconjurer is there a quick way to adjust the rewards estimate function for this PR so that it doesn't assume a compounding effect?

It's probably just a question of checking for undated NAM rather than NAM at epoch 0, but I'd have to refresh my memory. But if grarco is willing to help, I'll let him 😉

[grarco]

Actually, one small thing, I believe we need to review and update the shielded rewards estimate function:

namada/crates/shielded_token/src/masp/shielded_wallet.rs

Line 692 in 102adbb

async fn estimate_next_epoch_rewards(

We were handling the rewards with Epoch(0) and also accounting for the compound effect

Good point, thanks! I'm not so familiar with the rewards estimation code and would need some time to go through it. @batconjurer is there a quick way to adjust the rewards estimate function for this PR so that it doesn't assume a compounding effect?

@murisi I can help here but I'll wait to see if we want to move forward with this PR

[brentstone]

I'm going to close this, as I think we've decided to forgo this PR, still sound good @murisi? Here's a summary of the motivation for closing (@cwgoes):

• There are currently no actual plans to incentivize NAM, so this isn't really critical rn
• If the rewards from NAM are desired to be kept lower, this can be done just by limiting the max inflation rate with the current tools
• Allowing the NAM to autocompound actually might just prove to be an extra nice incentive to hold NAM, which is something we should want as a project
  • this is the same kind of thinking that motivates enforcing that gas fees in non-native tokens are more expensive than gas paid in NAM

Other things discussed and confirmed:

• though the current algo with compounding is more complex than what is in this PR, it has also been audited and tested on several testnets, whereas this new work is not audited or tested yet aside from PR review.
• even if non-compounding rewards are forced, a user can achieve the autocompounding effect by manually unshielding and reshielding the rewards (and has ~ 24 hours to do so in the current configuration as well)
• confirmed that unshielding and reshielding does not give more rewards than letting the NAM just sit and autocompound with the current implementations

[murisi]

I'm going to close this, as I think we've decided to forgo this PR, still sound good @murisi? Here's a summary of the motivation for closing (@cwgoes):

* There are currently no actual plans to incentivize NAM, so this isn't really critical rn

* If the rewards from NAM are desired to be kept lower, this can be done just by limiting the max inflation rate with the current tools

* Allowing the NAM to autocompound actually might just prove to be an extra nice incentive to hold NAM, which is something we should want as a project
  
  * this is the same kind of thinking that motivates enforcing that gas fees in non-native tokens are more expensive than gas paid in NAM

Other things discussed and confirmed:

* though the current algo with compounding is more complex than what is in this PR, it has also been audited and tested on several testnets, whereas this new work is not audited or tested yet aside from PR review.

* even if non-compounding rewards are forced, a user can achieve the autocompounding effect by manually unshielding and reshielding the rewards (and has ~ 24 hours to do so in the current configuration as well)

* confirmed that unshielding and reshielding does not give more rewards than letting the NAM just sit and autocompound with the current implementations

Agreed. As a minor clarification: the PR #4285 looks simpler because a lot of the logic it relies upon (like VP and transaction WASM changes, and introduction of storage keys) was factored out into #4303 (which was recently merged into main to resolve #4304 ). When initially working on shielded rewards, there were no undated shielded assets and I had not worked out the details of #4303 + #4285, and so the non-compounding option looked/seemed more complex.