Skip to content

Commit 0a56e71

Browse files
committed
fix(coin-zcash): bound input selection to device PCZT ceilings
1 parent 0f0627f commit 0a56e71

11 files changed

Lines changed: 539 additions & 22 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@ledgerhq/coin-zcash": patch
3+
---
4+
5+
Bound transparent-input and Ironwood-note selection to the device's per-PCZT ceilings on both pools, so a send from an account holding more UTXOs or notes than the device can sign in one PCZT no longer produces an unsignable transaction. A send whose full balance covers the requested amount but whose device-safe selection does not now reports a distinct, actionable error instead of a plain insufficient-balance one.

libs/coin-modules/coin-zcash/src/bridge/getTransactionStatus.test.ts

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { BigNumber } from "bignumber.js";
2+
import { NotEnoughBalance } from "@ledgerhq/ledger-wallet-framework/errors";
23
import { getTransactionStatus } from "./getTransactionStatus";
34
import { prepareTransaction } from "./prepareTransaction";
45
import {
@@ -9,10 +10,14 @@ import {
910
resolveTransparentUtxos,
1011
} from "./statusHelpers";
1112
import { TRANSPARENT_OUTPUT_DUST_THRESHOLD, ZIP317_MINIMUM_FEE } from "../logic/coin-selection";
12-
import { ZcashAmountBelowDustThreshold } from "../types/errors";
13+
import { ZcashAmountBelowDustThreshold, ZcashSendTooLarge } from "../types/errors";
1314
import type { BitcoinOutput, Transaction, ZcashAccount, ZcashTransferType } from "../types/bridge";
1415
import type { SpendableNote } from "../network/types";
15-
import { ZCASH_SHIELDED_SPENDABILITY_DELAY_BLOCKS } from "../constants";
16+
import {
17+
ZCASH_MAX_IRONWOOD_ACTIONS,
18+
ZCASH_MAX_TRANSPARENT_INPUTS,
19+
ZCASH_SHIELDED_SPENDABILITY_DELAY_BLOCKS,
20+
} from "../constants";
1621

1722
const T_ADDRESS = "t1b1Rbw2shhJkP6MCnCyxCPuyFedHrwKty8";
1823
const U_ADDRESS =
@@ -489,6 +494,122 @@ describe("getTransactionStatus, transparent-input flows", () => {
489494
});
490495
});
491496

