Skip to content

Commit c90d669

Browse files
committed
fix(psbt): reject impossible fees, surface locktime, RBF and dust
Outputs adding up to more than the inputs cannot happen on chain, but the review screen clamped the fee to zero and then rendered no fee row at all, so a PSBT with an understated input or an inflated output showed no fee and no warning. It is now refused, guarded on every input having supplied an amount since a missing one counts as zero and produces the same shape honestly. Where that guard applies the absent fee is named rather than left blank. nLockTime and nSequence appeared nowhere on the review screen, so a transaction that cannot be broadcast for years and one that is replaceable both read as ordinary. Outputs below the relay dust threshold were shown as ordinary spends; the threshold follows Bitcoin Core's GetDustThreshold, giving the familiar 294 / 330 / 546 values. Found by running the psbt_faker signing-test suite against the review path.
1 parent 1feb652 commit c90d669

5 files changed

Lines changed: 248 additions & 0 deletions

File tree

main/core/psbt.c

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ void psbt_audit_input_amounts(const struct wally_psbt *psbt,
111111

112112
for (size_t i = 0; i < out->num_inputs; i++) {
113113
psbt_input_amount_t amount = psbt_get_input_amount(psbt, i);
114+
out->total += amount.value;
114115
switch (amount.status) {
115116
case PSBT_AMOUNT_PROVEN:
116117
out->proven++;
@@ -136,6 +137,36 @@ uint64_t psbt_get_input_value(const struct wally_psbt *psbt, size_t index) {
136137
return psbt_get_input_amount(psbt, index).value;
137138
}
138139

140+
/* 21 million BTC. libwally already rejects a transaction whose outputs exceed
141+
* the supply, so this only backstops a total assembled some other way -- but
142+
* it is what keeps the sum below from silently wrapping. */
143+
#define PSBT_MAX_SATOSHI (21000000ull * WALLY_SATOSHI_PER_BTC)
144+
145+
bool psbt_total_output_value(const struct wally_psbt *psbt, uint64_t *out) {
146+
if (!psbt || !out)
147+
return false;
148+
*out = 0;
149+
150+
struct wally_tx *tx = psbt_tx_alloc(psbt);
151+
if (!tx)
152+
return false;
153+
154+
uint64_t total = 0;
155+
bool ok = true;
156+
for (size_t i = 0; i < tx->num_outputs && ok; i++) {
157+
uint64_t value = tx->outputs[i].satoshi;
158+
if (value > PSBT_MAX_SATOSHI || total > PSBT_MAX_SATOSHI - value)
159+
ok = false;
160+
else
161+
total += value;
162+
}
163+
wally_tx_free(tx);
164+
165+
if (ok)
166+
*out = total;
167+
return ok;
168+
}
169+
139170
struct wally_tx *psbt_tx_alloc(const struct wally_psbt *psbt) {
140171
struct wally_tx *tx = NULL;
141172
if (!psbt ||
@@ -152,6 +183,29 @@ static bool psbt_is_v2(const struct wally_psbt *psbt) {
152183
version == WALLY_PSBT_VERSION_2;
153184
}
154185

186+
/* Bitcoin Core's IsWitnessProgram: OP_0 or OP_1..OP_16, then a single push of
187+
* 2..40 bytes making up the whole script. */
188+
static bool spk_is_witness_program(const unsigned char *spk, size_t spk_len) {
189+
if (spk_len < 4 || spk_len > 42)
190+
return false;
191+
if (spk[0] != OP_0 && (spk[0] < OP_1 || spk[0] > OP_16))
192+
return false;
193+
return (size_t)spk[1] + 2 == spk_len;
194+
}
195+
196+
uint64_t psbt_output_dust_threshold(const unsigned char *spk, size_t spk_len) {
197+
if (!spk || !spk_len || spk[0] == OP_RETURN)
198+
return 0; /* provably unspendable: never dust */
199+
200+
/* Core's GetDustThreshold: the serialized output plus the cheapest input
201+
* that could spend it, priced at the 3000 sat/kvB dust relay fee. The input
202+
* side is 67 bytes for a witness program (the witness is discounted) and
203+
* 148 for everything else. Yields the familiar 294 / 330 / 546 thresholds. */
204+
size_t size = 8 + 1 + spk_len; /* value + script length + script */
205+
size += spk_is_witness_program(spk, spk_len) ? 67 : 148;
206+
return (uint64_t)size * 3000u / 1000u;
207+
}
208+
155209
bool psbt_sighash_is_supported(uint32_t sighash) {
156210
/* 0 is both the "no PSBT_IN_SIGHASH_TYPE field" encoding and taproot's
157211
* SIGHASH_DEFAULT; either way libwally signs with ALL semantics. */

main/core/psbt.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ typedef struct {
108108
size_t asserted;
109109
size_t invalid;
110110
size_t missing;
111+
/* Sum of every input's resolved value. A MISSING input contributes 0, so
112+
* this is only the real total when `missing` is 0. */
113+
uint64_t total;
111114
/* Index of the first input in each non-proven category, or num_inputs when
112115
* that category is empty. Used to name a concrete input in the warning. */
113116
size_t first_unproven;
@@ -132,6 +135,11 @@ static inline bool psbt_amounts_are_proven(const psbt_amount_audit_t *audit) {
132135
// Get input value in satoshis
133136
uint64_t psbt_get_input_value(const struct wally_psbt *psbt, size_t index);
134137

138+
// Sum of every output's value. False when the transaction cannot be read or
139+
// an amount is out of range, in which case nothing about the fee is knowable.
140+
KERN_WARN_UNUSED_RESULT bool
141+
psbt_total_output_value(const struct wally_psbt *psbt, uint64_t *out);
142+
135143
// The transaction under review, for either PSBT version. v0 carries it as a
136144
// global field; v2 spreads the same information across per-input and
137145
// per-output fields and it has to be rebuilt, which also resolves BIP-370's
@@ -142,6 +150,12 @@ uint64_t psbt_get_input_value(const struct wally_psbt *psbt, size_t index);
142150
KERN_WARN_UNUSED_RESULT struct wally_tx *
143151
psbt_tx_alloc(const struct wally_psbt *psbt);
144152

153+
// Bitcoin Core's dust threshold for an output paying `spk`: below this the
154+
// output costs more to spend than it holds and the transaction will not relay.
155+
// 0 for provably unspendable outputs, which Core never counts as dust.
156+
KERN_WARN_UNUSED_RESULT uint64_t
157+
psbt_output_dust_threshold(const unsigned char *spk, size_t spk_len);
158+
145159
// Sighash flags the review screen can honestly describe. SIGHASH_ALL (and the
146160
// taproot SIGHASH_DEFAULT, which is 0 and also the "unset" encoding) commit to
147161
// every input and output, so what was displayed is what gets mined. Under

main/core/test/test_psbt_classify.c

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,73 @@ static void test_fee_percent(void) {
14371437
PASS();
14381438
}
14391439

1440+
static void test_dust_threshold(void) {
1441+
TEST("psbt_output_dust_threshold: Core's per-script-type values");
1442+
1443+
const uint8_t p2wpkh[22] = {0x00, 0x14};
1444+
const uint8_t p2wsh[34] = {0x00, 0x20};
1445+
const uint8_t p2tr[34] = {0x51, 0x20};
1446+
const uint8_t p2pkh[25] = {0x76, 0xa9, 0x14};
1447+
const uint8_t p2sh[23] = {0xa9, 0x14};
1448+
const uint8_t op_return[] = {0x6a, 0x02, 0xde, 0xad};
1449+
1450+
if (psbt_output_dust_threshold(p2wpkh, sizeof(p2wpkh)) != 294)
1451+
FAIL("p2wpkh should be 294");
1452+
else if (psbt_output_dust_threshold(p2wsh, sizeof(p2wsh)) != 330)
1453+
FAIL("p2wsh should be 330");
1454+
else if (psbt_output_dust_threshold(p2tr, sizeof(p2tr)) != 330)
1455+
FAIL("p2tr should be 330");
1456+
else if (psbt_output_dust_threshold(p2pkh, sizeof(p2pkh)) != 546)
1457+
FAIL("p2pkh should be 546");
1458+
else if (psbt_output_dust_threshold(p2sh, sizeof(p2sh)) != 540)
1459+
FAIL("p2sh should be 540");
1460+
else if (psbt_output_dust_threshold(op_return, sizeof(op_return)) != 0)
1461+
FAIL("OP_RETURN is unspendable, never dust");
1462+
else if (psbt_output_dust_threshold(NULL, 0) != 0)
1463+
FAIL("no script should be 0");
1464+
else
1465+
PASS();
1466+
}
1467+
1468+
static void test_total_output_value(void) {
1469+
TEST("psbt_total_output_value: sums outputs at either PSBT version");
1470+
1471+
struct wally_tx *tx = NULL;
1472+
if (wally_tx_init_alloc(2, 0, 1, 2, &tx) != WALLY_OK) {
1473+
FAIL("tx_init_alloc");
1474+
return;
1475+
}
1476+
uint8_t txid[32] = {0xaa};
1477+
wally_tx_add_raw_input(tx, txid, sizeof(txid), 0, 0xffffffff, NULL, 0, NULL,
1478+
0);
1479+
wally_tx_add_raw_output(tx, 90000, REF_SPK_P2WPKH, sizeof(REF_SPK_P2WPKH), 0);
1480+
wally_tx_add_raw_output(tx, 5000, REF_SPK_P2WPKH, sizeof(REF_SPK_P2WPKH), 0);
1481+
1482+
struct wally_psbt *psbt = NULL;
1483+
int ret = wally_psbt_from_tx(tx, 0, 0, &psbt);
1484+
wally_tx_free(tx);
1485+
if (ret != WALLY_OK) {
1486+
FAIL("psbt_from_tx");
1487+
return;
1488+
}
1489+
1490+
uint64_t total = 0;
1491+
if (!psbt_total_output_value(psbt, &total))
1492+
FAIL("should read a v0 transaction");
1493+
else if (total != 95000)
1494+
FAIL("wrong v0 total");
1495+
else if (wally_psbt_set_version(psbt, 0, WALLY_PSBT_VERSION_2) != WALLY_OK)
1496+
FAIL("could not convert the fixture to v2");
1497+
else if (!psbt_total_output_value(psbt, &total))
1498+
FAIL("should read a v2 PSBT too");
1499+
else if (total != 95000)
1500+
FAIL("v2 total should match v0");
1501+
else
1502+
PASS();
1503+
1504+
wally_psbt_free(psbt);
1505+
}
1506+
14401507
/* An OWNED_SAFE single-sig fixture: p2wpkh input on the whitelisted BIP84
14411508
* path, so the signing policy clears it with no opt-in. */
14421509
static struct wally_psbt *make_safe_psbt(void) {
@@ -2438,6 +2505,8 @@ int main(void) {
24382505
test_sighash_audit_flags_input();
24392506
test_sign_refuses_unsupported_sighash();
24402507
test_fee_percent();
2508+
test_dust_threshold();
2509+
test_total_output_value();
24412510

24422511
printf("\n=== psbt_sign policy-gate tests ===\n\n");
24432512

main/pages/scan/psbt_sign_policy.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,31 @@ static bool reject_unbuildable_tx(struct wally_psbt *psbt,
236236
return true;
237237
}
238238

239+
/* sum(outputs) > sum(inputs) cannot happen on chain: the difference is the
240+
* fee and it is never negative. Either an input amount is understated or an
241+
* output amount is inflated, and both make every number on the review screen
242+
* a lie. Only decidable when each input actually supplied an amount -- a
243+
* MISSING one contributes 0, which produces the same shape honestly. */
244+
static bool reject_impossible_fee(struct wally_psbt *psbt,
245+
const psbt_amount_audit_t *audit,
246+
dialog_callback_t dismissed_cb) {
247+
uint64_t total_output = 0;
248+
if (audit->missing || !psbt_total_output_value(psbt, &total_output) ||
249+
total_output <= audit->total)
250+
return false;
251+
252+
char body[384];
253+
snprintf(body, sizeof(body),
254+
"The outputs add up to more than the inputs, which cannot happen: "
255+
"the difference between them is the fee and it is never "
256+
"negative.\n\n"
257+
"Inputs total %llu sats, outputs total %llu sats. An amount in this "
258+
"PSBT is wrong, so nothing it claims can be reviewed honestly.",
259+
(unsigned long long)audit->total, (unsigned long long)total_output);
260+
show_cannot_sign(body, dismissed_cb);
261+
return true;
262+
}
263+
239264
static bool reject_partial(const sign_policy_review_t *review,
240265
dialog_callback_t dismissed_cb) {
241266
if (!review->any_input_external || settings_get_partial_signing())
@@ -296,6 +321,8 @@ bool psbt_sign_policy_allows_review(struct wally_psbt *psbt, bool is_testnet,
296321
return false;
297322
if (reject_invalid_amount(&audit, dismissed_cb))
298323
return false;
324+
if (reject_impossible_fee(psbt, &audit, dismissed_cb))
325+
return false;
299326
if (reject_expected_owned(&review, dismissed_cb, load_descriptor_cb))
300327
return false;
301328
if (reject_permissive(&review, dismissed_cb))

main/pages/scan/scan.c

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ typedef struct {
6666
uint64_t value;
6767
char *address;
6868
uint32_t address_index;
69+
bool is_dust; /* below the relay dust threshold for its script type */
6970
char path[80]; /* populated for OWNED_UNSAFE / EXPECTED_OWNED */
7071
} classified_output_t;
7172

@@ -1199,6 +1200,16 @@ static bool create_psbt_info_display(void) {
11991200
for (size_t i = 0; i < num_outputs; i++) {
12001201
total_output_value += global_tx->outputs[i].satoshi;
12011202
}
1203+
1204+
/* Both are read off the global tx, which is freed before the notes are
1205+
* rendered. BIP-125 opts a transaction into replaceability when any input's
1206+
* sequence is below 0xfffffffe. */
1207+
uint32_t locktime = global_tx->locktime;
1208+
bool signals_rbf = false;
1209+
for (size_t i = 0; i < global_tx->num_inputs; i++) {
1210+
if (global_tx->inputs[i].sequence < 0xfffffffeu)
1211+
signals_rbf = true;
1212+
}
12021213
uint64_t fee = (total_input_value > total_output_value)
12031214
? (total_input_value - total_output_value)
12041215
: 0;
@@ -1229,6 +1240,10 @@ static bool create_psbt_info_display(void) {
12291240
classified_outputs[i].type = classify_output(
12301241
i, &classified_outputs[i].address_index, classified_outputs[i].path,
12311242
sizeof(classified_outputs[i].path));
1243+
classified_outputs[i].is_dust =
1244+
classified_outputs[i].value <
1245+
psbt_output_dust_threshold(global_tx->outputs[i].script,
1246+
global_tx->outputs[i].script_len);
12321247
}
12331248

12341249
size_t diagram_idx = 0;
@@ -1651,6 +1666,19 @@ static bool create_psbt_info_display(void) {
16511666
}
16521667
}
16531668

1669+
size_t dust_count = 0;
1670+
size_t first_dust = 0;
1671+
uint64_t first_dust_value = 0;
1672+
for (size_t i = 0; i < num_outputs; i++) {
1673+
if (!classified_outputs[i].is_dust)
1674+
continue;
1675+
if (!dust_count) {
1676+
first_dust = classified_outputs[i].index;
1677+
first_dust_value = classified_outputs[i].value;
1678+
}
1679+
dust_count++;
1680+
}
1681+
16541682
for (size_t i = 0; i < num_outputs; i++) {
16551683
if (classified_outputs[i].address) {
16561684
if (strcmp(classified_outputs[i].address, "OP_RETURN") == 0) {
@@ -1720,6 +1748,62 @@ static bool create_psbt_info_display(void) {
17201748
create_review_note(psbt_info_container, note, highlight_color());
17211749
}
17221750

1751+
/* The gate refuses a PSBT whose outputs exceed the inputs, so reaching here
1752+
* that way means an input never supplied an amount and was counted as zero.
1753+
* The unproven-fee note above already says the numbers are not backed; name
1754+
* the missing fee too, because no fee row at all otherwise reads as "no
1755+
* fee". */
1756+
if (total_output_value > total_input_value) {
1757+
create_review_note(psbt_info_container,
1758+
LV_SYMBOL_WARNING
1759+
" Fee unknown: the outputs exceed the input amounts "
1760+
"this PSBT supplied.",
1761+
error_color());
1762+
}
1763+
1764+
/* Below the relay dust threshold an output costs more to spend than it
1765+
* holds, so the transaction is unlikely to propagate at all. */
1766+
if (dust_count) {
1767+
char note[192];
1768+
if (dust_count == 1)
1769+
snprintf(note, sizeof(note),
1770+
LV_SYMBOL_WARNING " Dust: output %zu holds only %llu sats, "
1771+
"below the amount needed to relay.",
1772+
first_dust, (unsigned long long)first_dust_value);
1773+
else
1774+
snprintf(note, sizeof(note),
1775+
LV_SYMBOL_WARNING " Dust: %zu outputs are below the amount "
1776+
"needed to relay.",
1777+
dust_count);
1778+
create_review_note(psbt_info_container, note, highlight_color());
1779+
}
1780+
1781+
/* Neither is visible anywhere else on this screen, and both change what
1782+
* signing actually commits to: a future locktime is not broadcastable yet,
1783+
* and an RBF-signalling transaction can be replaced before it confirms. */
1784+
if (locktime) {
1785+
char note[160];
1786+
if (locktime < 500000000u)
1787+
snprintf(note, sizeof(note),
1788+
LV_SYMBOL_WARNING " Locked until block %" PRIu32
1789+
": not broadcastable before then.",
1790+
locktime);
1791+
else
1792+
snprintf(note, sizeof(note),
1793+
LV_SYMBOL_WARNING " Locked until unix time %" PRIu32
1794+
": not broadcastable before then.",
1795+
locktime);
1796+
create_review_note(psbt_info_container, note, highlight_color());
1797+
}
1798+
1799+
if (signals_rbf) {
1800+
create_review_note(psbt_info_container,
1801+
LV_SYMBOL_WARNING
1802+
" Replaceable (RBF): this transaction can be replaced "
1803+
"by a different one before it confirms.",
1804+
secondary_color());
1805+
}
1806+
17231807
create_sign_action_row(psbt_info_container, sign_button_cb);
17241808

17251809
return true;

0 commit comments

Comments
 (0)