Murisi/track undated assets

[murisi]

Describe your changes

This PR attempts to address issue #4304 by distributing rewards only among the dated assets. Doing this involved tracking the balance of dated/undated assets in the MASP at any point in time. More specifically, the following changes have been made:

• A new storage key has been introduced to track the amount of undated tokens of each token address in the MASP
• The transfer WASMs have been modified to update the storage key that tracks undated token amounts
• The MASP VP has been modified to check that the undated token tracking storage keys are updated correctly
• Rewards are now only distributed to the total undated assets of some token rather than to all assets of some token
  • Hence undated tokens that may have been locked into the MASP pool prior to enabling the rewards program for the token are no longer "rewarded"

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:

[grarco]

Net of the failing tests and some minor things this PR looks good to me

[batconjurer]

this could just be an impl Iterator<Item=&Address>

[murisi]

Thanks. The motivation for this change was to try and avoid doing the same precomputation twice (with the redundant querying and encoding that comes with it) if the input contains duplicates, not that this is actually a serious problem.

[batconjurer]

Just some nits

[sug0]

I have left a few suggestions, nothing too annoying to fix, I hope. Overall, this is a fine solution. The only thing I would point out is that it partitions the privacy set into dated and undated assets. After a shielding transaction is applied, you can tell whether the shielded asset was dated or not, based on the keys changed in storage. Whether this is relevant information or not isn't really clear to me.

EDIT: I guess this doesn't matter, because we could already tell before this PR if the asset was dated or not, given its presence or absence in the token map...

[sug0]

I believe some of the MASP integration tests had one or two cases where we were in fact testing the undated/dated rewards mechanism. Could you please quickly do a pass over all the tests in this file (crates/tests/src/integration/masp.rs) to check whether they still make sense, given the changes from this PR?

It seems like you just updated the balances in the failing tests :p

[sug0]

@murisi sorry to insist on this, but could you please check if the tests still make sense?

[murisi]

Could you please quickly do a pass over all the tests in this file (crates/tests/src/integration/masp.rs) to check whether they still make sense, given the changes from this PR?

@sug0 Thanks for highlighting that the integration tests might somehow be invalidated by these tracking changes, much appreciated. The logic of the tests still makes sense to me: everything about undated tokens never receiving rewards and dated tokens receiving rewards only after the rewards rate is raised above zero remains valid. The only exception to this is unrelated to the dated/undated concerns and can be found at

namada/crates/tests/src/integration/masp.rs

Line 3761 in 3558ccf

// Start distributing shielded rewards for NAM in next epoch

. I do not believe that the shielded rewards program can handle rewards for a token being disabled and then being re-enabled in the future. This is because allowed conversions are essentially telescoping series, and a disable followed by an enable interrupts the telescope.

It seems like you just updated the balances in the failing tests :p

I was a little surprised that the expected balances in dynamic_assets had to be raised in a disproportionate manner to make the tests pass. But this is plausible in a hypothetical situation where inflation = 10, total_tokens_in_masp = 6, and rewardable_tokens_in_masp = 5. If, as before, all dated and undated assets were being rewarded, then each note would get a reward of floor(10/6)=1, which would imply an inflation of 6*floor(10/6)=6. Whereas if only dated tokens are being rewarded, then each note would get a reward of floor(10/5)=2, which would imply an inflation of 5*floor(10/5)=10 which is higher than before.

[grarco]

I have left a few suggestions, nothing too annoying to fix, I hope. Overall, this is a fine solution. The only thing I would point out is that it partitions the privacy set into dated and undated assets. After a shielding transaction is applied, you can tell whether the shielded asset was dated or not, based on the keys changed in storage. Whether this is relevant information or not isn't really clear to me.

EDIT: I guess this doesn't matter, because we could already tell before this PR if the asset was dated or not, given its presence or absence in the token map...

I think it doesn't really matter since that information is taken from the sapling_value_balance which is already publicly available and displays the asset types and the amounts. That value is only populated with assets coming in or out of the pool: the assets and amounts that are instead circulating inside the pool won't show up there and therefore won't leak any data

[codecov]

Codecov Report

Attention: Patch coverage is 72.84768% with 82 lines in your changes missing coverage. Please review.

Project coverage is 74.25%. Comparing base (4990b9c) to head (ef4471d).
Report is 36 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/token/src/tx.rs	8.62%	53 Missing ⚠️
wasm/tx_ibc/src/lib.rs	0.00%	13 Missing ⚠️
crates/tx_prelude/src/token.rs	0.00%	8 Missing ⚠️
crates/shielded_token/src/vp.rs	92.59%	6 Missing ⚠️
crates/ibc/src/msg.rs	95.65%	1 Missing ⚠️
crates/shielded_token/src/masp/shielded_wallet.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4303      +/-   ##
==========================================
+ Coverage   74.17%   74.25%   +0.07%     
==========================================
  Files         345      343       -2     
  Lines      110624   110909     +285     
==========================================
+ Hits        82060    82360     +300     
+ Misses      28564    28549      -15     

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