From bc6c380101c266a2c29e4b53aa774f747f6e0dee Mon Sep 17 00:00:00 2001 From: Kyle Santiago Date: Fri, 30 Oct 2026 00:16:50 -0400 Subject: [PATCH 1/2] wallet: refuse to add to a PSBT that does not allow it A version 2 PSBT without PSBT_GLOBAL_TX_MODIFIABLE, or without its inputs or outputs bit, makes libwally refuse to add one, and psbt_add_output and psbt_append_input assert on that. A signer clears the bits once adding would invalidate its signature, so refuse rather than override them. Changelog-Fixed: `addpsbtoutput` and `addpsbtinput` refuse a version 2 PSBT that does not allow adding outputs or inputs, instead of crashing lightningd. --- tests/test_wallet.py | 32 +++++++++++++++++++++++++ tests/utils.py | 57 ++++++++++++++++++++++++++++++++++++++++++++ wallet/reservation.c | 14 +++++++++++ 3 files changed, 103 insertions(+) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a69ed03f41dc..0d1e162ebc12 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -7,6 +7,7 @@ only_one, wait_for, sync_blockheight, mine_funding_to_announce, VALGRIND, check_coin_moves, TailableProc, scriptpubkey_addr, check_utxos_channel, check_feerate, did_short_sig, first_scid, + psbt_maps, psbt_join, ) import os @@ -811,6 +812,37 @@ def test_addpsbtoutput(node_factory, bitcoind, chainparams): assert psbt_info['tx']['vout'][n]['scriptPubKey']['address'] == dest +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +def test_addpsbt_unmodifiable(node_factory, bitcoind): + """A version 2 PSBT without PSBT_GLOBAL_TX_MODIFIABLE does not allow + adding inputs or outputs.""" + l1 = node_factory.get_node() + l1.fundwallet(10**6) + glob, inputs, outputs = psbt_maps(l1.rpc.addpsbtoutput(100000)['psbt']) + assert any(k == b'\x06' for k, _ in glob) + psbt = psbt_join([(k, v) for k, v in glob if k != b'\x06'], inputs, outputs) + + with pytest.raises(RpcError, match='PSBT does not allow adding outputs'): + l1.rpc.addpsbtoutput(100000, psbt) + with pytest.raises(RpcError, match='PSBT does not allow adding inputs'): + l1.rpc.call('addpsbtinput', {'satoshi': 200000, 'initialpsbt': psbt}) + + # Either flag alone allows only that. + def with_flags(flags): + return psbt_join([(k, v) for k, v in glob if k != b'\x06'] + [(b'\x06', bytes([flags]))], + inputs, outputs) + + inputs_only = with_flags(1) + l1.rpc.call('addpsbtinput', {'satoshi': 200000, 'initialpsbt': inputs_only, 'reserve': 0}) + with pytest.raises(RpcError, match='PSBT does not allow adding outputs'): + l1.rpc.addpsbtoutput(100000, inputs_only) + + outputs_only = with_flags(2) + l1.rpc.addpsbtoutput(100000, outputs_only) + with pytest.raises(RpcError, match='PSBT does not allow adding inputs'): + l1.rpc.call('addpsbtinput', {'satoshi': 200000, 'initialpsbt': outputs_only}) + + def test_utxopsbt(node_factory, bitcoind, chainparams): amount = 1000000 l1 = node_factory.get_node() diff --git a/tests/utils.py b/tests/utils.py index 8647c16e6436..d9d0d11b2dde 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,5 +1,6 @@ from pyln.testing.utils import TEST_NETWORK, TIMEOUT, VALGRIND, DEPRECATED_APIS # noqa: F401 from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum, mine_funding_to_announce, scid_to_int # noqa: F401 +import base64 import bitstring from pyln.client import Millisatoshi from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND @@ -683,3 +684,59 @@ def check_feerate(nodes, actual_feerate, expected_feerate): return # Use assert as it shows the actual values on failure assert actual_feerate < expected_feerate + 2 + + +def _psbt_read_compact(b, i): + n = b[i] + if n < 0xfd: + return n, i + 1 + size = {0xfd: 2, 0xfe: 4, 0xff: 8}[n] + return int.from_bytes(b[i + 1:i + 1 + size], 'little'), i + 1 + size + + +def _psbt_compact(n): + if n < 0xfd: + return bytes([n]) + if n <= 0xffff: + return b'\xfd' + n.to_bytes(2, 'little') + if n <= 0xffffffff: + return b'\xfe' + n.to_bytes(4, 'little') + return b'\xff' + n.to_bytes(8, 'little') + + +def psbt_maps(psbt): + """Split a base64 version 2 PSBT into its global, input and output + maps, each a list of (key, value).""" + b = base64.b64decode(psbt) + assert b[:5] == b'psbt\xff' + i = 5 + maps = [] + cur = [] + counts = {} + while True: + klen, i = _psbt_read_compact(b, i) + if klen == 0: + maps.append(cur) + cur = [] + if len(maps) == 1 + counts[0x04] + counts[0x05]: + break + continue + key = b[i:i + klen] + i += klen + vlen, i = _psbt_read_compact(b, i) + val = b[i:i + vlen] + i += vlen + if not maps and key[0] in (0x04, 0x05): + counts[key[0]], _ = _psbt_read_compact(val, 0) + cur.append((key, val)) + return maps[0], maps[1:1 + counts[0x04]], maps[1 + counts[0x04]:] + + +def psbt_join(glob, inputs, outputs): + """The inverse of psbt_maps.""" + out = b'psbt\xff' + for m in [glob] + inputs + outputs: + for k, v in m: + out += _psbt_compact(len(k)) + k + _psbt_compact(len(v)) + v + out += b'\x00' + return base64.b64encode(out).decode() diff --git a/wallet/reservation.c b/wallet/reservation.c index f268998cb268..d53342ede384 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -686,6 +686,13 @@ static struct command_result *json_addpsbtoutput(struct command *cmd, FUNDING_PSBT_INVALID, "PSBT failed to validate."); + /* Without this flag, adding an output may invalidate a signature + * already made. */ + if (psbt->version == 2 + && !(psbt->tx_modifiable_flags & WALLY_PSBT_TXMOD_OUTPUTS)) + return command_fail(cmd, FUNDING_PSBT_INVALID, + "PSBT does not allow adding outputs"); + if (amount_sat_less(*amount, chainparams->dust_limit)) return command_fail(cmd, FUND_OUTPUT_IS_DUST, "Receive amount is below dust limit (%s)", @@ -797,6 +804,13 @@ static struct command_result *json_addpsbtinput(struct command *cmd, FUNDING_PSBT_INVALID, "PSBT failed to validate."); + /* Without this flag, adding an input may invalidate a signature + * already made. */ + if (psbt->version == 2 + && !(psbt->tx_modifiable_flags & WALLY_PSBT_TXMOD_INPUTS)) + return command_fail(cmd, FUNDING_PSBT_INVALID, + "PSBT does not allow adding inputs"); + if (!req_amount || amount_sat_is_zero(*req_amount)) return command_fail(cmd, FUND_INPUT_IS_ZERO, "Cannot add input" " value of zero."); From d9cb0085dd13192b6d0f70c99c12641ab9946c9b Mon Sep 17 00:00:00 2001 From: Kyle Santiago Date: Fri, 30 Oct 2026 00:16:50 -0400 Subject: [PATCH 2/2] bitcoin: refuse a PSBT whose inputs leave it no valid locktime One input requiring a time locktime and another a height locktime leave a version 2 PSBT no valid locktime, and psbt_txid asserts when it cannot extract the transaction. Refuse such a PSBT when parsing it, which covers every RPC parameter and plugin reply. Changelog-Fixed: A PSBT whose inputs require both a time and a height locktime is refused, instead of crashing lightningd in `fundchannel_complete`. --- bitcoin/psbt.c | 8 ++++++++ tests/test_connection.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index 10dada97edea..1e9197438f55 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -870,6 +870,7 @@ struct wally_psbt *psbt_from_b64(const tal_t *ctx, size_t b64len) { struct wally_psbt *psbt; + size_t locktime; tal_wally_start(); if (wally_psbt_from_base64_n(b64, b64len, /* flags */ 0, &psbt) == WALLY_OK) @@ -878,6 +879,13 @@ struct wally_psbt *psbt_from_b64(const tal_t *ctx, psbt = NULL; tal_wally_end_onto(ctx, psbt, struct wally_psbt); + /* One input requiring a time locktime and another a height locktime + * leave the transaction no valid locktime, and psbt_txid aborts on + * it. */ + if (psbt && psbt->version == 2 + && wally_psbt_get_locktime(psbt, &locktime) != WALLY_OK) + return tal_free(psbt); + return psbt; } diff --git a/tests/test_connection.py b/tests/test_connection.py index c1dd24f779c0..b3d0c9bcb2c3 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -13,7 +13,7 @@ check_coin_moves, first_channel_id, account_balance, basic_fee, scriptpubkey_addr, default_ln_port, mine_funding_to_announce, first_scid, - CHANNEL_SIZE + CHANNEL_SIZE, psbt_maps, psbt_join, ) from pyln.testing.utils import VALGRIND, ASAN, EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT, RUST, SLOW_MACHINE @@ -1850,6 +1850,34 @@ def test_fundchannel_complete_rejects_nonsegwit_unsigned(node_factory, bitcoind) l1.rpc.fundchannel_cancel(l2.info['id']) +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +@pytest.mark.openchannel('v1') +def test_fundchannel_complete_conflicting_locktimes(node_factory, bitcoind): + """One input requiring a time locktime and another a height locktime + leave the funding transaction no valid locktime.""" + l1, l2 = node_factory.get_nodes(2) + l1.fundwallet(10**6) + l1.fundwallet(10**6) + l1.connect(l2) + + amount = 1_500_000 + start = l1.rpc.fundchannel_start(l2.info['id'], amount) + psbt = l1.rpc.fundpsbt(amount, '253perkw', 250, reserve=0)['psbt'] + psbt = l1.rpc.setpsbtversion(psbt, 2)['psbt'] + psbt = l1.rpc.addpsbtoutput(amount, psbt, + destination=start['funding_address'])['psbt'] + glob, inputs, outputs = psbt_maps(psbt) + assert len(inputs) == 2 + inputs[0].append((b'\x11', struct.pack('