Skip to content

refactor(evolution): replace byte-splice witness merge with WithFormat round-trip#192

Merged
solidsnakedev merged 4 commits intomainfrom
refactor/cbor-witness-merge-with-format
Mar 11, 2026
Merged

refactor(evolution): replace byte-splice witness merge with WithFormat round-trip#192
solidsnakedev merged 4 commits intomainfrom
refactor/cbor-witness-merge-with-format

Conversation

@solidsnakedev
Copy link
Collaborator

addVKeyWitnessesBytes was performing manual CBOR byte surgery to inject vkey witnesses into an existing transaction — parsing map headers at the byte level, calculating offsets, and splicing raw bytes. This reimplemented low-level CBOR concerns that the WithFormat infrastructure already handles, and the four private helpers it required (cborHeaderSize, readMapCount, encodeMapHeader, unwrapVkeyArray) had no other callers.

addVKeyWitnessesBytes now captures the format tree with fromCBORBytesWithFormat, merges witnesses at the domain level via addVKeyWitnesses, and re-encodes with toCBORBytesWithFormat. The WithFormat reconciliation engine appends new map entries with a default simple format after preserving all existing format entries, so key 0 is encoded correctly even when absent in the original transaction. The four private helpers are removed. The CBOR encoding preservation spec is updated to treat the witness merge as an informative use case rather than normative content.

Copilot AI review requested due to automatic review settings March 10, 2026 15:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors addVKeyWitnessesBytes in the Evolution SDK to replace manual CBOR byte-splicing logic with a cleaner WithFormat round-trip approach. It also rewrites the CBOR encoding preservation specification from an implementation-plan style document into a normative API contract specification.

Changes:

  • addVKeyWitnessesBytes now uses fromCBORBytesWithFormataddVKeyWitnessestoCBORBytesWithFormat instead of manual byte surgery
  • Four private CBOR helpers (cborHeaderSize, readMapCount, encodeMapHeader, unwrapVkeyArray) are removed
  • The CBOR encoding preservation spec is restructured as a normative specification with appendices demoting the witness merge to an informative use case

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/evolution/src/Transaction.ts Replaces byte-splice implementation with WithFormat round-trip; removes four private helpers
.specs/cbor-encoding-preservation.md Rewrites spec from implementation plan to normative contract with appendices
.changeset/witness-merge-withformat.md Adds patch changeset entry describing the refactor

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +224 to +226
const walletWs = TransactionWitnessSet.fromCBORBytes(walletWitnessSetBytes, options)
const walletVkeys = walletWs.vkeyWitnesses ?? []
if (walletVkeys.length === 0) return txBytes
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The options parameter is passed to the wallet witness set decode but is not passed (and indeed cannot be passed) to fromCBORBytesWithFormat on line 229. The fromCBORBytesWithFormat function takes only bytes with no options parameter, so the options argument accepted by addVKeyWitnessesBytes has no effect on the transaction decode or re-encode path. This is a silent behavior change: callers who relied on passing custom CodecOptions (e.g., non-CML-default settings) to control the transaction encoding will now get the hardcoded behavior of fromCBORBytesWithFormat/toCBORBytesWithFormat regardless of what they pass. The options parameter should either be threaded through to fromCBORBytesWithFormat/toCBORBytesWithFormat if those APIs support it, or the parameter should be removed from the function signature to make the change explicit.

Copilot uses AI. Check for mistakes.
@solidsnakedev solidsnakedev merged commit 15d2774 into main Mar 11, 2026
5 of 7 checks passed
solidsnakedev added a commit that referenced this pull request Mar 11, 2026
PR #192 deleted the four private byte-splice helpers but extractBodyBytes
still relied on cborHeaderSize to locate the transaction array header before
extracting the raw body bytes. Restore it as a private helper scoped to the
body-extraction section.
solidsnakedev added a commit that referenced this pull request Mar 11, 2026
PR #192 deleted the four private byte-splice helpers but extractBodyBytes
still relied on cborHeaderSize to locate the transaction array header before
extracting the raw body bytes. Restore it as a private helper scoped to the
body-extraction section.
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.

2 participants