Skip to content

Commit e2f5728

Browse files
vincenzopalazzorustyrussell
authored andcommitted
fix: release txprepare reservations on send failure
Clean up the original PSBT reservation when withdraw/txsend fail after signing or broadcast. Include the final withdraw regression coverage, including the liquid-safe address handling needed for CI. Fixes #8925
1 parent 2346d37 commit e2f5728

2 files changed

Lines changed: 117 additions & 3 deletions

File tree

plugins/txprepare.c

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,45 @@ static struct wally_psbt *json_tok_psbt(const tal_t *ctx,
5050
return psbt_from_b64(ctx, buffer + tok->start, tok->end - tok->start);
5151
}
5252

53+
struct txprepare_cleanup {
54+
char *error_json;
55+
};
56+
57+
static struct command_result *
58+
txprepare_cleanup_done(struct command *cmd,
59+
const char *method UNUSED,
60+
const char *buf UNUSED,
61+
const jsmntok_t *result UNUSED,
62+
struct txprepare_cleanup *cleanup)
63+
{
64+
return command_err_raw(cmd, cleanup->error_json);
65+
}
66+
67+
/* Immediate send flows must drop the original PSBT input reservation. */
68+
static struct command_result *
69+
txprepare_forward_error(struct command *cmd,
70+
const char *method,
71+
const char *buf,
72+
const jsmntok_t *error,
73+
struct unreleased_tx *utx)
74+
{
75+
struct out_req *req;
76+
struct txprepare_cleanup *cleanup;
77+
78+
if (!utx->psbt)
79+
return forward_error(cmd, method, buf, error, NULL);
80+
81+
cleanup = tal(cmd, struct txprepare_cleanup);
82+
cleanup->error_json = json_strdup(cleanup, buf, error);
83+
84+
req = jsonrpc_request_start(cmd, "unreserveinputs",
85+
txprepare_cleanup_done,
86+
txprepare_cleanup_done,
87+
cleanup);
88+
json_add_psbt(req->js, "psbt", utx->psbt);
89+
return send_outreq(req);
90+
}
91+
5392
static struct command_result *param_outputs(struct command *cmd,
5493
const char *name,
5594
const char *buffer,
@@ -179,7 +218,8 @@ static struct command_result *signpsbt_done(struct command *cmd,
179218
fmt_wally_psbt(tmpctx, utx->psbt));
180219

181220
req = jsonrpc_request_start(cmd, "sendpsbt",
182-
sendpsbt_done, forward_error,
221+
sendpsbt_done,
222+
txprepare_forward_error,
183223
utx);
184224
json_add_psbt(req->js, "psbt", utx->psbt);
185225
return send_outreq(req);
@@ -229,7 +269,8 @@ static struct command_result *finish_txprepare(struct command *cmd,
229269
/* Won't live beyond this cmd. */
230270
tal_steal(cmd, utx);
231271
req = jsonrpc_request_start(cmd, "signpsbt",
232-
signpsbt_done, forward_error,
272+
signpsbt_done,
273+
txprepare_forward_error,
233274
utx);
234275
json_add_psbt(req->js, "psbt", utx->psbt);
235276
return send_outreq(req);
@@ -464,7 +505,8 @@ static struct command_result *json_txsend(struct command *cmd,
464505
tal_steal(cmd, utx);
465506

466507
req = jsonrpc_request_start(cmd, "signpsbt",
467-
signpsbt_done, forward_error,
508+
signpsbt_done,
509+
txprepare_forward_error,
468510
utx);
469511
json_add_psbt(req->js, "psbt", utx->psbt);
470512
return send_outreq(req);

tests/test_wallet.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,33 @@ def test_withdraw(node_factory, bitcoind):
211211
l1.rpc.withdraw(l1.rpc.newaddr("p2tr")["p2tr"], 10**5, feerate="1000perkb")
212212

213213

214+
def test_withdraw_unreserves_inputs_on_send_failure(node_factory, bitcoind):
215+
amount = 10**7
216+
addrtype = good_addrtype()
217+
l1 = node_factory.get_node(random_hsm=True)
218+
addr = l1.rpc.newaddr(addrtype)[addrtype]
219+
220+
bitcoind.rpc.sendtoaddress(addr, amount / 10**8)
221+
bitcoind.generate_block(1)
222+
wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1)
223+
224+
def mock_sendrawtransaction(r):
225+
return {'id': r['id'],
226+
'error': {'code': 100,
227+
'message': 'feerate below mempool minimum: 251 < 253'}}
228+
229+
l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_sendrawtransaction)
230+
231+
with pytest.raises(RpcError, match=r'251 < 253'):
232+
l1.rpc.withdraw(bitcoind.getnewaddress(), 'all', feerate='slow')
233+
234+
assert not any(o['reserved'] for o in l1.rpc.listfunds()['outputs'])
235+
236+
l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)
237+
sent = l1.rpc.withdraw(bitcoind.getnewaddress(), 'all', feerate='slow')
238+
bitcoind.rpc.getmempoolentry(sent['txid'])
239+
240+
214241
def test_minconf_withdraw(node_factory, bitcoind):
215242
"""Issue 2518: ensure that ridiculous confirmation levels don't overflow
216243
@@ -2860,6 +2887,51 @@ def test_rescan_missing_utxo(node_factory, bitcoind):
28602887
assert not l3.daemon.is_in_log("Scanning for missed UTXOs", start=oldstart_l3)
28612888

28622889

2890+
@unittest.skipIf(TEST_NETWORK != 'regtest', "Uses regtest-specific address types")
2891+
def test_withdraw_unreserves_on_broadcast_failure(node_factory, bitcoind):
2892+
"""Test withdraw releases reservations after broadcast rejection."""
2893+
l1 = node_factory.get_node(random_hsm=True)
2894+
addr = l1.rpc.newaddr('p2tr')['p2tr']
2895+
2896+
# Fund the node
2897+
bitcoind.rpc.sendtoaddress(addr, 0.01)
2898+
bitcoind.generate_block(1)
2899+
wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1)
2900+
2901+
output = only_one(l1.rpc.listfunds()['outputs'])
2902+
assert output['status'] == 'confirmed'
2903+
assert not output.get('reserved', False)
2904+
2905+
waddr = bitcoind.rpc.getnewaddress()
2906+
2907+
# Mock sendrawtransaction to simulate bitcoind rejecting the transaction
2908+
# because the feerate is below its mempoolminfee
2909+
def mock_fail_sendrawtx(r):
2910+
# Self-remove after first call so subsequent transactions aren't blocked
2911+
l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', None)
2912+
return {
2913+
'id': r['id'],
2914+
'error': {
2915+
'code': -26,
2916+
'message': 'min relay fee not met, 253 < 5000',
2917+
},
2918+
'result': None,
2919+
}
2920+
2921+
l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_fail_sendrawtx)
2922+
2923+
with pytest.raises(RpcError, match=r'Error broadcasting transaction'):
2924+
l1.rpc.withdraw(waddr, 'all')
2925+
2926+
outputs = l1.rpc.listfunds()['outputs']
2927+
assert not any(o.get('reserved', False) for o in outputs)
2928+
2929+
l1.rpc.withdraw(waddr, 'all')
2930+
bitcoind.generate_block(1)
2931+
sync_blockheight(bitcoind, [l1])
2932+
assert l1.db_query('SELECT COUNT(*) as c FROM outputs WHERE status=0')[0]['c'] == 0
2933+
2934+
28632935
@unittest.skipIf(TEST_NETWORK != 'regtest', "Uses regtest-specific address types")
28642936
def test_withdraw_stuck_reserved_on_broadcast_failure(node_factory, bitcoind):
28652937
"""Test funds don't get stuck as reserved after withdraw fails due to

0 commit comments

Comments
 (0)