fix(Transaction): preserve CBOR encoding format through default decode/encode path#236
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes lossy CBOR round-trips for Transaction.fromCBORHex / fromCBORBytes by preserving the original CBOR encoding format (e.g., indefinite-length arrays) across default decode/encode flows, preventing scriptDataHash mismatches after witness additions.
Changes:
- Cache decoded
CBORFormatperTransactionin a module-levelWeakMap, and prefertoCBOR*WithFormaton encode when cached. - Transfer cached format across
Transaction.addVKeyWitnessesresults to keep encoding stable after witness merges. - Add regression tests using a real-world transaction containing indefinite-length arrays in redeemer
PlutusData, plus a patch changeset.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/evolution/src/transaction/Transaction.ts | Implements format caching + cache-aware encoding and cache transfer in addVKeyWitnesses. |
| packages/evolution/test/Transaction-format-preservation.test.ts | Adds regression coverage for exact CBOR round-trips and witness-addition format preservation. |
| .changeset/fix-cbor-format-preservation.md | Patch release note describing the behavior fix and rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const fromCBORBytes = (bytes: Uint8Array, _options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS): Transaction => { | ||
| const { format, value } = fromCBORBytesWithFormat(bytes) | ||
| formatCache.set(value, format) | ||
| return value | ||
| } | ||
|
|
||
| export const fromCBORHex = (hex: string, options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS) => | ||
| Schema.decodeSync(FromCBORHex(options))(hex) | ||
| export const fromCBORHex = (hex: string, _options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS): Transaction => { | ||
| const { format, value } = fromCBORHexWithFormat(hex) | ||
| formatCache.set(value, format) | ||
| return value |
There was a problem hiding this comment.
fromCBORBytes/fromCBORHex keep the options parameter but no longer use it (it’s renamed to _options and the WithFormat decode path doesn’t accept options). This is an observable behavior change for callers that relied on non-default CBOR.CodecOptions (e.g., mapsAsObjects). Consider either (1) threading options through the WithFormat decode path (e.g., add CBOR.fromCBOR*WithFormat(bytes, options) / internalDecodeWithFormatSync(data, options)), or (2) if options is non-default, fall back to the previous lossy Schema.decodeSync(FromCBOR*(options)) path and skip caching so the parameter still has effect.
| export const toCBORBytes = (data: Transaction, options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS): Uint8Array => { | ||
| const cached = formatCache.get(data) | ||
| if (cached) return toCBORBytesWithFormat(data, cached) | ||
| return Schema.encodeSync(FromCBORBytes(options))(data) | ||
| } | ||
|
|
||
| export const toCBORHex = (data: Transaction, options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS) => | ||
| Schema.encodeSync(FromCBORHex(options))(data) | ||
| export const toCBORHex = (data: Transaction, options: CBOR.CodecOptions = CBOR.CML_DEFAULT_OPTIONS): string => { | ||
| const cached = formatCache.get(data) | ||
| if (cached) return toCBORHexWithFormat(data, cached) | ||
| return Schema.encodeSync(FromCBORHex(options))(data) | ||
| } |
There was a problem hiding this comment.
toCBORBytes/toCBORHex ignore the passed options whenever a cached format exists (the cache path delegates to toCBOR*WithFormat, which always encodes with CML_DEFAULT_OPTIONS). This can surprise callers who pass CodecOptions expecting canonicalization or other behaviors. Consider bypassing the cache when options is non-default (preserve legacy semantics), or extend the WithFormat encode helpers to accept codec options so both format preservation and caller options can be honored explicitly.
- fromCBOR*: skip format cache when options !== CML_DEFAULT_OPTIONS - toCBOR*: bypass cached format when caller passes non-default options
Transaction.fromCBORHexandfromCBORBytesused the lossy CBOR decode path, which normalised indefinite-length arrays (0x9f) to definite-length (0x81). When a transaction contained non-canonical PlutusData in redeemers, re-encoding afteraddVKeyWitnessesproduced different bytes than the original, silently breakingscriptDataHashvalidation on node submission.The default decode functions now internally call the WithFormat path and cache the resulting
CBORFormattree in a module-levelWeakMap<Transaction, CBORFormat>. The encode functions check the cache first and delegate totoCBOR*WithFormatwhen a cached format exists.addVKeyWitnessestransfers the cached format from the source to the result transaction. Zero API surface change — callers get lossless round-trips without changing any code.Closes #235