MASP Bug Fixes

[murisi]

Describe your changes

• Refactored the validation of shielded transaction transparent outputs:
  • Now avoid assuming that transparent outputs are ordered.
  • No longer assuming that there must be transparent outputs.
  • Now ensure that total of transparent output values match the containing transaction.
• Increased the validation of transparent inputs to be similar to that of transparent outputs.
• Redefined MaspAmount to be an instantiation of ValueSum
• Reduced usage of HashMaps in representing numbers
• Reduced the back-and-forth encoding and decoding of AssetTypes
• Nearly halved average MASP transaction sizes by eliminating redundant notes from two sources:
  • Zero-valued output notes emanating from parts of u256 integer being zero
  • The note inclusion algorithm including spending notes that are not needed for the transaction
• Allow the spending of assets from previous epochs in addition to the current one. Involved:
  • Adjusting the output description creation algorithm to select assets from more than one epoch
  • Modifying the balance printing algorithms to not ignore assets from older epochs
  • Adjusted the growth of the MASP pool's balance in order to be able to fully satisfy shielded withdrawals involving trace amounts
• Fixed the printing of a shielded balance for a specific token across multiple users
  • Before the balances for all tokens across all users was being printed out (which is more than requested)
• Unified the encode_asset_type and make_asset_type functions to make sure their encodings never diverge
  • Additionally, serialize_to_vec is now called on only a single token representation (inside encode_asset_type) to ensure that conceptually different tokens never map to the same AssetType
• Enabled shielded transaction validation when the asset type is not in the conversion tree

Indicate on which release or other PRs this topic is based on

Namada v0.29.0

Checklist before merging to draft

• [x] I have added a changelog
• [x] Git history is in acceptable state

[murisi]

What is reasoning behind allowing users to spend assets not in the current epoch?

Hi! The reasoning is as follows: MASP conversions are discrete in the sense that for every x tokens you hold of some asset, you get y tokens of some other asset. Unfortunately, this means that if your shielded balance is not a multiple of x, then balance % x cannot be upgraded to the latest epoch. In the past this meant that small parts of a user's balance could disappear after epoch changes because assets from older epochs were ignored - I believe that you observed this during your genesis flow work. This PR now handles older epoch assets so that parts of a balance do not disappear.

[batconjurer]

What is reasoning behind allowing users to spend assets not in the current epoch?

Hi! The reasoning is as follows: MASP conversions are discrete in the sense that for every x tokens you hold of some asset, you get y tokens of some other asset. Unfortunately, this means that if your shielded balance is not a multiple of x, then balance % x cannot be upgraded to the latest epoch. In the past this meant that small parts of a user's balance could disappear after epoch changes because assets from older epochs were ignored - I believe that you observed this during your genesis flow work. This PR now handles older epoch assets so that parts of a balance do not disappear.

Ah, I see. That's how we're fixing that issue.

[murisi]

Also, shouldn't we have such cases in the integration tests?

I believe the integration tests are already implicitly using notes from older epochs. But you're right, I will try to write a test to explicitly test for this behaviour. Thanks!