Remove duplicate user command fields #884
Conversation
There was a problem hiding this comment.
This looks very thorough, thanks @nholland94!
Unit tests seem to fail because the js_of_ocaml build artifact doesn't reflect these changes yet. Mina-signer doesn't depend on OCaml by itself, but the tests that check mina-signer for correctness do.
Which brings up the question: Is there a Mina branch this corresponds to? This probably won't be compatible with the current testnet, so we shouldn't merge this PR to main. I'll switch the base branch to berkeley, which is where we land upcoming changes that correspond to Mina's berkeley branch
EDIT: looks like we haven't updated berkeley in a while, so this includes a bunch of changes from main now. This is fine, I reviewed this PR and I think it only needs updated bindings before it's good to go
…y lines from stack trace and improve readability
…ning '/builtin/' instead of just '/builtin/+stdlib.js'
Clean up previous proof arguments handling in Pickles/SnarkyJS
…ck trace path to improve readability of error messages
…olocation of responsibilities
…Stacktrace keyword covers both class and method decorators
…newTransaction function
… stacktrace in case of error
…o catch and prettify stacktraces
Improve stack traces thrown from OCaml
nholland94
force-pushed
the
remove-duplicate-user-command-fields
branch
3 times, most recently
from
31dcf69
to
835c6fc
Compare
As part of MinaProtocol/mina#13048, a bug was discovered with how the signed command transaction logic would handle inputs where duplicate fields must be equal (this bug only existed in the berkeley version of the code, not what is released to mainnet). As part of fixing this, I opted to remove the duplicate fields to ensure that we aren't missing checks anywhere. The duplicate fields only existed to support user command tokens before mainnet, a feature which was cut from scope for mainnet, and has now been superseded by the zkapps token design.
This PR propagates these changes into SnarkyJS.