Repository navigation
Conversation
…O#574) encodeTagSync always wrote tag 258 for a Tag node and encoded the inner array with no format, so a set decoded without the tag gained d90102 on re-encode: the body bytes changed, the transaction id changed, and witnesses already collected stopped verifying. When the captured format at a tag-258 position is an array node, the inner array is now encoded with that node and the tag is omitted; a tag node or no format writes the tag as before, so freshly built transactions do not change. TransactionWitnessSet.withAddedFormats kept the same assumption for Plutus-data witness sets, converting an untagged set's format to the fresh tagged one when items were added; it now keeps the decoded array node and takes only the added items' formats from the fresh tree. One existing test asserted the old tagged outcome for an added datum; its expectation now asserts the untagged form. Tests: 5 new CBOR-level checks (definite/indefinite array nodes, nested widths, tag-node and no-format controls) and 9 new transaction-level checks (byte-identical round trips via WithFormat and the cached default path, CML body-hash equality, certificates/collateral/signers/ reference inputs, untagged vkey witnesses, addVKeyWitnessesHex body stability, appended witness keeping the untagged form). Full suite: 1704 passed / 75 skipped / 0 failed.
Author
|
Correction to the test count in the description above (now edited there too): the full suite result is 1692 passed / 75 skipped / 0 failed, not 1704. The 1704 figure added the 14 new tests to the wrong baseline (1690 was a previous branch's total; upstream main's baseline is 1678, and 1678 + 14 = 1692). The commit message still says 1704 — I am not amending it (no force-push); this comment and the edited description carry the correct count. Nothing else in the description changes. |
This branch has not been deployed
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #574: a transaction whose sets are plain or indefinite arrays, with no tag 258, came back from
Transaction.fromCBORHex→Transaction.toCBORHexwithd90102added. The body bytes changed, so the transaction id changed and every witness already collected no longer verified.I followed the fix the issue prescribes: let the captured format decide whether the tag is written.
The bug, reproduced first
New regression tests failed on main with exactly the reported symptom — every untagged set position gained
d90102:1800sits inside the array node the decoder captured, and the oldencodeTagSyncdropped that whole subtree by encoding the inner array with no formatAt the CBOR level the vector is minimal: a
Tag258 value encoded with an array-node format producedd90102820102instead of820102.Fix
CBOR.ts—encodeTagSync: when the tag is 258 and the captured format at that position is an array node, encode the inner array with that node and omit the tag. When the format is a tag node the tag is written (with its captured width), and with no captured format the tag is written exactly as before, so freshly built transactions do not change.TransactionWitnessSet.ts—withAddedFormats: its array branch used to convert an untagged set's format to the fresh (tagged) format when items were added, on the assumption that a decoded untagged set is written tagged. It now keeps the decoded array node and takes only the added items' formats from the fresh tree's inner array, so an edit — a datum appended to an untagged datum set — keeps the original form of everything around it, as the issue requires. One existing test asserted the old tagged outcome for that case; its expectation now asserts the untagged form and says why.Nothing else changes: no encoder change was needed in
TransactionBody.tsor the witness-set transform — both already build theTagnode, and the format tree is where the tag/no-tag fact lives.Tests
1800index), tag node keeps the tag, no format keeps the tag.fromCBORHex/toCBORHexpath; body hash equality against CML; the combined certificates/collateral/signers/reference-inputs body; an untagged vkey witness set;addVKeyWitnessesHexleaving the body bytes unchanged; a witness appended to an untagged witness set keeping the untagged form; and the tagged/fresh controls.tsc --noEmitand eslint clean; the touched files' prettier diff-line counts exactly equal main's; a changeset is included.—
Independent builder, not affiliated with the Cardano Foundation, IOG or Intersect.
@Kshot9000 (https://x.com/kshot9000) · Tips: addr1q8hnl6vl5a6k3rw3n5g3jtte696zcl76kfatzv7gpswa9r0dj7fma6klq55y4ffm7tf0em09udnyhuk4ah92pl5x9jpqjae44v
cc @cardano-foundation @IntersectMBO