You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
• Register a new MultiToken benchmark entry in the workspace benchmark manifest.
• Add a MultiToken benchmark covering mint/transfer/burn and commitment flows.
• Extend test utilities with MultiToken deploy, commitment initialization, and event decoding
helpers.
The following are alternative approaches to this PR:
1. Avoid wallet-internals by using simulate()+send() APIs
➕ Reduces coupling to WalletWithInternals/privateExecutionResult shapes
➕ Less brittle across Aztec SDK upgrades
➖ May not currently expose the partial-note commitment return value needed for commitment-based benchmarks
➖ Could require changes in benchmark workflow or waiting for SDK support
2. Derive commitments via contract-level observable output
➕ Eliminates dependency on private execution result internals
➖ Would require contract changes (e.g., emitting commitments) that may be undesirable for privacy/semantics
➖ Broadens PR scope beyond benchmarks/utilities
Recommendation: Current approach is reasonable for benchmarking today because commitment values are required and (per TODO) the public APIs may not expose them yet. Keep the internals-based helper tightly scoped (as done), and consider migrating to simulate()+send() once the SDK provides a supported way to obtain private return values.
• Introduces a 'Benchmark' implementation that deploys a MultiToken contract, pre-initializes transfer commitments, and benchmarks core methods (mint, transfers across privacy domains, burns, and commitment-based transfers). Uses raw u128-style amounts consistent with MultiToken's no-decimals design.
utils.tsAdd MultiToken deploy, commitment, and TransferSingle event helpers+181/-0
Add MultiToken deploy, commitment, and TransferSingle event helpers
• Adds MultiToken-specific fixtures and helpers: packing short strings into Fields for constructor args, a deploy helper with configurable minter/auth hook, a commitment initializer that extracts the commitment from proven private execution results, and utilities to decode/assert 4-field 'TransferSingle' public events.
Nargo.tomlRegister MultiToken benchmark in workspace manifest+1/-0
Register MultiToken benchmark in workspace manifest
• Adds a new benchmark entry mapping 'multitoken' to the MultiToken benchmark script so it can be discovered and run alongside existing token/nft/vault/escrow benchmarks.
1. Unchecked nested result access 🐞 Bug☼ Reliability
Description
initializeMultiTokenTransferCommitment blindly indexes nestedExecutionResults[0] and
returnValues[0], which will throw a TypeError if the private execution result has no nested
results/return values (making benchmark setup failures hard to debug). Add explicit shape checks
(and a clear error) before indexing, or search for the first nested result containing return values.
The new helper directly indexes nestedExecutionResults[0] and then returnValues[0] without any
checks, so an empty/malformed execution result will crash at runtime during setup.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`initializeMultiTokenTransferCommitment` assumes `provenTx.privateExecutionResult.entrypoint.nestedExecutionResults[0].returnValues[0]` exists. If that array is empty or the return values are missing (e.g., due to an execution layout change or a failed proof producing a different structure), the helper crashes with a non-actionable `TypeError`.
### Issue Context
This helper is used in benchmark setup to pre-create commitments; when it fails it blocks running the benchmark and the error message won’t indicate what was missing.
### Fix Focus Areas
- src/ts/test/utils.ts[854-878]
### Suggested fix
- Validate that `privateExecutionResult`, `entrypoint`, `nestedExecutionResults.length > 0`, and `returnValues.length > 0` before indexing.
- If invalid, throw an `Error` that includes the function name and a short description of what was missing (e.g. `No nestedExecutionResults in provenTx.privateExecutionResult`).
- Optionally, instead of hard-coding `[0]`, find the first nested execution result with non-empty `returnValues` and use that.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Unsafe bigint normalization 🐞 Bug☼ Reliability
Description
compressedStringToBigInt will call v.toBigInt() for any non-bigint/number value, which can
crash with an opaque error if the SDK decode result is null/undefined or doesn’t implement
toBigInt(). Add explicit null/shape checks and throw a descriptive conversion error.
+export function compressedStringToBigInt(result: any): bigint {+ const v = result?.value ?? result;+ if (typeof v === 'bigint') return v;+ if (typeof v === 'number') return BigInt(v);+ return v.toBigInt();+}
Evidence
The helper’s fallback path unconditionally calls toBigInt() on the derived value, which is not
guaranteed to exist for all decode shapes.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`compressedStringToBigInt` assumes any non-primitive input has a `toBigInt()` method. Unexpected decode shapes (including `undefined`/`null`) will cause a runtime crash with a low-signal message.
### Issue Context
This helper is meant to tolerate SDK decoding differences; as written it still fails hard on several plausible shapes.
### Fix Focus Areas
- src/ts/test/utils.ts[809-814]
### Suggested fix
- Change parameter type from `any` to `unknown`.
- Handle `null`/`undefined` explicitly.
- Before calling, check `typeof (v as any)?.toBigInt === 'function'`.
- Otherwise throw `new Error('compressedStringToBigInt: unsupported decode shape')` (optionally include `typeof v`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author self-review: I have reviewed the code review findings, and addressed the relevant ones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
🤖 Linear
Closes AZT-XXX
Description
add benchmarks