Conversation
|
Strong PR, and the write-up with the We build a mobile Cardano wallet and maintain a differential harness that runs an encoder against CSL 15.0.3, CML 6.2.0 and two cores of our own, comparing bytes on construction rather than round-tripping. We ran The catch is that four encode paths build the same byte-keyed maps inline instead of delegating to the modules this PR fixes, and three of them are on the transaction path.
That is probably why the repro in your PR description validated after the fix. The failing transaction you describe was a three-policy mint, which is exactly the field that goes through Reproductionconst P1 = PolicyId.fromHex("11".repeat(28)), P0 = PolicyId.fromHex("00".repeat(28))
let ma = MultiAsset.singleton(P1, AssetName.fromHex("aabbcc"), 5n)
ma = MultiAsset.addAsset(ma, P1, AssetName.fromHex("ff"), 7n)
ma = MultiAsset.addAsset(ma, P0, AssetName.fromHex("ff"), 9n)
MultiAsset.toCBORHex(ma) // fixed by this PR
Value.toCBORHex(Value.withAssets(1234567n, ma)) // still insertion orderA Babbage output built on this branch, same assets: The SuggestionHave On the tests
The general point, which is what we would most like to contribute: a round-trip property test is structurally blind to this class of bug, because decode preserves whatever order the input had, so encode∘decode is order-neutral no matter what the encoder does. Only construct-from-scratch-then-compare-against-a-reference finds a construction ordering defect. Three of the What we plan to doWe are preparing a few PRs against the SDK and would like to pick up this rework: the four delegations above, plus construct-then-compare oracle tests at Two limits on the above, so nothing reads as more than it is. We have not run your suite on this branch, so we make no regression claim, only a statement about what the new test covers. And we have not put a transaction from this branch in front of a device ourselves. The ordering results are byte comparison against CSL; the hardware consequence is yours, from your repro, not something we re-observed. |
|
thx for the quick reply. |
|
Thanks for the careful write-up and the real-world repro; this is exactly the kind of evidence that helps. We are keeping the default encoding as it is, so we won't merge a change that reorders freshly built maps. Hardware-wallet signing will go through an opt-in canonical mode instead, which needs no per-module sorting: What is missing is the builder side, which you described in #585: the script data hash, metadata hash and fee are computed with the default encoding. The plan there is #579 first, then #581 so a built transaction keeps the bytes it was built with, then an Closing this in favour of #585. Your test vectors (the three-policy order and the |
Problem
Mint,MultiAssetandWithdrawalsencode their CBOR maps in insertion order. CIP-21 requires policy IDs, asset names and withdrawal reward accounts in canonical CBOR key order (RFC 7049 §3.9: shorter first, then bytewise).Hardware wallets never receive the transaction bytes. They get the body field by field, serialize it themselves in canonical order, and sign the hash of their own serialization. When the SDK emits keys in another order, the device's body hash doesn't match the transaction, and the signature is useless.
This came up with a real preprod deployment built with the SDK: the tx minted under three policies, inserted in the order
56ed…, 8cba…, 7379….cardano-hw-cli transaction validatereportedCBOR is not canonical, and the Ledger could not sign it.Change
Bytes.compareCanonical: the canonical byte-string key order.Mint.FromCDDL,MultiAsset.FromCDDLandWithdrawals.FromCDDLsort entries with it on encode. Decode is unchanged.Decoded transactions keep their bytes. The format-preserving path (
Transaction.fromCBORHex→toCBORHex) replays each map's recordedkeyOrder(CBOR.ts, the map-encoding branch). A transaction decoded from a provider or wallet therefore re-encodes byte-for-byte, so its hash and existing signatures stay valid. Only freshly built values get the canonical order.No hashes are affected. The script data hash covers the redeemers, datums and language views, not the body. Mint redeemer indices were already derived from sorted policy IDs (
txBuilder.ts).Tests
test/CIP21.mapKeyOrder.test.ts:ff, 0000, 01. These must come out as01, ff, 0000: length decides before byte value.With the
src/change reverted, the three ordering tests fail and the round-trip test passes. With it, all four pass. The fullpackages/evolutionsuite passes: 1260 passed, 75 skipped.tsc -b tsconfig.src.jsonis clean.Not in this PR
CANONICAL_OPTIONSis not a fix for this. Re-encoding a Plutus transaction with it rewrites the redeemer encoding, but the body keeps the script data hash the builder computed under the default options, so the result fails ledger validation. Details: #585.