497+
describe("getTransactionStatus, bounded-selection shortfall (ZcashSendTooLarge)", () => {
498+
const FEE = 10_000;
499+
500+
it.each(["transparent", "transparent-to-shielded"] as ZcashTransferType[])(
501+
"resolves without an amount error for a %s send within the bounded max, from an account above the bound",
502+
async transferType => {
503+
const utxoCount = ZCASH_MAX_TRANSPARENT_INPUTS + 5;
504+
const acc = account({ utxos: Array(utxoCount).fill(100_000) });
505+
const boundedBalance = ZCASH_MAX_TRANSPARENT_INPUTS * 100_000;
506+
const recipient = transferType === "transparent" ? T_ADDRESS : U_ADDRESS;
507+
const tx = transaction({
508+
transferType,
509+
recipient,
510+
amount: new BigNumber(boundedBalance - FEE),
511+
zcashFee: new BigNumber(FEE),
512+
});
513+
514+
const status = await getTransactionStatus(acc, tx);
515+
516+
expect(status.errors.amount).toBeUndefined();
517+
},
518+
);
519+
520+
it.each(["transparent", "transparent-to-shielded"] as ZcashTransferType[])(
521+
"rejects a %s send between the bounded max and the full balance with ZcashSendTooLarge",
522+
async transferType => {
523+
const utxoCount = ZCASH_MAX_TRANSPARENT_INPUTS + 5;
524+
const acc = account({ utxos: Array(utxoCount).fill(100_000) });
525+
const boundedBalance = ZCASH_MAX_TRANSPARENT_INPUTS * 100_000;
526+
const fullBalance = utxoCount * 100_000;
527+
const recipient = transferType === "transparent" ? T_ADDRESS : U_ADDRESS;
528+
// amount + FEE sits strictly between boundedBalance and fullBalance.
529+
const tx = transaction({
530+
transferType,
531+
recipient,
532+
amount: new BigNumber(boundedBalance),
533+
zcashFee: new BigNumber(FEE),
534+
});
535+
536+
const status = await getTransactionStatus(acc, tx);
537+
538+
expect(status.errors.amount).toBeInstanceOf(ZcashSendTooLarge);
539+
expect(status.errors.amount).not.toBeInstanceOf(NotEnoughBalance);
540+
expect(boundedBalance + FEE).toBeLessThanOrEqual(fullBalance);
541+
},
542+
);
543+
544+
it("keeps reporting NotEnoughBalance for a genuine shortfall, on an account below the bound", async () => {
545+
const acc = account({ utxos: [10_000, 10_000, 10_000] }); // full balance 30_000, well below the bound
546+
const tx = transaction({
547+
amount: new BigNumber(100_000),
548+
zcashFee: new BigNumber(FEE),
549+
});
550+
551+
const status = await getTransactionStatus(acc, tx);
552+
553+
expect(status.errors.amount).toEqual(new NotEnoughBalance());
554+
expect(status.errors.amount).not.toBeInstanceOf(ZcashSendTooLarge);
555+
});
556+
557+
it("resolves without an amount error for a shielded send within the bounded max, from a pool above the bound", async () => {
558+
const noteCount = ZCASH_MAX_IRONWOOD_ACTIONS + 5;
559+
const acc = account({ ironwoodNotes: Array(noteCount).fill(100_000) });
560+
const boundedTotal = ZCASH_MAX_IRONWOOD_ACTIONS * 100_000;
561+
const selectedNotes = Array.from({ length: ZCASH_MAX_IRONWOOD_ACTIONS }, (_, i) =>
562+
note(100_000, i),
563+
);
564+
const tx = transaction({
565+
transferType: "shielded",
566+
recipient: U_ADDRESS,
567+
amount: new BigNumber(boundedTotal - FEE),
568+
selectedNotes,
569+
zcashFee: new BigNumber(FEE),
570+
});
571+
572+
const status = await getTransactionStatus(acc, tx);
573+
574+
expect(status.errors.amount).toBeUndefined();
575+
});
576+
577+
it("rejects a shielded send between the bounded and full pool with ZcashSendTooLarge, not the generic insufficiency error", async () => {
578+
const noteCount = ZCASH_MAX_IRONWOOD_ACTIONS + 5;
579+
const acc = account({ ironwoodNotes: Array(noteCount).fill(100_000) });
580+
const boundedTotal = ZCASH_MAX_IRONWOOD_ACTIONS * 100_000;
581+
const fullTotal = noteCount * 100_000;
582+
const tx = transaction({
583+
transferType: "shielded",
584+
recipient: U_ADDRESS,
585+
amount: new BigNumber(boundedTotal),
586+
zcashFee: new BigNumber(FEE),
587+
});
588+
589+
const status = await getTransactionStatus(acc, tx);
590+
591+
expect(status.errors.amount).toBeInstanceOf(ZcashSendTooLarge);
592+
expect(status.errors.amount).not.toEqual(new Error("Insufficient shielded balance"));
593+
expect(boundedTotal + FEE).toBeLessThanOrEqual(fullTotal);
594+
});
595+
596+
it("keeps reporting the generic insufficiency error for a genuine shielded shortfall, on a pool below the bound", async () => {
597+
const acc = account({ ironwoodNotes: [10_000, 10_000] }); // pool 20_000, well below the bound
598+
const tx = transaction({
599+
transferType: "shielded",
600+
recipient: U_ADDRESS,
601+
amount: new BigNumber(100_000),
602+
selectedNotes: [note(20_000)],
603+
zcashFee: new BigNumber(FEE),
604+
});
605+
606+
const status = await getTransactionStatus(acc, tx);
607+
608+
expect(status.errors.amount).toEqual(new Error("Insufficient shielded balance"));
609+
expect(status.errors.amount).not.toBeInstanceOf(ZcashSendTooLarge);
610+
});
611+
});
612+
492613
describe("getTransactionStatus, note-spending flows", () => {
493614
it("refuses to price anything before the shielded scan has run", async () => {
494615
const status = await getTransactionStatus(

libs/coin-modules/coin-zcash/src/bridge/getTransactionStatus.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,26 @@ import { NotEnoughBalance } from "@ledgerhq/ledger-wallet-framework/errors";
33
import type { AccountBridge } from "@ledgerhq/types-live";
44
import type { Transaction, TransactionStatus, ZcashAccount } from "../types/bridge";
55
import { ZIP317_MINIMUM_FEE } from "../logic/coin-selection";
6-
import { ZcashAmountBelowDustThreshold, ZcashMemoTooLong } from "../types/errors";
6+
import {
7+
ZcashAmountBelowDustThreshold,
8+
ZcashMemoTooLong,
9+
ZcashSendTooLarge,
10+
} from "../types/errors";
711
import { ZCASH_MEMO_MAX_BYTES } from "../constants";
812
import {
913
computeAmountError,
1014
computeRecipientError,
15+
hasBoundedTransparentShortfall,
1116
hasShieldedKey,
1217
isTransparentInputTransfer,
1318
isTransparentOutputDust,
1419
resolveTransparentUtxos,
1520
} from "./statusHelpers";
1621
import { getReservedNullifiers } from "./note-reservation";
17-
import { getSpendableIronwoodBalance } from "../logic/account/spendability";
22+
import {
23+
getSpendableIronwoodBalance,
24+
hasBoundedIronwoodShortfall,
25+
} from "../logic/account/spendability";
1826

1927
const encoder = new TextEncoder();
2028

@@ -54,7 +62,9 @@ function getTransparentInputStatus(
5462
if (tx.amount.lte(0) && !tx.useAllAmount) {
5563
errors.amount = new Error("Amount must be positive");
5664
} else if (totalSpent.gt(transparentBalance)) {
57-
errors.amount = new NotEnoughBalance();
65+
errors.amount = hasBoundedTransparentShortfall(account, tx, totalSpent)
66+
? new ZcashSendTooLarge()
67+
: new NotEnoughBalance();
5868
} else if (tx.transferType === "transparent" && isTransparentOutputDust(tx.amount)) {
5969
errors.amount = new ZcashAmountBelowDustThreshold();
6070
}
@@ -99,7 +109,8 @@ export const getTransactionStatus: AccountBridge<
99109
// Shielded sends spend the Ironwood pool, so validate the amount against the
100110
// mature, unreserved figure -- the same one selection draws from, so the
101111
// status can never accept an amount selection cannot cover.
102-
const poolBalance = getSpendableIronwoodBalance(account, getReservedNullifiers(account));
112+
const reserved = getReservedNullifiers(account);
113+
const poolBalance = getSpendableIronwoodBalance(account, reserved);
103114
const fee = transaction.zcashFee ?? new BigNumber(ZIP317_MINIMUM_FEE);
104115
const totalSpent = transaction.amount.plus(fee);
105116

@@ -110,16 +121,20 @@ export const getTransactionStatus: AccountBridge<
110121
);
111122
if (recipientError) errors.recipient = recipientError;
112123

113-
const amountError = computeAmountError(transaction, totalSpent, poolBalance);
114-
if (amountError) {
115-
errors.amount = amountError;
116-
} else if (
117-
transaction.transferType === "shielded-to-transparent" &&
118-
isTransparentOutputDust(transaction.amount)
119-
) {
120-
// A shielded-to-transparent (z->t) recipient is also a transparent
121-
// output, so it needs the same dust check as a t->t send.
122-
errors.amount = new ZcashAmountBelowDustThreshold();
124+
if (hasBoundedIronwoodShortfall(account, reserved, totalSpent)) {
125+
errors.amount = new ZcashSendTooLarge();
126+
} else {
127+
const amountError = computeAmountError(transaction, totalSpent, poolBalance);
128+
if (amountError) {
129+
errors.amount = amountError;
130+
} else if (
131+
transaction.transferType === "shielded-to-transparent" &&
132+
isTransparentOutputDust(transaction.amount)
133+
) {
134+
// A shielded-to-transparent (z->t) recipient is also a transparent
135+
// output, so it needs the same dust check as a t->t send.
136+
errors.amount = new ZcashAmountBelowDustThreshold();
137+
}
123138
}
124139

125140
return {

libs/coin-modules/coin-zcash/src/bridge/mapping.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ export async function mapTransparentInputs(
7575
);
7676
}
7777

78+
/**
79+
* Always exactly one requested output. The native builder adds at most one
80+
* change output on top (craft.rs) -- coin-zcash itself never requests more
81+
* than one, so the transparent/shielded output count this module can produce
82+
* is capped at 2 regardless of account size, comfortably under either pool's
83+
* device output ceiling. This is why input/spend selection is bounded but
84+
* outputs are not: there is nothing to bound on the output side.
85+
*/
7886
export function mapOutputs(tx: Transaction): OutputRequestJs[] {
7987
return [
8088
{

0 commit comments

Comments
 (0)