fix(zcash): restrict raw source-tx override to V5 (LIVE-35215) - #20295
fix(zcash): restrict raw source-tx override to V5 (LIVE-35215)#20295cted-ledger wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Zcash transparent signing failures when one or more selected UTXOs come from a V4 transaction by restricting serializedPreviousTransactionOverride usage to V5+ source transactions (where shielded bundles can exist outside extraData). It also improves device-action error mapping so untagged task failures keep useful context instead of becoming message-less errors, and adds regression tests plus a changeset.
Changes:
- Restrict raw previous-tx override to V5+ source transactions based on parsed transaction version.
- Improve device-action error mapping to preserve messages for untagged
Errors and describe non-Errorfailures. - Add unit tests covering V4/V5 override behavior, mixed-input scenarios, and error-mapping fallbacks; add a patch changeset.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| libs/live-signer-zcash/src/DmkSignerZcash.ts | Gate serializedPreviousTransactionOverride to V5+ and preserve error context for untagged device-action failures. |
| libs/live-signer-zcash/tests/DmkSignerZcash.test.ts | Add regression tests for per-input override decisions and improved error mapping behavior. |
| .changeset/PROD-12599-zcash-v4-source-tx-override.md | Document the patch release and user-facing impact of the signing fix and error-message improvement. |
Suppressed comments (2)
libs/live-signer-zcash/tests/DmkSignerZcash.test.ts:496
- Similarly here, the block comment is long and repeats the scenario already captured by the test name. Consider collapsing it to a brief reference to PROD-12599 so the test stays readable and the rationale doesn’t go stale.
// The payload reported in PROD-12599: three inputs, the first two funded
// by V5 transactions and the last by a V4 one. Both V5 inputs were chunked
// and their trusted inputs obtained from the device, so the send died only
// when the V4 input's turn came — after nineteen APDU exchanges, and still
// before any review screen.
libs/live-signer-zcash/tests/DmkSignerZcash.test.ts:657
- This comment block is helpful context but fairly long for a unit test; the two test cases below already document the behavior. Consider shrinking it to a one-liner with a ticket reference to keep the suite easier to scan.
// A device action reports its own failures as tagged errors, but a plain
// exception thrown inside one of its tasks arrives untagged. It used to be
// turned into `new Error(undefined)`, which is how a crash while chunking a
// source transaction reached users as a message-less "Something went wrong"
// and left support logs with nothing to go on (PROD-12599).
Web Tools Build Status
|
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-preloaderPath:
📁 desktop-rendererPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
cted-ledger
changed the title
Signing a transparent transaction whose input came from a V4 transaction failed before any device prompt. The signer kit chunks the overridden raw bytes expecting Ledger's internal serialization, whose V4 header carries a consensus branch id that the on-chain bytes lack, so it read the input count four bytes late and threw while building the first APDU. The override exists for V5, whose Orchard bundle the kit's serialization would strip, producing a wrong ZIP-244 txid. It is now restricted to those versions; a V4 source transaction goes back through the serialization path that has always handled it, its Sapling fields travelling in extraData. Untagged device action errors are no longer flattened into a message-less Error, so a failure inside a device action task names itself in the logs instead of surfacing only as "Something went wrong". Their fields are read off a narrowed object, since a task can also reject with a primitive, null or undefined, which the previous inspection would have thrown on.
1a296bb to
fac80f1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libs/live-signer-zcash/tests/DmkSignerZcash.test.ts:476
- This V4 fixture inherits
nVersionGroupIdfromprevTx(V5), making the object internally inconsistent. Setting the V4 version group id here makes the test data accurate and avoids accidental coupling to the V5 defaults.
...prevTx,
version: Buffer.from([0x04, 0x00, 0x00, 0x80]),
rawTxHex: "0400008085202f89" + "ab".repeat(59),
};
libs/live-signer-zcash/src/DmkSignerZcash.ts:70
mapErrornow explicitly supports untagged / non-object rejections at runtime, but its TypeScript signature still constrainserrorto{ _tag: string }. This is misleading and can reintroduce unsafe assumptions at call sites; consider loosening the signature tounknown(private method, so no API impact).
private mapError<E extends { _tag: string }>(error: E): Error {
// A task can also reject with an untagged value, down to a primitive, so
// nothing here may assume an object shape (LIVE-35215).
const details: { errorCode?: unknown; _tag?: unknown } =
typeof error === "object" && error !== null ? error : {};
|
📝 Description
Signing a Zcash transparent transaction failed with "Something went wrong", no prompt ever reaching the device, whenever one of the selected UTXOs came from a V4 transaction. Reported in production as PROD-12599 and reproduced on
develop.Previous behaviour:
DmkSignerZcashhands the raw source-transaction bytes to the signer kit throughserializedPreviousTransactionOverride. That override exists for V5 sources, whose Orchard bundle the kit's own serialization strips — which would make the device commit to a wrong ZIP-244 txid. It was applied to every version, but the kit chunks those bytes expecting Ledger's internal serialization, whose V4 header carries a consensus branch id that the on-chain bytes do not have. It therefore read the input count four bytes late and threw while chunking that input, so the transaction died mid-flight, after any earlier V5 inputs had already been streamed.mapErrorthen flattened the untagged error into a message-lessError, which is why the UI could only report "Something went wrong".Fix: the override is now restricted to the versions that can carry a shielded bundle, V5 and above. A V4 source transaction goes back through the serialization path that has always handled it, its Sapling fields travelling in
extraData. Untagged device-action errors keep their context instead of being flattened, so a failure inside a task names itself in the logs.Regression cover: unit tests assert that the override is applied for V5, omitted for V4, and omitted when the version field is too short to read; a multi-input case mirrors the reported logs, with two V5 sources and one V4 among them, the V4 being the only one skipping the override. The error-mapping fallbacks are covered as well.
Verified on device: the account that reproduced the failure now signs.
This is one half of the Zcash signing problem. The other half — the device computing a wrong txid for a V5 source carrying an Orchard bundle, which surfaced as a 504 on broadcast — is fixed device-side in LedgerHQ/device-sdk-ts#1702 and will reach this repo through a
@ledgerhq/device-signer-kit-zcashbump.🔗 Context