snarkVM icon indicating copy to clipboard operation
snarkVM copied to clipboard

Authorization deserialization fix

Open lukenewman opened this issue 1 year ago • 1 comments

Motivation

When deserializing an Authorization from string a call stack that included multiple transitions, the structural assertion logic would erroneously fail. This PR fixes the assertion logic but does not change the underlying structure/order of the Authorization fields.

Test Plan

Our (Puzzle) delegated proving stack takes in user-generated Authorizations in string form. We then parse these and execute the proof request using snarkVM crates. I've tested that these Authorization deserializations and subsequent proofs do indeed land onchain properly.

I can provide screenshots/videos on request. For now, you can see the following transaction that was generated through our stack and has landed onchain: https://testnet.aleoscan.io/transaction?id=at14d4edujs6ztghev2zppp55v4xlffu7q3p379dlvdsu3yuu2cluysd84ktk

I am also willing to create a sample project showing this fix in action.

A more formal test plan would involve embedding a multi-transition Authorization unit test into snarkVM itself and not just relying on a fee_public test case.

Related PRs

none

lukenewman avatar Jun 06 '24 13:06 lukenewman

Great find!

embedding a multi-transition Authorization unit test

Yes please. You can see some recent examples of adjusted tests in ledger/src/tests.rs in: https://github.com/ProvableHQ/snarkVM/pull/13/files . There are also a lot in synthesizer/src/vm/mod.rs

vicsn avatar Jun 06 '24 21:06 vicsn

@vicsn this is good for another pass! added unit tests

lukenewman avatar Jan 31 '25 22:01 lukenewman

@niklaslong thanks for the feedback! it's been addressed.

lukenewman avatar Feb 19 '25 20:02 lukenewman

I've had a dig around and we seem to:

  1. order requests from parent to child: they are passed in when constructing the Authorization
  2. order transitions from child to parent, as inner calls are processed first and added one by one to the Authorization

This means the mismatch isn't due to deserialisation but rather the expectation that requests and transitions be ordered in the same way within an Authorization. In other words, the reordering proposed in this PR might not be sufficient, should there be more than two nested calls.

Perhaps we could either establish the correspondence between requests and transitions using the program id in the check or (more simply) rely on the collections being in reverse order relative to each other for the time being?

niklaslong avatar Mar 05 '25 13:03 niklaslong

alright @niklaslong @vicsn we're now matching based on the transition commitment

lukenewman avatar Apr 07 '25 21:04 lukenewman

@d0cd thanks for the review! addressed your feedback and also simplified the tcm_indices creation

lukenewman avatar Apr 24 '25 00:04 lukenewman