Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Murisi+grarco/multi tx masp vp rebased2 #3264

Merged
merged 25 commits into from
May 21, 2024

Conversation

murisi
Copy link
Contributor

@murisi murisi commented May 17, 2024

Describe your changes

Tried to increase the readability, strength, and generality of the MASP validity predicate whilst decreasing its verbosity. Closes #3068 . The changes are as follows:

  • Transaction inputs and outputs that exceed pre-balances and post-balances respectively are no longer allowed.
  • Now use the Sapling value balance in order to verify that movement in and out of the shielded pool is correct
  • Now handle all AssetType decoding in one place before starting to check the transparent bundles
  • The VP should now support Transactions and unrelated Transfers touching the same addresses/tokens
  • Tried to remove the code that gave IBC minting transactions special treatment now that Sapling value balance is used
  • Reduced dependency on the multitoken VP by directly checking the correctness of MASP VP balance changes
  • Now check that all Transactions and the balancing transaction are sufficiently authorized; prevents malleability
  • Now allow changes to the MASP token map when authorized by governance.

What would still need to be handled/fixed if this code turns out to be relevant:

  • Add integration tests. Like checking that an unshielding Transaction to Bertha is embeddable inside a Transfer to Christel only if Bertha has signed the entire transaction (in addition to the usual MASP checks).

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

Namada 0.35.1
anoma/masp#83
heliaxdev/num-traits#1

Checklist before merging to draft

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

@murisi murisi force-pushed the murisi+grarco/multi-tx-masp-vp-rebased2 branch from a8034ea to 6bba813 Compare May 17, 2024 18:30
Namada 0.35.1 is a patch release that fixes a couple build issues with the last minor release.
@murisi murisi force-pushed the murisi+grarco/multi-tx-masp-vp-rebased2 branch from 6bba813 to 75b71b5 Compare May 17, 2024 18:45
Copy link

codecov bot commented May 17, 2024

Codecov Report

Attention: Patch coverage is 6.03715% with 607 lines in your changes are missing coverage. Please review.

Project coverage is 60.12%. Comparing base (4ed6229) to head (96218d8).

Files Patch % Lines
crates/namada/src/ledger/native_vp/masp.rs 0.00% 397 Missing ⚠️
crates/core/src/uint.rs 1.63% 120 Missing ⚠️
...ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs 0.00% 24 Missing ⚠️
crates/sdk/src/queries/shell.rs 0.00% 22 Missing ⚠️
crates/sdk/src/masp.rs 0.00% 16 Missing ⚠️
crates/trans_token/src/storage_key.rs 0.00% 14 Missing ⚠️
crates/core/src/token.rs 53.33% 7 Missing ⚠️
crates/shielded_token/src/storage_key.rs 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3264      +/-   ##
==========================================
- Coverage   60.24%   60.12%   -0.13%     
==========================================
  Files         303      302       -1     
  Lines       93191    93389     +198     
==========================================
+ Hits        56145    56147       +2     
- Misses      37046    37242     +196     

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

@murisi murisi marked this pull request as ready for review May 20, 2024 19:25
@murisi murisi force-pushed the murisi+grarco/multi-tx-masp-vp-rebased2 branch from c1115c9 to a892e00 Compare May 20, 2024 19:26
@murisi murisi mentioned this pull request May 20, 2024
2 tasks
@murisi murisi requested review from grarco and sug0 May 20, 2024 19:27
@murisi murisi force-pushed the murisi+grarco/multi-tx-masp-vp-rebased2 branch from a892e00 to 96218d8 Compare May 20, 2024 21:12
brentstone added a commit that referenced this pull request May 21, 2024
* origin/murisi+grarco/multi-tx-masp-vp-rebased2:
  Enable governance to change MASP token map.
  Moved SignedAmount into core and removed use of unchecked arithmetic.
  Use try_fold to build the ChangedBalances object.
  Created a structure for representing conversion leafs.
  Factored the accumulation code out of verify_sapling_balancing_value.
  Updated the WASM binaries.
  Extend SignedAmount to hold MASP value balance.
  Use a MASP crate that guarantees ValueSums.
  Make MASP VP accept IBC transactions.
  Ensure that all implied transfers have been authorized by the relevant party.
  Split validate_transparent_bundle into 3 functions.
  Now check that the diff between pre and post is the value balance.
  Now decode AssetTypes separately.
  Unifies `DeltaBalance` and `SignedAmount`
  Fix transparent bundle validation bug
  Fixes balances check in masp vp to allow non-masp transfers
  Fixes balance owner when token is minted
  Changelog #2690
  Refactors transparent bundle validation into a separate function. Removes unchecked operation.
  `valid_spend_descriptions_anchor` does not expect the presence of the sapling bundle anymore
  Refactors balances split
  Fixes unrecognized assets
  Refactors transparent bundle check to just compare maps
  Reworks masp vp to accept multiple transfers in a single tx
@tzemanovic tzemanovic merged commit 708af2a into main May 21, 2024
16 of 19 checks passed
@tzemanovic tzemanovic deleted the murisi+grarco/multi-tx-masp-vp-rebased2 branch May 21, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MASP VP should support governance changes
4 participants