namada
namada copied to clipboard
MASP nullifier uniqueness
Describe your changes
Addresses #1373.
Implements a nullifier set to prevent double spending in masp transactions. Updates the masp VP to check that the transactions writes the correct nullifiers and that these haven't already been revealed.
NOTE: testing this with an integration or e2e test is impossible cause we cannot force the submission of the tx if the builder fails to construct the shielded Transaction
. We'd need to test this with a unit test (which I think would be the correct way regardless of the force
issue) but at the moment we don't have the tools to write unit tests for the masp vp (at least to my knowledge)
Indicate on which release or other PRs this topic is based on
v0.27.0
Checklist before merging to draft
- [x] I have added a changelog
- [x] Git history is in acceptable state
Do you know why the unit and integration tests are failing in the CI? They seem to be timing out for some reason... Do they pass on your machine? Have you tried regenerating the MASP test fixtures maybe?
Both unit and integration tests pass on my machine without the need to rebuild the proofs
Nice. Could you please just check out the
serialize_to_vec
issue in the comments to see if there is anything to it? And if you get the chance (but it's not strictly necessary), could you please try seeing if movinghandle_masp_tx
fromtx_prelude/src/token.rs
intocore/src/ledger/masp_utils.rs
is trivial or not?
Ok, I've moved the entire handle_masp_tx
to the masp_utils
file. This has also solved the double serialization issue (which I believe like you it was a bug). To be able to share this function also with the IBC context I had to remove the call to insert_verifier(masp_addr)
from the function (otherwise I would have needed to implement TxEnv
on the IBC context), which shouldn't be a problem because the tx writes some masp keys anyway so those should already trigger the vp.
I had to keep a wrapper inside the IbcStorageContext
trait cause otherwise we would have needed to implement StorageRead
for IbcActions
which I think might be more work