Skip to content

General multi-request signing#1355

Open
Antonio95 wants to merge 12 commits into
add_nested_signing_testfrom
general_nested_signing_test
Open

General multi-request signing#1355
Antonio95 wants to merge 12 commits into
add_nested_signing_testfrom
general_nested_signing_test

Conversation

@Antonio95

@Antonio95 Antonio95 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

This PR adds an example test with the general external-signing flow for arbitrary transactions (that is, transactions with any call structure and record flows).

  • The test case is “Should match ExecutionRequest.sign for arbitrary flow with multiple requests” in sdk/tests/external-signing.test.ts. It is specifically run on a call to ldgbatcher_p28/transfer_private_3, but the flow is independent of that transaction’s structure.
  • An important piece of this PR relies on snarkVM’s sample_authorization_extended from Extend sample_authorization to facilitate multi-request flows snarkVM#3310, so this PR is in draft mode and has a path-patched Cargo.toml until the snarkVM PR is merged. The snarkVM changes are now meged and the snarkVM dependencies in the SDK have been bumped to a revision containing those changes.
  • The test matches the delegated-signing flow of our signing spec (which is amply commented in the test itself):
    • Authorizer has view_key but no private key.
    • Signer has the private key.
    • Authorizer calls sample_authorization_extended. Then it engages in one round of communication per request (i.e. per call in the transaction) with the signer as follows:
      1. Authorizer preprocess the request’s inputs for delegated signing, computing the to_fields representation for most types and (H, tag) for static-record inputs.
      2. Signer completes input processing and signs the requests, sending back the signature, the tvk and the gammas of the static-record inputs.
      3. Authorizer computes the input ID and puts together a complete, correct request.
      4. Authorizer uses the record-tracking information (from sample_authorization_extended) to update the nonces of record-like (static/external/dynamic-record) inputs to subsequent requests.
  • In order to run the above flow, this PR implements (or exposes from snarkVM) a couple of extra structures and methods:
    • sampleAuthorizationExtended (exposed from snarkVM), corresponding to the step before 1 above.
    • fromPreprocessedInputs, corresponding to step 2 above and returning a new structure PreprocessedSigningResult. It internally calls the also new signRequestFromPreprocessedInputs.
    • computeMintedNonce and applyMintedNonce, corresponding to step 4 above.
    • authorizeRequests (exposed from snarkVM), which is called after step 4 in the test. This is optional, and I believe in our case it is done on the the DPS side.

Tests

  • As indicated above, only one new test case is left on the PR, “Should match ExecutionRequest.sign for arbitrary flow with multiple requests”. This case performs the aforementioned flow and makes a few sanity checks on the result.
  • It was important to check the resulting requests can be proved and verified, but (as far as I can tell) the SDK repo is ill-equipped for that. So, in commit 8263ed4, I (read: the agent) added the necessary machinery to be able to locally mimic a ledger, prove, and verify the transaction. It worked (and it helped identify a bug, after fixing which the verification passed). The machinery is complex and I don’t think we want to keep it around (also, the test became long to run), so it was removed in the next commit and only a couple of relevant small pieces were added back in the commit after.
  • Even though only one transaction is tested here (although one with an already fairly interesting record flow), the snarkVM tests from Extend sample_authorization to facilitate multi-request flows snarkVM#3310 check sample_authorization_extended returns the correct mocked requests and (importantly) record-tracking machinery on hundreds of transactions, including quite wild flows of static/dynamic/external records and conversion between them — so we can be confident on that front.

Note

High Risk
Changes delegated signing and authorization assembly against a new snarkVM revision; incorrect nonce or signature handling could produce invalid or unsafe transactions.

Overview
Adds general multi-request external signing so an authorizer (view key only) and signer (private key) can walk an arbitrary call graph request-by-request, patch minted record nonces, and assemble a valid Authorization.

WASM / snarkVM: Bumps snarkVM to 8e2397ee… and exposes ProgramManager.authorizeRequests, sampleAuthorizationWithRecordTracking (mock auth plus record-tracking, checksums), and ExecutionRequest.fromPreprocessedInputs / signRequestFromPreprocessedInputs for signing from computeExternalSigningInputs output. Adds Group.gScalarMultiply for protocol-curve scalar multiplication.

SDK: Exports computeMintedNonce (HashToScalar([tvk, output_index]) * G) for patching consumer record _nonce after each signed minter request.

Tests: Adds LDGBATCHER_P28_PROGRAM and an end-to-end test for transfer_private_3 (four requests) that runs preprocess → fromPreprocessedInputsfromExternallySignedDataWithViewKey → nonce patching → verifyauthorizeRequests. Minor typing/consensus-version test tweaks.

Reviewed by Cursor Bugbot for commit dd3c831. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Comment thread wasm/src/types/group.rs Outdated
Comment thread wasm/src/programs/request.rs
Comment thread wasm/src/programs/request.rs Outdated
Comment on lines +472 to +475
let function_id_str = get_field(&external_inputs, "functionId")?
.as_string()
.ok_or_else(|| "external_inputs.functionId must be a string".to_string())?;
let function_id = FieldNative::from_str(&function_id_str).map_err(|e| e.to_string())?;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true, this boils down to the mix of representations here: https://github.com/ProvableHQ/sdk/blob/mainnet/wasm/src/programs/request.rs#L759-L762. I will fix as suggested (i.e. maintain both and be able to handle them) since I don't know whether changing to "functionId" will have any unexpected consequences (e.g. other libraries deserializing this objects breaking).

Comment thread wasm/src/programs/request.rs Outdated
Comment thread wasm/Cargo.toml Outdated
Comment thread sdk/src/external-signing.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Antonio Mejías Gil <anmegi.95@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Antonio Mejías Gil <anmegi.95@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Antonio Mejías Gil <anmegi.95@gmail.com>
@Antonio95 Antonio95 marked this pull request as ready for review June 24, 2026 15:15

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 81266c7. Configure here.

Comment thread wasm/src/programs/request.rs

@iamalwaysuncomfortable iamalwaysuncomfortable left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests were failing + lints were failing, I fixed those.

Otherwise this PR is very well done, very excellent first contribution to the SDK!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants