From 74a6b2779d01cdde6c9a07027f3a3b420a166b95 Mon Sep 17 00:00:00 2001 From: Amperstrand Date: Sun, 30 Aug 2026 11:17:18 +0200 Subject: [PATCH 1/2] wallet: refuse (don't abort) when the emergency change cannot exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit change_for_emergency() promises the split-excess branch a change output that covers the (reduced) reserve requirement after its own fee. That promise can be hollow in two ways, both of which previously reached the equality assert and aborted lightningd: 1. The reserve shortfall itself is below the dust limit — wallet_has_funds() reduces `needed` to emergency_sat minus the unselected wallet, and when that shortfall is < dust_limit no change output can carry it: change_amount() dust-caps to 0, so assert(amount_sat_eq(change_amount(*change, ...), needed)) fails with needed > 0. (Deterministic repros on both RPC surfaces: test_utxopsbt_emergency_reserve_dust_shortfall and test_fundpsbt_emergency_reserve_dust_shortfall — a 60k-sat selected UTXO plus a 24_900-sat unselected output against a 25k reserve; the shortfall of 100 < dust 546 aborts the daemon on v26.06 and on current master. The fundpsbt variant forces the selection split with minconf=2.) 2. An entering change (excess_as_change=true sets change = excess before the call) makes the final change cover c0 + needed, not needed exactly — unsatisfiable for any c0 > 0. Observed live on v26.06 (production signet, 2026-08-29): five lightningd cores in five minutes — callers that retry the RPC crash-loop the daemon. Both are funding-availability corners: refuse with the caller's typed FUND_CANNOT_AFFORD_WITH_EMERGENCY instead of crashing. A property walk (random reserves, wallet granularities, rest-of-wallet shapes, asks) pins the invariants: daemon always survives, every call answers with a typed error or funds with a change covering the shortfall. Changelog-Fixed: wallet: fundpsbt/utxopsbt no longer abort lightningd (FATAL SIGNAL 6 in change_for_emergency) when the min-emergency-msat shortfall is below the dust limit or excess_as_change is set; they now fail with FUND_CANNOT_AFFORD_WITH_EMERGENCY. --- tests/test_wallet.py | 163 +++++++++++++++++++++++++++++++++++++++++++ wallet/reservation.c | 19 ++++- 2 files changed, 179 insertions(+), 3 deletions(-) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a69ed03f41dc..b0ec343345ef 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -2959,3 +2959,166 @@ def mock_fail_sendrawtx(r): bitcoind.generate_block(1) sync_blockheight(bitcoind, [l1]) assert l1.db_query('SELECT COUNT(*) as c FROM our_outputs WHERE spendheight IS NULL AND reserved_til = 0')[0]['c'] == 0 + + +def test_utxopsbt_emergency_reserve_property(node_factory, bitcoind, + chainparams): + """Randomized walk over the emergency-reserve funding path (the + crash class above): seeded and deterministic. Each round spins a + node with a random reserve, funds 1-3 selectable UTXOs plus a + rest-of-wallet UTXO near (or below) the reserve, then issues random + asks with/without excess_as_change. + + Invariants on EVERY call: + 1. the daemon survives (getinfo answers) — pre-fix this class + SIGABRTs whenever the shortfall is small and the excess lands + in the dust-capped window; + 2. every call either raises a typed RpcError or funds; + 3. oracle: when funding succeeded with excess_as_change and a + change output exists, that change must cover the remaining + shortfall (reserve minus the unselected wallet).""" + import random + rng = random.Random(9452) + rounds = int(os.getenv("EMERGENCY_PROPERTY_ROUNDS", "10")) + for i in range(rounds): + reserve_sat = rng.choice([10_000, 25_000, 50_000]) + utxo_sats = [rng.choice([30_000, 40_000, 50_000, 70_000, 100_000]) + for _ in range(rng.randint(1, 3))] + rest_sat = rng.choice([0, reserve_sat - rng.randint(0, 800), + reserve_sat - rng.randint(800, 5_000)]) + l1 = node_factory.get_node( + options={'min-emergency-msat': reserve_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + outpoints = [fund(s) for s in utxo_sats if s > 0] + if rest_sat > 0: + fund(rest_sat) # unselected: this is the window opener + bitcoind.generate_block(1) + expected = len(outpoints) + (1 if rest_sat > 0 else 0) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == expected) + total_sat = sum(utxo_sats) + + for j in range(3): + ask_hi = max(20_000, total_sat - reserve_sat - 3_000) + ask = rng.randint(15_000, max(15_001, ask_hi)) + excess_as_change = rng.random() < 0.7 + try: + f = l1.rpc.call('utxopsbt', { + 'satoshi': str(ask) + 'sat', 'feerate': '253perkw', + 'startweight': 100, 'utxos': outpoints, 'reserve': 0, + 'excess_as_change': excess_as_change, + 'opening_anchor_channel': True}) + except RpcError: + f = None + + # invariant 1: the daemon must still be answering + assert l1.rpc.getinfo()['id'], \ + "daemon died on round %d ask %d" % (i, j) + + if f is None or not excess_as_change \ + or 'change_outnum' not in f: + continue + + psbt = bitcoind.rpc.decodepsbt(f['psbt']) + outputs = psbt['outputs'] if chainparams['elements'] \ + else psbt['tx']['vout'] + change_val = int(Decimal( + str(outputs[f['change_outnum']]['value'])) * 10**8) + shortfall = max(0, reserve_sat - rest_sat) + assert change_val >= shortfall, ( + "change %d below the %d shortfall (round %d ask %d, " + "rest %d)" % (change_val, shortfall, i, j, rest_sat)) + + l1.stop() + + +def test_utxopsbt_emergency_reserve_dust_shortfall(node_factory, bitcoind, + chainparams): + """change_for_emergency must not abort when the reserve shortfall is + below the dust limit (crash observed live on v26.06: five lightningd + cores 2026-08-29, crash-loop on retry). + + The window, exactly: wallet_has_funds() reduces `needed` to the + shortfall (emergency_sat minus the UNSELECTED wallet). When that + shortfall is below the dust limit, the change output the split + branch promises cannot exist (change_amount() dust-caps it to 0), + and the old equality assert + + assert(amount_sat_eq(change_amount(*change, ...), needed)) + + fails -> FATAL SIGNAL 6. Note excess_as_change is irrelevant here + (it actually prevents the crash by zeroing the split's excess and + taking the typed 313 path); the crash combo is a plain call against + a wallet whose unselected part sits within dust of the reserve. + + Wallet shape: selected 60k-sat UTXO, rest 24_900 sat against a 25k + reserve (shortfall 100 < dust 546). Pre-fix the daemon SIGABRTs on + this call; post-fix it refuses with the typed + FUND_CANNOT_AFFORD_WITH_EMERGENCY (313) and the daemon survives. + """ + emergency_sat = 25_000 + rest_sat = 24_900 + l1 = node_factory.get_node( + options={'min-emergency-msat': emergency_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + selected = fund(60_000) + fund(rest_sat) # stays UNSELECTED: shortfall = 100 < dust + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + + with pytest.raises(RpcError, match='min-emergency-msat'): + l1.rpc.call('utxopsbt', { + 'satoshi': '59500sat', 'feerate': '253perkw', + 'startweight': 100, 'utxos': [selected], 'reserve': 0, + 'excess_as_change': False, 'opening_anchor_channel': True}) + + # the daemon survived the call + assert l1.rpc.getinfo()['id'] + +def test_fundpsbt_emergency_reserve_dust_shortfall(node_factory, bitcoind, + chainparams): + """Same crash class as test_utxopsbt_emergency_reserve_dust_shortfall, + reached through fundpsbt's own coin selection: CLN must select the + single sufficient UTXO, leave the near-reserve output unselected + (shortfall < dust), and previously aborted in change_for_emergency. + Post-fix: typed 313, daemon alive.""" + emergency_sat = 25_000 + l1 = node_factory.get_node( + options={'min-emergency-msat': emergency_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + # rest-vs-selection split via minconf: the 60k output gets two + # confirmations, the 24.9k rest only one — fundpsbt(minconf=2) + # can select ONLY the 60k UTXO, while wallet_has_funds counts both + # (shortfall 100 < dust). Two same-depth outputs would let the + # selector take both and mask the window. + fund(60_000) + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1) + fund(24_900) + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + + with pytest.raises(RpcError, match='min-emergency-msat'): + l1.rpc.call('fundpsbt', { + 'satoshi': '59500sat', 'feerate': '253perkw', + 'startweight': 100, 'reserve': 0, 'minconf': 2, + 'excess_as_change': False, 'opening_anchor_channel': True}) + + assert l1.rpc.getinfo()['id'] diff --git a/wallet/reservation.c b/wallet/reservation.c index f268998cb268..46c00f9fbb1a 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -477,9 +477,22 @@ static bool change_for_emergency(struct lightningd *ld, || !amount_sat_add(change, *change, needed)) abort(); - /* We *will* get a change output now! */ - assert(amount_sat_eq(change_amount(*change, feerate_per_kw, weight), - needed)); + /* We promise a change output that covers `needed` after its own + * fee. Two reasons that promise can be hollow, both of which + * previously hit the equality assert below and aborted lightningd: + * + * 1. the reserve shortfall itself is below the dust limit (the + * unselected wallet sits within dust of the reserve) — no + * change output can carry it, change_amount() dust-caps to 0; + * 2. an entering change (excess_as_change) makes the final change + * cover c0 + needed, not needed exactly. + * + * Both are funding-availability corners: refuse with the caller's + * typed FUND_CANNOT_AFFORD_WITH_EMERGENCY instead of crashing the + * daemon (observed live on v26.06: crash-loop, five cores). */ + if (amount_sat_less(change_amount(*change, feerate_per_kw, weight), + needed)) + return false; return true; } From 7772c518469a991873e7ea2854f3cf75b1454b1d Mon Sep 17 00:00:00 2001 From: Amperstrand Date: Tue, 1 Sep 2026 14:48:12 +0200 Subject: [PATCH 2/2] wallet: round the emergency change up to a viable output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit change_for_emergency()'s split branch moved exactly fee + shortfall from the excess into the change. When the shortfall is below the chain's minimum viable change output (330 sat P2TR on Bitcoin), the promised output cannot exist: change_amount() dust-caps it to zero. 9455 turned the resulting daemon abort into a typed FUND_CANNOT_AFFORD_WITH_EMERGENCY — but that refuses funding even when the transaction excess could afford a LARGER, viable change output that satisfies the reserve. min-emergency-msat is a floor on what the wallet retains, not an exact change-output target. The split branch now targets the smallest viable change: max(entering change + shortfall, min_change_amount()), plus the change output fee, taken from the excess. If the excess cannot cover it, the caller reports the typed error as before. For shortfalls at or above the minimum viable output the arithmetic is identical to the previous code; only the sub-dust corner changes (from refusal/crash to funding with a rounded-up change). The dust floor moves into min_change_amount() (used by change_amount() as well) so the two cannot drift. Note it must be constructed with amount_sat(), not the AMOUNT_SAT() macro: the macro poisons runtime (non-compile-constant) arguments by -2 via its BUILD_ASSERT_OR_ZERO machinery, silently. Tests: the dust-shortfall fixtures now assert the roundup funds (exact 330-sat change, exact leftover excess, reserve invariant, both RPC surfaces) including the exactly-affordable boundary; a new insufficient-excess test pins the typed refusal one sat short; new tests pin the above-dust and existing-change-covers paths; the property walk's oracle now covers every successful call (change output must exist and cover the shortfall whenever the unselected wallet is below the reserve). Changelog-Fixed: wallet: fundpsbt/utxopsbt now succeed (instead of failing with FUND_CANNOT_AFFORD_WITH_EMERGENCY) when the min-emergency-msat shortfall is below the dust limit but the transaction excess can afford a minimum-size viable change output. --- bitcoin/tx.c | 17 ++-- bitcoin/tx.h | 8 ++ tests/test_wallet.py | 221 ++++++++++++++++++++++++++++++++++++------- wallet/reservation.c | 54 ++++++----- 4 files changed, 233 insertions(+), 67 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index dabd5255619d..31a42b0b438f 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -977,6 +977,14 @@ struct amount_sat change_fee(u32 feerate_perkw, size_t total_weight) return fee; } +struct amount_sat min_change_amount(void) +{ + /* Dust limit for the change output's script type (P2TR on + * Bitcoin, P2WPKH on Elements). amount_sat() not AMOUNT_SAT(): + * the latter demands a compile-time constant. */ + return amount_sat(chainparams->is_elements ? 546 : 330); +} + struct amount_sat change_amount(struct amount_sat excess, u32 feerate_perkw, size_t total_weight) { @@ -985,13 +993,8 @@ struct amount_sat change_amount(struct amount_sat excess, u32 feerate_perkw, if (!amount_sat_sub(&excess, excess, fee)) return AMOUNT_SAT(0); - if (chainparams->is_elements) { - if (!amount_sat_greater_eq(excess, AMOUNT_SAT(546))) - return AMOUNT_SAT(0); - } else { - if (!amount_sat_greater_eq(excess, AMOUNT_SAT(330))) - return AMOUNT_SAT(0); - } + if (!amount_sat_greater_eq(excess, min_change_amount())) + return AMOUNT_SAT(0); return excess; } diff --git a/bitcoin/tx.h b/bitcoin/tx.h index 1e28130d8a97..c982ba6d3c9f 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -349,6 +349,14 @@ size_t change_weight(void); */ struct amount_sat change_fee(u32 feerate_perkw, size_t total_weight); +/** + * min_change_amount - the minimum amount a change output can carry. + * + * Change script is P2TR for Bitcoin, P2WPKH for Elements: below these + * amounts change_amount() dust-caps the change to zero. + */ +struct amount_sat min_change_amount(void); + /** * change_amount - Is it worth making a change output at this feerate? * @excess: input amount we have above the tx fee and other outputs. diff --git a/tests/test_wallet.py b/tests/test_wallet.py index b0ec343345ef..a4cc3fdede11 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -2974,9 +2974,11 @@ def test_utxopsbt_emergency_reserve_property(node_factory, bitcoind, SIGABRTs whenever the shortfall is small and the excess lands in the dust-capped window; 2. every call either raises a typed RpcError or funds; - 3. oracle: when funding succeeded with excess_as_change and a - change output exists, that change must cover the remaining - shortfall (reserve minus the unselected wallet).""" + 3. reserve oracle: whenever funding succeeded and the unselected + wallet is below the reserve, a change output MUST exist and + cover the remaining shortfall (reserve minus the unselected + wallet) — min-emergency-msat is retained either in the rest + of the wallet or as usable change.""" import random rng = random.Random(9452) rounds = int(os.getenv("EMERGENCY_PROPERTY_ROUNDS", "10")) @@ -3020,16 +3022,19 @@ def fund(sats): assert l1.rpc.getinfo()['id'], \ "daemon died on round %d ask %d" % (i, j) - if f is None or not excess_as_change \ - or 'change_outnum' not in f: + # invariant 3: success with a below-reserve unselected + # wallet implies a change output covering the shortfall + shortfall = max(0, reserve_sat - rest_sat) + if f is None or shortfall == 0: continue - + assert 'change_outnum' in f, ( + "funded below-reserve wallet with no change output " + "(round %d ask %d, rest %d)" % (i, j, rest_sat)) psbt = bitcoind.rpc.decodepsbt(f['psbt']) outputs = psbt['outputs'] if chainparams['elements'] \ else psbt['tx']['vout'] change_val = int(Decimal( str(outputs[f['change_outnum']]['value'])) * 10**8) - shortfall = max(0, reserve_sat - rest_sat) assert change_val >= shortfall, ( "change %d below the %d shortfall (round %d ask %d, " "rest %d)" % (change_val, shortfall, i, j, rest_sat)) @@ -3039,28 +3044,31 @@ def fund(sats): def test_utxopsbt_emergency_reserve_dust_shortfall(node_factory, bitcoind, chainparams): - """change_for_emergency must not abort when the reserve shortfall is - below the dust limit (crash observed live on v26.06: five lightningd - cores 2026-08-29, crash-loop on retry). + """A reserve shortfall below the dust limit must neither abort the + daemon (the pre-9455 crash: five lightningd cores on v26.06, + 2026-08-29) nor falsely refuse funding when the transaction excess + can afford a viable (rounded-up) change output. The window, exactly: wallet_has_funds() reduces `needed` to the - shortfall (emergency_sat minus the UNSELECTED wallet). When that - shortfall is below the dust limit, the change output the split - branch promises cannot exist (change_amount() dust-caps it to 0), - and the old equality assert - - assert(amount_sat_eq(change_amount(*change, ...), needed)) - - fails -> FATAL SIGNAL 6. Note excess_as_change is irrelevant here - (it actually prevents the crash by zeroing the split's excess and - taking the typed 313 path); the crash combo is a plain call against - a wallet whose unselected part sits within dust of the reserve. - - Wallet shape: selected 60k-sat UTXO, rest 24_900 sat against a 25k - reserve (shortfall 100 < dust 546). Pre-fix the daemon SIGABRTs on - this call; post-fix it refuses with the typed - FUND_CANNOT_AFFORD_WITH_EMERGENCY (313) and the daemon survives. + shortfall (emergency_sat minus the UNSELECTED wallet). A shortfall + of 100 sat cannot ride in its own change output (change_amount() + dust-caps anything under 330 sat post-fee on Bitcoin), but the + reserve is a FLOOR on what the wallet retains, not an exact + change-output target: if the excess covers the smallest viable + change (330 sat) plus its output fee, funding should succeed and + leave the wallet above the reserve. + + Exact arithmetic for this fixture (feerate 253perkw, startweight + 100, one P2WPKH input = 271 weight): + total weight 371 -> base fee 93 -> excess = 60000-59500-93 = 407 + change output (P2TR, 172 weight) fee = 44 + roundup target = max(shortfall 100, min change 330) + 44 = 374 + 407 >= 374 -> funds with a 330-sat change, 33 sat excess left, + and the wallet retains 24900 + 330 = 25230 >= 25000. """ + if chainparams['elements']: + pytest.skip("exact roundup fixtures are Bitcoin-P2TR-tuned; " + "elements floor/weights differ (property test covers it)") emergency_sat = 25_000 rest_sat = 24_900 l1 = node_factory.get_node( @@ -3073,26 +3081,155 @@ def fund(sats): return '{}:{}'.format(txid, vout) selected = fund(60_000) - fund(rest_sat) # stays UNSELECTED: shortfall = 100 < dust + fund(rest_sat) # stays UNSELECTED: shortfall = 100 < min change + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + + def utxopsbt(sats): + return l1.rpc.call('utxopsbt', { + 'satoshi': str(sats) + 'sat', 'feerate': '253perkw', + 'startweight': 100, 'utxos': [selected], 'reserve': 0, + 'excess_as_change': False, 'opening_anchor_channel': True}) + + # Exactly-affordable boundary: excess 374 == roundup cost 374 + f = utxopsbt(59_533) + assert f['change_outnum'] is not None + assert f['excess_msat'] == Millisatoshi(0) + + # One sat of slack: excess 407, roundup cost 374 -> 33 left over + f = utxopsbt(59_500) + psbt = bitcoind.rpc.decodepsbt(f['psbt']) + change_val = Millisatoshi("{}btc".format( + psbt['tx']['vout'][f['change_outnum']]['value'])) + assert change_val == Millisatoshi(330_000) + assert f['excess_msat'] == Millisatoshi(33_000) + assert f['estimated_final_weight'] == 100 + 271 + 172 + # the reserve invariant, checked where it lives: unselected wallet + # plus usable change must cover the emergency reserve + assert Millisatoshi(rest_sat * 1000) + change_val >= Millisatoshi(emergency_sat * 1000) + + # the daemon survived the calls + assert l1.rpc.getinfo()['id'] + + +def test_utxopsbt_emergency_reserve_insufficient_excess(node_factory, + bitcoind, + chainparams): + """Same sub-dust shortfall as the roundup test, but with the excess + one sat too small to create a viable change output (excess 367 < + roundup cost 374): funding must fail with the typed + FUND_CANNOT_AFFORD_WITH_EMERGENCY, and the daemon must survive + (this is the corner that SIGABRTed lightningd pre-9455).""" + emergency_sat = 25_000 + l1 = node_factory.get_node( + options={'min-emergency-msat': emergency_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + selected = fund(60_000) + fund(24_900) # unselected: shortfall = 100, below any dust floor bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) with pytest.raises(RpcError, match='min-emergency-msat'): l1.rpc.call('utxopsbt', { - 'satoshi': '59500sat', 'feerate': '253perkw', + 'satoshi': '59540sat', 'feerate': '253perkw', 'startweight': 100, 'utxos': [selected], 'reserve': 0, 'excess_as_change': False, 'opening_anchor_channel': True}) # the daemon survived the call assert l1.rpc.getinfo()['id'] + +def test_utxopsbt_emergency_reserve_above_dust_shortfall(node_factory, + bitcoind, + chainparams): + """A shortfall at or above the minimum viable change output needs + no rounding: the change output carries exactly the shortfall, as + before the roundup change (regression pin for the healthy path).""" + if chainparams['elements']: + pytest.skip("exact fixtures are Bitcoin-P2TR-tuned") + emergency_sat = 25_000 + rest_sat = 24_600 # shortfall = 400 >= 330: viable as-is + l1 = node_factory.get_node( + options={'min-emergency-msat': emergency_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + selected = fund(60_000) + fund(rest_sat) + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + + # excess = 60000-59000-93 = 907; cost = 400 shortfall + 44 fee = 444 + f = l1.rpc.call('utxopsbt', { + 'satoshi': '59000sat', 'feerate': '253perkw', + 'startweight': 100, 'utxos': [selected], 'reserve': 0, + 'excess_as_change': False, 'opening_anchor_channel': True}) + psbt = bitcoind.rpc.decodepsbt(f['psbt']) + change_val = Millisatoshi("{}btc".format( + psbt['tx']['vout'][f['change_outnum']]['value'])) + assert change_val == Millisatoshi(400_000) + assert f['excess_msat'] == Millisatoshi(463_000) + assert Millisatoshi(rest_sat * 1000) + change_val >= Millisatoshi(emergency_sat * 1000) + assert l1.rpc.getinfo()['id'] + + +def test_utxopsbt_emergency_reserve_existing_change_covers(node_factory, + bitcoind, + chainparams): + """When the entering change (excess_as_change=true) already covers + the shortfall after its own fee, no additional sats are moved: + the change output keeps exactly its excess-derived value.""" + if chainparams['elements']: + pytest.skip("exact fixtures are Bitcoin-P2TR-tuned") + emergency_sat = 25_000 + l1 = node_factory.get_node( + options={'min-emergency-msat': emergency_sat * 1000}) + + def fund(sats): + addr = l1.rpc.newaddr('bech32')['bech32'] + txid = bitcoind.rpc.sendtoaddress(addr, sats / 10**8) + vout = bitcoind.rpc.gettransaction(txid)['details'][0]['vout'] + return '{}:{}'.format(txid, vout) + + selected = fund(60_000) + fund(24_900) # shortfall = 100 + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + + # diff = 60000-59407-93 = 500 -> post-fee change 456 >= shortfall + # 100: covered, untouched (no top-up of fee+shortfall on top) + f = l1.rpc.call('utxopsbt', { + 'satoshi': '59407sat', 'feerate': '253perkw', + 'startweight': 100, 'utxos': [selected], 'reserve': 0, + 'excess_as_change': True, 'opening_anchor_channel': True}) + psbt = bitcoind.rpc.decodepsbt(f['psbt']) + change_val = Millisatoshi("{}btc".format( + psbt['tx']['vout'][f['change_outnum']]['value'])) + assert change_val == Millisatoshi(456_000) + assert f['excess_msat'] == Millisatoshi(0) + assert l1.rpc.getinfo()['id'] + + def test_fundpsbt_emergency_reserve_dust_shortfall(node_factory, bitcoind, chainparams): - """Same crash class as test_utxopsbt_emergency_reserve_dust_shortfall, - reached through fundpsbt's own coin selection: CLN must select the - single sufficient UTXO, leave the near-reserve output unselected - (shortfall < dust), and previously aborted in change_for_emergency. - Post-fix: typed 313, daemon alive.""" + """Same dust-shortfall window as the utxopsbt test, reached through + fundpsbt's own coin selection: CLN must select the single + sufficient UTXO, leave the near-reserve output unselected, and + (post-roundup-fix) fund successfully by rounding the emergency + change up to a viable output — with the insufficient-excess ask + still failing with the typed 313.""" + if chainparams['elements']: + pytest.skip("exact roundup fixtures are Bitcoin-P2TR-tuned") emergency_sat = 25_000 l1 = node_factory.get_node( options={'min-emergency-msat': emergency_sat * 1000}) @@ -3106,8 +3243,8 @@ def fund(sats): # rest-vs-selection split via minconf: the 60k output gets two # confirmations, the 24.9k rest only one — fundpsbt(minconf=2) # can select ONLY the 60k UTXO, while wallet_has_funds counts both - # (shortfall 100 < dust). Two same-depth outputs would let the - # selector take both and mask the window. + # (shortfall 100 < min change). Two same-depth outputs would let + # the selector take both and mask the window. fund(60_000) bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1) @@ -3115,9 +3252,21 @@ def fund(sats): bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2) + # roundup: excess 407 >= cost 374 -> funds with a 330-sat change + f = l1.rpc.call('fundpsbt', { + 'satoshi': '59500sat', 'feerate': '253perkw', + 'startweight': 100, 'reserve': 0, 'minconf': 2, + 'excess_as_change': False, 'opening_anchor_channel': True}) + psbt = bitcoind.rpc.decodepsbt(f['psbt']) + change_val = Millisatoshi("{}btc".format( + psbt['tx']['vout'][f['change_outnum']]['value'])) + assert change_val == Millisatoshi(330_000) + assert f['excess_msat'] == Millisatoshi(33_000) + + # one sat short of a viable change: typed refusal, daemon alive with pytest.raises(RpcError, match='min-emergency-msat'): l1.rpc.call('fundpsbt', { - 'satoshi': '59500sat', 'feerate': '253perkw', + 'satoshi': '59540sat', 'feerate': '253perkw', 'startweight': 100, 'reserve': 0, 'minconf': 2, 'excess_as_change': False, 'opening_anchor_channel': True}) diff --git a/wallet/reservation.c b/wallet/reservation.c index 46c00f9fbb1a..e3185e5c7efc 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -446,7 +446,7 @@ static bool change_for_emergency(struct lightningd *ld, struct amount_sat *excess, struct amount_sat *change) { - struct amount_sat needed = ld->emergency_sat, fee; + struct amount_sat needed = ld->emergency_sat, fee, target; /* Only needed for anchor channels */ if (!have_anchor_channel) @@ -467,32 +467,38 @@ static bool change_for_emergency(struct lightningd *ld, needed)) return true; - /* Try splitting excess to add to change. */ - fee = change_fee(feerate_per_kw, weight); - if (!amount_sat_sub(excess, *excess, fee) - || !amount_sat_sub(excess, *excess, needed)) + /* We top the change up from excess so that, after paying its own + * fee, the change output carries at least the reserve shortfall + * `needed`. But change_amount() dust-caps any change whose + * post-fee value is under the chain's minimum viable output, so if + * the natural target (entering change + shortfall) is below that + * minimum, round UP to it: min-emergency-msat is a floor on what + * the wallet retains, not an exact change-output target. (The + * pre-rounding code crashed lightningd here: a shortfall below the + * dust limit made the promised change output unconstructible.) */ + target = *change; + if (!amount_sat_add(&target, target, needed)) return false; + if (amount_sat_less(target, min_change_amount())) + target = min_change_amount(); - if (!amount_sat_add(change, *change, fee) - || !amount_sat_add(change, *change, needed)) - abort(); - - /* We promise a change output that covers `needed` after its own - * fee. Two reasons that promise can be hollow, both of which - * previously hit the equality assert below and aborted lightningd: - * - * 1. the reserve shortfall itself is below the dust limit (the - * unselected wallet sits within dust of the reserve) — no - * change output can carry it, change_amount() dust-caps to 0; - * 2. an entering change (excess_as_change) makes the final change - * cover c0 + needed, not needed exactly. - * - * Both are funding-availability corners: refuse with the caller's - * typed FUND_CANNOT_AFFORD_WITH_EMERGENCY instead of crashing the - * daemon (observed live on v26.06: crash-loop, five cores). */ - if (amount_sat_less(change_amount(*change, feerate_per_kw, weight), - needed)) + /* The excess must cover the change itself plus the fee for adding + * the output, minus whatever change already holds. If it can't, + * that's a funding-availability corner: the caller reports the + * typed FUND_CANNOT_AFFORD_WITH_EMERGENCY, never an assert. */ + fee = change_fee(feerate_per_kw, weight); + if (!amount_sat_add(&target, target, fee)) return false; + if (!amount_sat_sub(&target, target, *change)) + return false; + if (!amount_sat_sub(excess, *excess, target)) + return false; + if (!amount_sat_add(change, *change, target)) + return false; + + /* The change now carries >= min_change_amount() + fee, so + * change_amount() will not dust-cap it, and its post-fee value + * covers `needed`: the wallet keeps its emergency reserve. */ return true; }