Optional burnStateMask in TokenSplit.split + tolerant CertificationResponse parsing#125
Conversation
…nResponse parsing - TokenSplit.split gains an optional burnStateMask parameter (default: random, backward compatible) — the last internally generated random that prevented rebuilding a byte-identical split after a crash. Callers doing crash-resumable transfers derive it deterministically. - CertificationResponse.fromJSON no longer throws on status strings outside the CertificationStatus enum: an aggregator may emit statuses unknown to this SDK version (e.g. STATE_ID_EXISTS, removed in aef6ffa but still emitted by deployed aggregators until their removal lands). Unknown statuses parse and must be treated as 'not accepted - probe the inclusion proof'. The status type is CertificationStatus | (string & {}) so existing enum comparisons keep a shared enum type. Tests: unknown-status parse/round-trip + empty-status rejection; byte-identical burn transaction from a fixed mask, random default preserved. Closes #124
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two main changes: first, it relaxes the status validation in CertificationResponse to tolerate unknown status strings, preventing parsing failures when interacting with different aggregator versions. Second, it adds an optional burnStateMask parameter to TokenSplit.split to allow callers to reconstruct identical burn transactions for crash-resiliency. Corresponding unit and functional tests have been added. The feedback suggests validating that the caller-supplied burnStateMask is exactly 32 bytes long to prevent potential cryptographic or protocol errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| burnStateMask?: Uint8Array, | ||
| ): Promise<ISplit> { | ||
| const hasher = new DataHasherFactory(HashAlgorithm.SHA256, DataHasher); |
There was a problem hiding this comment.
To prevent potential cryptographic or protocol errors, we should validate that the caller-supplied burnStateMask is exactly 32 bytes long (matching the length of the default random mask).
| burnStateMask?: Uint8Array, | |
| ): Promise<ISplit> { | |
| const hasher = new DataHasherFactory(HashAlgorithm.SHA256, DataHasher); | |
| burnStateMask?: Uint8Array, | |
| ): Promise<ISplit> { | |
| if (burnStateMask !== undefined && burnStateMask.length !== 32) { | |
| throw new RangeError('The burnStateMask must be exactly 32 bytes long.'); | |
| } | |
| const hasher = new DataHasherFactory(HashAlgorithm.SHA256, DataHasher); |
| */ | ||
| export class CertificationResponse { | ||
| public constructor(public readonly status: CertificationStatus) {} | ||
| public constructor(public readonly status: CertificationStatus | (string & {})) {} |
There was a problem hiding this comment.
I think we can assume that aggregator response is always unknown string and on comparison compare against enum
… validate burnStateMask length - CertificationResponse.status is now plain string (the aggregator response is treated as an arbitrary status string; known values stay enumerated in CertificationStatus and comparison sites compare against them) - TokenSplit.split rejects a burnStateMask that is not exactly 32 bytes (RangeError), with a test
|
Both review items addressed in the latest push: (1) |
2 participants
Closes #124
Two changes for the wallet-api program's recoverable engine (one rc release can cover both):
burnStateMask?: Uint8ArrayonTokenSplit.split— default random (backward compatible). The burn transaction's state mask was the last internally generated random preventing a crash-resumable split: a caller that derives the mask deterministically (HKDF from wallet key + transferId) can now rebuild the byte-identical burn transaction after a failure and recover viagetInclusionProof+ match-verify.Tolerant
CertificationResponseparsing —fromJSONno longer throwsInvalidJsonStructureErroron status strings outside the enum. Deployed aggregators still emitSTATE_ID_EXISTS(removed from the enum in aef6ffa/[TGE] Migrate minting from caller-supplied token ids tonetworkId+salt#115) until their own removal lands; an unknown status now parses and is treated by callers as "not accepted — probe the inclusion proof". Transitional by design: stays harmless after the aggregator catches up, and also future-proofs against statuses added later. The status type isCertificationStatus | (string & {})so existing enum comparisons keep a shared enum type (lint-verified).Verification: full suite 58/58 green (2 new tests: unknown-status parse/round-trip + empty-status rejection; byte-identical burn tx from a fixed mask with the random default preserved); lint + build clean; both changes verified through the packed
lib/artifact shape.Review + rc publish: deferred to maintainers per program decision (wallet-api owner, 2026-06-11) — please sanity-check the tolerant-parsing direction against the post-#115 aggregator roadmap.