From 6821ab121dd87e2046a8d0b2376ab94dbda8dc77 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 5 Feb 2024 02:47:01 +1030 Subject: [PATCH 1/8] lightningd: correctly handle case where we have no fee estimates opening anchor channel. We tried to open a channel with feerate 0 in this case! Rework so it's clear that we have two different feerates here. Signed-off-by: Rusty Russell --- lightningd/opening_control.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index dfda12c2fe6e..a75d3baeda7b 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -1148,7 +1148,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd, struct node_id *id; struct peer *peer; bool *announce_channel; - u32 *feerate_per_kw, *mindepth; + u32 *feerate_non_anchor, feerate_anchor, *mindepth; int fds[2]; struct amount_sat *amount, *reserve; struct amount_msat *push_msat; @@ -1165,7 +1165,7 @@ static struct command_result *json_fundchannel_start(struct command *cmd, if (!param_check(fc->cmd, buffer, params, p_req("id", param_node_id, &id), p_req("amount", param_sat, &amount), - p_opt("feerate", param_feerate, &feerate_per_kw), + p_opt("feerate", param_feerate, &feerate_non_anchor), p_opt_def("announce", param_bool, &announce_channel, true), p_opt("close_to", param_bitcoin_address, &fc->our_upfront_shutdown_script), p_opt("push_msat", param_msat, &push_msat), @@ -1196,19 +1196,32 @@ static struct command_result *json_fundchannel_start(struct command *cmd, type_to_string(tmpctx, struct amount_sat, amount)); fc->funding_sats = *amount; - if (!feerate_per_kw) { - feerate_per_kw = tal(cmd, u32); - *feerate_per_kw = opening_feerate(cmd->ld->topology); - if (!*feerate_per_kw) { + if (!feerate_non_anchor) { + /* For non-anchors, we default to a low feerate for first + * commitment, and update it almost immediately. That saves + * money in the immediate-close case, which is probably soon + * and thus current feerates are sufficient. */ + feerate_non_anchor = tal(cmd, u32); + *feerate_non_anchor = opening_feerate(cmd->ld->topology); + if (!*feerate_non_anchor) { return command_fail(cmd, LIGHTNINGD, "Cannot estimate fees"); } } - if (*feerate_per_kw < get_feerate_floor(cmd->ld->topology)) { + feerate_anchor = unilateral_feerate(cmd->ld->topology, true); + /* Only complain here if we could possibly open one! */ + if (!feerate_anchor + && feature_offered(cmd->ld->our_features->bits[INIT_FEATURE], + OPT_ANCHORS_ZERO_FEE_HTLC_TX)) { return command_fail(cmd, LIGHTNINGD, - "Feerate below feerate floor %u perkw", - get_feerate_floor(cmd->ld->topology)); + "Cannot estimate fees"); + } + + if (*feerate_non_anchor < get_feerate_floor(cmd->ld->topology)) { + return command_fail(cmd, LIGHTNINGD, + "Feerate for non-anchor (%u perkw) below feerate floor %u perkw", + *feerate_non_anchor, get_feerate_floor(cmd->ld->topology)); } if (!topology_synced(cmd->ld->topology)) { @@ -1324,8 +1337,8 @@ static struct command_result *json_fundchannel_start(struct command *cmd, fc->push, fc->our_upfront_shutdown_script, upfront_shutdown_script_wallet_index, - *feerate_per_kw, - unilateral_feerate(cmd->ld->topology, true), + *feerate_non_anchor, + feerate_anchor, &tmp_channel_id, fc->channel_flags, fc->uc->reserve, From 9486fbde3efa9bd15bf4065a8156a80fcea946a0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 5 Feb 2024 02:48:01 +1030 Subject: [PATCH 2/8] wallet: be a little more flexible with change for emergency reserve. We used to look for either other outputs which are sufficient for reserve, *or* be able to create sufficient change to meet the emergency reserve. Allow the sum of both to meet the requirements: otherwise test_funder_contribution_limits can flake. Signed-off-by: Rusty Russell --- wallet/reservation.c | 20 ++++++++++---------- wallet/wallet.c | 15 +++------------ wallet/wallet.h | 4 ++-- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/wallet/reservation.c b/wallet/reservation.c index fe092fc916cc..de477cb77596 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -461,40 +461,40 @@ static bool change_for_emergency(struct lightningd *ld, struct amount_sat *excess, struct amount_sat *change) { - struct amount_sat fee; + struct amount_sat needed = ld->emergency_sat, fee; /* Only needed for anchor channels */ if (!have_anchor_channel) return true; - /* Fine if rest of wallet has funds. */ + /* Fine if rest of wallet has funds. Otherwise it may reduce + * needed amount. */ if (wallet_has_funds(ld->wallet, cast_const2(const struct utxo **, utxos), get_block_height(ld->topology), - ld->emergency_sat)) + &needed)) return true; - /* If we can afford with existing change output, great (or + /* If we can afford the rest with existing change output, great (or * ld->emergency_sat is 0) */ if (amount_sat_greater_eq(change_amount(*change, feerate_per_kw, weight), - ld->emergency_sat)) + 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, ld->emergency_sat)) + || !amount_sat_sub(excess, *excess, needed)) return false; if (!amount_sat_add(change, *change, fee) - || !amount_sat_add(change, *change, ld->emergency_sat)) + || !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), - ld->emergency_sat)); + assert(amount_sat_eq(change_amount(*change, feerate_per_kw, weight), + needed)); return true; } diff --git a/wallet/wallet.c b/wallet/wallet.c index da9d789c86dc..a75ceba2e4f2 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -674,10 +674,9 @@ struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, bool wallet_has_funds(struct wallet *w, const struct utxo **excludes, u32 current_blockheight, - struct amount_sat sats) + struct amount_sat *needed) { struct db_stmt *stmt; - struct amount_sat total = AMOUNT_SAT(0); stmt = db_prepare_v2(w->db, SQL("SELECT" " prev_out_tx" @@ -712,17 +711,9 @@ bool wallet_has_funds(struct wallet *w, continue; } - /* Overflow Should Not Happen */ - if (!amount_sat_add(&total, total, utxo->amount)) { - db_fatal(w->db, "Invalid value for %s: %s", - type_to_string(tmpctx, - struct bitcoin_outpoint, - &utxo->outpoint), - fmt_amount_sat(tmpctx, utxo->amount)); - } - /* If we've found enough, answer is yes. */ - if (amount_sat_greater_eq(total, sats)) { + if (!amount_sat_sub(needed, *needed, utxo->amount)) { + *needed = AMOUNT_SAT(0); tal_free(stmt); return true; } diff --git a/wallet/wallet.h b/wallet/wallet.h index 904cd2301fac..a33ea4676f72 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -482,7 +482,7 @@ struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, * @w: the wallet * @excludes: the utxos not to count (tal_arr or NULL) * @current_blockheight: current chain length. - * @sats: the target + * @needed: the target, reduced if we find some funds * * This is a gross estimate, since it doesn't take into account the fees we * would need to actually spend these utxos! @@ -490,7 +490,7 @@ struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, bool wallet_has_funds(struct wallet *wallet, const struct utxo **excludes, u32 current_blockheight, - struct amount_sat sats); + struct amount_sat *needed); /** * wallet_add_onchaind_utxo - Add a UTXO with spending info from onchaind. From 56fdaaf21bdd31700014936bfaba0acde28051af Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 5 Feb 2024 02:49:01 +1030 Subject: [PATCH 3/8] funder: don't try to spend emergency_reserve We might have (or be getting!) anchor channels, so keep this aside when funding. Signed-off-by: Rusty Russell --- plugins/funder.c | 35 ++++++++++++++++++++++++++++++++++- tests/test_opening.py | 4 ++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/plugins/funder.c b/plugins/funder.c index 8fb08d4aef90..fa1c6c35f21f 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -6,6 +6,7 @@ #include "config.h" #include #include +#include #include #include #include @@ -35,6 +36,9 @@ struct pending_open { const struct wally_psbt *psbt; }; +/* How much do we need to keep in reserve for anchor spends? */ +static struct amount_sat emergency_reserve; + static struct pending_open * find_channel_pending_open(const struct channel_id *cid) { @@ -564,7 +568,7 @@ listfunds_success(struct command *cmd, const jsmntok_t *result, struct open_info *info) { - struct amount_sat available_funds, committed_funds, est_fee; + struct amount_sat available_funds, committed_funds, total_fee; const jsmntok_t *outputs_tok, *tok; struct out_req *req; struct bitcoin_outpoint **avail_prev_outs; @@ -583,9 +587,11 @@ listfunds_success(struct command *cmd, available_funds = AMOUNT_SAT(0); committed_funds = AMOUNT_SAT(0); + total_fee = AMOUNT_SAT(0); avail_prev_outs = tal_arr(info, struct bitcoin_outpoint *, 0); json_for_each_arr(i, tok, outputs_tok) { struct funder_utxo *utxo; + struct amount_sat est_fee; bool is_reserved; struct bitcoin_outpoint *prev_out; char *status; @@ -639,6 +645,10 @@ listfunds_success(struct command *cmd, plugin_err(cmd->plugin, "`listfunds` overflowed output values"); + if (!amount_sat_add(&total_fee, total_fee, est_fee)) + plugin_err(cmd->plugin, + "`listfunds` overflowed fee values"); + /* If this is an RBF, we keep track of available utxos */ if (info->prev_outs) { /* if not previously reserved, it's committed */ @@ -660,6 +670,21 @@ listfunds_success(struct command *cmd, } } + /* Even if we don't have an anchor channel yet, we might soon: + * keep reserve, even after fee! Assume two outputs, one for + * change. */ + if (!amount_sat_add(&total_fee, total_fee, + amount_tx_fee(info->funding_feerate_perkw, + BITCOIN_SCRIPTPUBKEY_P2WSH_LEN + + change_weight()))) { + plugin_err(cmd->plugin, + "fee value overflow for estimating total fee"); + } + + if (!amount_sat_sub(&available_funds, available_funds, total_fee) + || !amount_sat_sub(&available_funds, available_funds, emergency_reserve)) + available_funds = AMOUNT_SAT(0); + funding_err = calculate_our_funding(current_policy, info->id, info->their_funding, @@ -1467,6 +1492,7 @@ static void memleak_mark(struct plugin *p, struct htable *memtable) static const char *init(struct plugin *p, const char *b, const jsmntok_t *t) { const char *err; + struct amount_msat msat; list_head_init(&pending_opens); @@ -1477,6 +1503,13 @@ static const char *init(struct plugin *p, const char *b, const jsmntok_t *t) if (current_policy->rates) tell_lightningd_lease_rates(p, current_policy->rates); + rpc_scan(p, "listconfigs", + take(json_out_obj(NULL, NULL, NULL)), + "{configs:" + "{min-emergency-msat:{value_msat:%}}}", + JSON_SCAN(json_to_msat, &msat)); + + emergency_reserve = amount_msat_to_sat_round_down(msat); plugin_set_memleak_handler(p, memleak_mark); return NULL; diff --git a/tests/test_opening.py b/tests/test_opening.py index d0f9b83116de..80f3ed050248 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -1539,8 +1539,8 @@ def test_funder_contribution_limits(node_factory, bitcoind): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 10**7) - assert l2.daemon.is_in_log('Policy .* returned funding amount of 141780sat') - assert l2.daemon.is_in_log(r'calling `signpsbt` .* 6 inputs') + assert l2.daemon.is_in_log('Policy .* returned funding amount of 107530sat') + assert l2.daemon.is_in_log(r'calling `signpsbt` .* inputs') l1.rpc.connect(l3.info['id'], 'localhost', l3.port) l1.fundchannel(l3, 10**7) From ce0b7c27ba1744896ab387052ff1a6df4c2c9bf7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Feb 2024 13:42:50 +1030 Subject: [PATCH 4/8] lightningd: allow dev-force-features to unset even if not set. This simplifies our tests which will want to turn off anchors, even though they won't be set for elements. Signed-off-by: Rusty Russell --- lightningd/options.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightningd/options.c b/lightningd/options.c index 80d110412c4a..0127853b1ca1 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -759,9 +759,9 @@ static char *opt_force_featureset(const char *optarg, return "Invalid feature number"; f = feature_set_for_feature(NULL, n); - if (strstarts(optarg, "-") - && !feature_set_sub(ld->our_features, take(f))) - return "Feature unknown"; + if (strstarts(optarg, "-")) { + feature_set_sub(ld->our_features, take(f)); + } if (strstarts(optarg, "+") && !feature_set_or(ld->our_features, take(f))) From d17a12e421943cd183cfc7d775cba9c48bf11ecf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Feb 2024 13:42:59 +1030 Subject: [PATCH 5/8] pytest: changed tests if we're using experimental-anchors. This is in anticipation of changing the defaults for non-elements. Signed-off-by: Rusty Russell --- tests/test_closing.py | 77 ++++++++++++++++++++++++++-------------- tests/test_connection.py | 53 ++++++++++++++++++--------- tests/test_misc.py | 21 +++++++---- tests/test_pay.py | 32 +++++------------ tests/test_plugin.py | 10 ++++-- 5 files changed, 119 insertions(+), 74 deletions(-) diff --git a/tests/test_closing.py b/tests/test_closing.py index 90fb0a3a34a5..caa2df2d352f 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -2643,8 +2643,9 @@ def test_onchain_different_fees(node_factory, bitcoind, executor): l2.daemon.wait_for_log('htlc 1: SENT_ADD_ACK_COMMIT->RCVD_ADD_ACK_REVOCATION') # Restart with different feerate for second HTLC. - l1.set_feerates((5000, 5000, 5000, 3750)) - l1.restart() + l1.stop() + l1.set_feerates((5000, 5000, 5000, 5000), wait_for_effect=False) + l1.start() l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE') p3 = executor.submit(l1.pay, l2, 800000000) @@ -2658,23 +2659,40 @@ def test_onchain_different_fees(node_factory, bitcoind, executor): l1.daemon.wait_for_log(' to ONCHAIN') l2.daemon.wait_for_log(' to ONCHAIN') + # Elements still uses non-anchor version. + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + expected = {'min_possible_feerate': 3750, + 'max_possible_feerate': 5005} + else: + expected = {'min_possible_feerate': 5005, + 'max_possible_feerate': 11005} + # Both sides should have correct feerate - assert l1.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [{ - 'min_possible_feerate': 5005, - 'max_possible_feerate': 11005 - }] - assert l2.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [{ - 'min_possible_feerate': 5005, - 'max_possible_feerate': 11005 - }] + assert l1.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [expected] + + assert l2.db_query('SELECT min_possible_feerate, max_possible_feerate FROM channels;') == [expected] bitcoind.generate_block(5) - # Three HTLCs, and one for the to-us output. - l1.daemon.wait_for_logs(['sendrawtx exit 0'] * 4) - # We use 3 blocks for "reasonable depth" - bitcoind.generate_block(3) + # We will, over the next few blocks, spend the to-us and the HTLCs. + # We don't have enough outputs, so the results are a bit random, + # but we will make progress every block! + mark = l1.daemon.logsearch_start + + r = re.compile('sendrawtx exit 0') + def count_successful_txs(): + l1.daemon.logs_catchup() + return sum([r.search(l) is not None for l in l1.daemon.logs[mark:]]) + + # First iteration we expect one HTLC-tx and the to-us, then two more htlc txs + num_successful = 1 + while num_successful < 3 + 1: + wait_for(lambda: count_successful_txs() > num_successful) + num_successful = count_successful_txs() + bitcoind.generate_block(1) + + # This takes at least 3 blocks: now payments can fail with pytest.raises(Exception): p1.result(10) with pytest.raises(Exception): @@ -2682,9 +2700,12 @@ def test_onchain_different_fees(node_factory, bitcoind, executor): with pytest.raises(Exception): p3.result(10) - # Two more for HTLC timeout tx to be spent. - bitcoind.generate_block(2) - l1.daemon.wait_for_logs(['sendrawtx exit 0'] * 3) + # Now we need the return-to-wallet spending the htlc tx. + bitcoind.generate_block(4) + while num_successful < 3 + 3 + 1: + wait_for(lambda: count_successful_txs() > num_successful) + num_successful = count_successful_txs() + bitcoind.generate_block(1) # Now, 100 blocks it should be done. bitcoind.generate_block(100) @@ -2898,7 +2919,7 @@ def test_onchain_multihtlc_our_unilateral(node_factory, bitcoind): # until l4 says 'all outputs resolved'. while not l4.daemon.is_in_log('All outputs resolved'): bitcoind.generate_block(1) - assert bitcoind.rpc.getblockcount() < 200 + assert bitcoind.rpc.getblockcount() < 250 sync_blockheight(bitcoind, [l4, l5]) # All payments should be long resolved. @@ -2953,7 +2974,7 @@ def test_onchain_multihtlc_their_unilateral(node_factory, bitcoind): # until l5 says 'all outputs resolved'. while not l5.daemon.is_in_log('All outputs resolved'): bitcoind.generate_block(1) - assert bitcoind.rpc.getblockcount() < 200 + assert bitcoind.rpc.getblockcount() < 250 sync_blockheight(bitcoind, [l4, l5]) # All payments should be long resolved. @@ -2981,6 +3002,8 @@ def test_permfail_htlc_in(node_factory, bitcoind, executor): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 10**6) + # Give it some sats for anchor spend! (Extra for elements!) + l2.fundwallet(30000, mine_block=False) # This will fail at l2's end. t = executor.submit(l1.pay, l2, 200000000) @@ -3574,11 +3597,9 @@ def save_notifications(message, progress, request, **kwargs): def test_close_twice(node_factory, executor): # First feerate is too low, second fixes it. - l1, l2 = node_factory.line_graph(2, opts=[{'allow_warning': True, - 'may_reconnect': True}, - {'allow_warning': True, - 'may_reconnect': True, - 'feerates': (15000, 15000, 15000, 15000)}]) + l1, l2 = node_factory.line_graph(2, opts={'allow_warning': True, + 'may_reconnect': True, + 'feerates': (15000, 15000, 15000, 15000)}) # This makes it disconnect, since feerate is too low. fut = executor.submit(l1.rpc.close, l2.info['id'], feerange=['253perkw', '500perkw']) @@ -3978,7 +3999,7 @@ def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): def test_closing_cpfp(node_factory, bitcoind): - l1, l2 = node_factory.line_graph(2) + l1, l2 = node_factory.line_graph(2, opts={'min-emergency-msat': '2500sat'}) # We want to ignore l1's change output change = only_one(l1.rpc.listfunds()['outputs']) @@ -4004,4 +4025,8 @@ def test_closing_cpfp(node_factory, bitcoind): # They should now see a single additional output each sync_blockheight(bitcoind, [l1, l2]) assert len(l1.rpc.listfunds()['outputs']) == 2 - assert len(l2.rpc.listfunds()['outputs']) == 1 + # This one will also have emergency change if anchors + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + assert len(l2.rpc.listfunds()['outputs']) == 2 + else: + assert len(l2.rpc.listfunds()['outputs']) == 1 diff --git a/tests/test_connection.py b/tests/test_connection.py index cbf3ad6b6f92..b5518d99a1ca 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -437,8 +437,8 @@ def test_channel_abandon(node_factory, bitcoind): SATS = 10**6 - # Add some for fees - l1.fundwallet(SATS + 10000) + # Add some for fees/emergency-reserve + l1.fundwallet(SATS + 35000) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.fundchannel(l2.info['id'], SATS, feerate='1875perkw') @@ -1077,8 +1077,12 @@ def test_funding_all(node_factory, bitcoind): l1.rpc.fundchannel(l2.info['id'], "all") + # Keeps emergency reserve! outputs = l1.db_query('SELECT value FROM outputs WHERE status=0;') - assert len(outputs) == 0 + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + assert outputs == [{'value': 25000}] + else: + assert outputs == [] @pytest.mark.openchannel('v1') @@ -1089,7 +1093,7 @@ def test_funding_all_too_much(node_factory): # l2 isn't wumbo, so channel should not be! l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=[{}, {'dev-force-features': '-19'}]) - addr, txid = l1.fundwallet(2**24 + 10000) + addr, txid = l1.fundwallet(2**24 + 35000) l1.rpc.fundchannel(l2.info['id'], "all") assert l1.daemon.is_in_log("'all' was too large for non-wumbo channel, trimming") @@ -2419,7 +2423,10 @@ def test_update_fee(node_factory, bitcoind): # Make payments. l1.pay(l2, 200000000) # First payment causes fee update. - l2.daemon.wait_for_log('peer updated fee to 11005') + if 'anchors_zero_fee_htlc_tx/even' in only_one(l2.rpc.listpeerchannels()['channels'])['channel_type']['names']: + l2.daemon.wait_for_log('peer updated fee to 3755') + else: + l2.daemon.wait_for_log('peer updated fee to 11005') l2.pay(l1, 100000000) # Now shutdown cleanly. @@ -2459,24 +2466,32 @@ def test_fee_limits(node_factory, bitcoind): # L1 asks for stupid low fee (will actually hit the floor of 253) l1.stop() l1.set_feerates((15, 15, 15, 15), False) + # We need to increase l2's floor, so it rejects l1. + l2.set_feerates((15000, 11000, 7500, 3750, 2000)) l1.start() - l1.daemon.wait_for_log('Received WARNING .*: update_fee 258 outside range 1875-75000') + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + fee = 1255 + range = '2000-75000' + else: + fee = 258 + range = '1875-75000' + l1.daemon.wait_for_log(f'Received WARNING .*: update_fee {fee} outside range {range}') # They hang up on *us* l1.daemon.wait_for_log('Peer transient failure in CHANNELD_NORMAL: channeld: Owning subdaemon channeld died') # Disconnects, but does not error. Make sure it's noted in their status though. # FIXME: does not happen for l1! # assert 'update_fee 253 outside range 1875-75000' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'][0] - assert 'update_fee 258 outside range 1875-75000' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0] + assert f'update_fee {fee} outside range {range}' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0] - assert only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] != 258 + assert only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] != fee # Make l2 accept those fees, and it should recover. assert only_one(l2.rpc.setchannel(l1.get_channel_scid(l2), ignorefeelimits=True)['channels'])['ignore_fee_limits'] is True assert only_one(l2.rpc.listpeerchannels()['channels'])['ignore_fee_limits'] is True # Now we stay happy (and connected!) - wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] == 258) + wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] == fee) assert only_one(l2.rpc.listpeerchannels()['channels'])['peer_connected'] is True l1.rpc.close(l2.info['id']) @@ -2490,7 +2505,7 @@ def test_fee_limits(node_factory, bitcoind): # Trying to open a channel with too low a fee-rate is denied l1.rpc.connect(l4.info['id'], 'localhost', l4.port) - with pytest.raises(RpcError, match='They sent (ERROR|WARNING) .* feerate_per_kw 253 below minimum'): + with pytest.raises(RpcError, match='They sent (ERROR|WARNING) .* feerate_per_kw .* below minimum'): l1.fundchannel(l4, 10**6) # Restore to normal. @@ -2507,7 +2522,7 @@ def test_fee_limits(node_factory, bitcoind): # Try stupid high fees l1.stop() - l1.set_feerates((15000, 11000 * 10, 7500, 3750), False) + l1.set_feerates((15000, 15000, 15000, 15000, 15000), False) l1.start() l3.daemon.wait_for_log('peer_in WIRE_UPDATE_FEE') @@ -2589,7 +2604,7 @@ def test_update_fee_reconnect(node_factory, bitcoind): # Make l1 send out feechange; triggers disconnect/reconnect. # (Note: < 10% change, so no smoothing here!) - l1.set_feerates((14000, 14000, 14000, 3750)) + l1.set_feerates((14000, 14000, 14000, 14000)) l1.daemon.wait_for_log('Setting REMOTE feerate to 14005') l2.daemon.wait_for_log('Setting LOCAL feerate to 14005') l1.daemon.wait_for_log(r'dev_disconnect: \+WIRE_COMMITMENT_SIGNED') @@ -3354,14 +3369,20 @@ def test_feerate_spam(node_factory, chainparams): l1.pay(l2, 10**9 - slack) # It will send this once (may have happened before line_graph's wait) - wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 11005')) + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 3755')) + else: + wait_for(lambda: l1.daemon.is_in_log('Setting REMOTE feerate to 11005')) wait_for(lambda: l1.daemon.is_in_log('peer_out WIRE_UPDATE_FEE')) # Now change feerates to something l1 can't afford. - l1.set_feerates((100000, 100000, 100000, 100000)) + l1.set_feerates((200000, 200000, 200000, 200000)) - # It will raise as far as it can (48000) - maxfeerate = 48000 + # It will raise as far as it can (30000) + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + maxfeerate = 30000 + else: + maxfeerate = 48000 l1.daemon.wait_for_log('Setting REMOTE feerate to {}'.format(maxfeerate)) l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE') diff --git a/tests/test_misc.py b/tests/test_misc.py index 09410cd6a146..f7d4c8f3a23b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -295,15 +295,16 @@ def test_htlc_sig_persistence(node_factory, bitcoind, executor): _, txid, blocks = l1.wait_for_onchaind_tx('OUR_HTLC_TIMEOUT_TO_US', 'THEIR_UNILATERAL/OUR_HTLC') assert blocks == 5 + bitcoind.generate_block(5) bitcoind.generate_block(1, wait_for_mempool=txid) l1.daemon.wait_for_logs([ r'Owning output . (\d+)sat .SEGWIT. txid', ]) - # We should now have a) the change from funding, b) the - # unilateral to us, and c) the HTLC respend to us - assert len(l1.rpc.listfunds()['outputs']) == 3 + # We should now have 1) the unilateral to us, and b) the HTLC respend to us + # and maybe (c) change. + assert 2 <= len(l1.rpc.listfunds()['outputs']) <= 3 def test_htlc_out_timeout(node_factory, bitcoind, executor): @@ -385,6 +386,8 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor): options={'dev-no-reconnect': None}, feerates=(7500, 7500, 7500, 7500)) l2 = node_factory.get_node() + # Give it some sats for anchor spend! + l2.fundwallet(25000, mine_block=False) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) chanid, _ = l1.fundchannel(l2, 10**6) @@ -3876,9 +3879,15 @@ def test_set_feerate_offset(node_factory, bitcoind): l1.pay(l2, 200000000) # First payment causes fee update, which should reflect the feerate offset. - l1.daemon.wait_for_log('lightningd: update_feerates: feerate = 11100, ' - 'min=1875, max=150000, penalty=7500') - l2.daemon.wait_for_log('peer updated fee to 11100') + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + feerate = 3850 + min_feerate = 253 + else: + feerate = 11100 + min_feerate = 1875 + l1.daemon.wait_for_log(f'lightningd: update_feerates: feerate = {feerate}, ' + f'min={min_feerate}, max=150000, penalty=7500') + l2.daemon.wait_for_log(f'peer updated fee to {feerate}') l2.pay(l1, 100000000) # Now shutdown cleanly. diff --git a/tests/test_pay.py b/tests/test_pay.py index ad157ce442b5..ade0abfe2052 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -275,7 +275,10 @@ def test_pay_disconnect(node_factory, bitcoind): l1.set_feerates((10**6, 10**6, 10**6, 10**6), False) # Wait for l1 notice - l1.daemon.wait_for_log(r'WARNING .*: update_fee \d+ outside range 1875-75000') + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + l1.daemon.wait_for_log(r'WARNING .*: update_fee \d+ outside range 253-75000') + else: + l1.daemon.wait_for_log(r'WARNING .*: update_fee \d+ outside range 1875-75000') # They hang up on us l1.daemon.wait_for_log(r'Peer transient failure in CHANNELD_NORMAL') @@ -2736,42 +2739,25 @@ def test_htlc_too_dusty_outgoing(node_factory, bitcoind, chainparams): res = only_one(l1.rpc.listsendpays(payment_hash=inv['payment_hash'])['payments']) assert res['status'] == 'pending' - # Ok, adjust our feerate upward, so the non-dust htlcs are now dust - # note that this is above the buffer we've been keeping, so the channel - # should automatically fail - l1.set_feerates([feerate * 2] * 4, False) - l1.restart() - - # Make sure fails before we try sending htlc! - l1.daemon.wait_for_log('Too much dust to update fee') - - # the channel should start warning -- too much dust - inv = l2.rpc.invoice(htlc_val_msat, str(num_dusty_htlcs + 1), str(num_dusty_htlcs + 1)) - with pytest.raises(RpcError, match=r'WIRE_TEMPORARY_CHANNEL_FAILURE'): - l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret']) - def test_htlc_too_dusty_incoming(node_factory, bitcoind): """ Try to hit the 'too much dust' limit, should fail the HTLC """ - feerate = 30000 l1, l2, l3 = node_factory.line_graph(3, opts=[{'may_reconnect': True, - 'feerates': (feerate, feerate, feerate, feerate), 'max-dust-htlc-exposure-msat': '200000sat'}, {'may_reconnect': True, - 'feerates': (feerate, feerate, feerate, feerate), - 'max-dust-htlc-exposure-msat': '100000sat', + 'max-dust-htlc-exposure-msat': '1000sat', 'fee-base': 0, 'fee-per-satoshi': 0}, - {'max-dust-htlc-exposure-msat': '500000sat'}], + {'max-dust-htlc-exposure-msat': '1000sat'}], wait_for_announce=True) # on the l2->l3, and l3 holds all the htlcs hostage # have l3 hold onto all the htlcs and not fulfill them l3.rpc.dev_ignore_htlcs(id=l2.info['id'], ignore=True) - # l2's max dust limit is set to 100k - max_dust_limit_sat = 100000 - htlc_val_sat = 10000 + # l2's max dust limit is set to 1k + max_dust_limit_sat = 1000 + htlc_val_sat = 250 htlc_val_msat = htlc_val_sat * 1000 num_dusty_htlcs = max_dust_limit_sat // htlc_val_sat route = l1.rpc.getroute(l3.info['id'], htlc_val_msat, 1)['route'] diff --git a/tests/test_plugin.py b/tests/test_plugin.py index f9c77fe70bf6..98fbf621da53 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -668,12 +668,16 @@ def test_openchannel_hook(node_factory, bitcoind): 'to_self_delay': '5', } + if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: + feerate = 3750 + else: + feerate = 7500 if l2.config('experimental-dual-fund'): # openchannel2 var checks expected.update({ 'channel_id': '.*', 'channel_max_msat': 2100000000000000000, - 'commitment_feerate_per_kw': '7500', + 'commitment_feerate_per_kw': feerate, 'funding_feerate_per_kw': '7500', 'feerate_our_max': '150000', 'feerate_our_min': '1875', @@ -684,7 +688,7 @@ def test_openchannel_hook(node_factory, bitcoind): else: expected.update({ 'channel_reserve_msat': 1000000, - 'feerate_per_kw': '7500', + 'feerate_per_kw': feerate, 'funding_msat': 100000000, 'push_msat': 0, }) @@ -1879,7 +1883,7 @@ def test_hook_crash(node_factory, executor, bitcoind): # For simplicity, give us N UTXOs to spend. addr = l1.rpc.newaddr('p2tr')['p2tr'] for n in nodes: - bitcoind.rpc.sendtoaddress(addr, (FUNDAMOUNT + 5000) / 10**8) + bitcoind.rpc.sendtoaddress(addr, (FUNDAMOUNT + 30000) / 10**8) bitcoind.generate_block(1, wait_for_mempool=len(nodes)) sync_blockheight(bitcoind, [l1]) From 787a36e47e8fb3265fec636db75a9c11e18d5bb6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Feb 2024 14:43:28 +1030 Subject: [PATCH 6/8] options: make anchors enabled by default, ignore experimental-anchors. We still want to test non-anchor channels, as we still support them, but we've made it non-experimental. To test non-anchor channels, we use dev-force-features: -23. Changelog-Added: Protocol: `option_anchors_zero_fee_htlc_tx` enabled, no longer experimental. Changelog-Changed: Config: `experimental-anchors` now does nothing (it's enabled by default). Signed-off-by: Rusty Russell Header from folded patch 'fixup!_options__make_anchors_enabled_by_default,_ignore_experimental-anchors.patch': fixup! options: make anchors enabled by default, ignore experimental-anchors. --- contrib/pyln-testing/pyln/testing/utils.py | 23 +++++- doc/lightningd-config.5.md | 10 --- lightningd/hsm_control.c | 2 +- lightningd/lightningd.c | 7 +- lightningd/options.c | 15 ++-- tests/test_bookkeeper.py | 3 +- tests/test_closing.py | 90 +++++++++++----------- tests/test_connection.py | 53 +++++++------ tests/test_misc.py | 9 ++- tests/test_opening.py | 41 +++++----- tests/test_pay.py | 13 ++-- tests/utils.py | 6 ++ 12 files changed, 144 insertions(+), 128 deletions(-) diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 37aeecab6e8f..f0a4e2336bb6 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -939,10 +939,10 @@ def fundbalancedchannel(self, remote_node, total_capacity=FUNDAMOUNT, announce=T else: total_capacity = int(total_capacity) - self.fundwallet(total_capacity + 10000) + self.fundwallet(total_capacity + 35000) if remote_node.config('experimental-dual-fund'): - remote_node.fundwallet(total_capacity + 10000) + remote_node.fundwallet(total_capacity + 35000) # We cut the total_capacity in half, since the peer's # expected to contribute that same amount chan_capacity = total_capacity // 2 @@ -1274,6 +1274,25 @@ def mock_estimatesmartfee(r): } self.daemon.rpcproxy.mock_rpc('estimatesmartfee', mock_estimatesmartfee) + def mock_getmempoolinfo(r): + return { + 'id': r['id'], + 'error': None, + 'result': { + 'mempoolminfee': Decimal(feerates[4] * 4) / 10**8, + 'minrelaytxfee': Decimal(feerates[4] * 4) / 10**8, + }, + } + + # Did they want to set minfee as well? + if len(feerates) > 4: + assert len(feerates) == 5 + self.daemon.rpcproxy.mock_rpc('getmempoolinfo', mock_getmempoolinfo) + + if wait_for_effect: + wait_for(lambda: + self.rpc.feerates(style='perkb')['perkb']['floor'] == feerates[4] * 4) + # Technically, this waits until it's called, not until it's processed. # We wait until all four levels have been called. if wait_for_effect: diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 177d6b624fd0..009bf5ad3e56 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -461,7 +461,6 @@ specified multuple times. (Added in v23.08). * **min-emergency-msat**=*msat* This is the amount of funds to keep in the wallet to close anchor channels (which don't carry their own transaction fees). It defaults to 25000sat, and is only maintained if there are any anchor channels (or, when opening an anchor channel). This amount may be insufficient for multiple closes at once, however. - ### Cleanup control options: @@ -793,15 +792,6 @@ protocol to update channel types. At the moment, we only support setting this option. -* **experimental-anchors** - - Specifying this option turns on the `option_anchors_zero_fee_htlc_tx` -feature, meaning we can open anchor-based channels. This will become -the default for new channels in future, after more testing. Anchor-based -channels use larger commitment transactions, with the trade-off that they -don't have to use a worst-case fee, but can bump the commitment transaction -if it's needed. Note that this means that we need to keep -some funds aside: see `min-emergency-msat`. BUGS ---- diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c index 3765787c63f6..0765af142691 100644 --- a/lightningd/hsm_control.c +++ b/lightningd/hsm_control.c @@ -160,7 +160,7 @@ struct ext_key *hsm_init(struct lightningd *ld) if (feature_offered(ld->our_features->bits[INIT_FEATURE], OPT_ANCHORS_ZERO_FEE_HTLC_TX) && !hsm_capable(ld, WIRE_HSMD_SIGN_ANCHORSPEND)) { - fatal("--experimental-anchors needs HSM capable of signing anchors!"); + fatal("anchors needs HSM capable of signing anchors!"); } if (feature_offered(ld->our_features->bits[INIT_FEATURE], diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 3913f364ea55..e13750de02f0 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -906,11 +906,14 @@ static struct feature_set *default_features(const tal_t *ctx) OPTIONAL_FEATURE(OPT_ZEROCONF), OPTIONAL_FEATURE(OPT_CHANNEL_TYPE), OPTIONAL_FEATURE(OPT_ROUTE_BLINDING), + /* Removed later for elements */ + OPTIONAL_FEATURE(OPT_ANCHORS_ZERO_FEE_HTLC_TX), }; for (size_t i = 0; i < ARRAY_SIZE(features); i++) { - struct feature_set *f - = feature_set_for_feature(NULL, features[i]); + struct feature_set *f; + + f = feature_set_for_feature(NULL, features[i]); if (!ret) ret = tal_steal(ctx, f); else diff --git a/lightningd/options.c b/lightningd/options.c index 0127853b1ca1..38250b325358 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -1258,10 +1258,7 @@ static char *opt_set_quiesce(struct lightningd *ld) static char *opt_set_anchor_zero_fee_htlc_tx(struct lightningd *ld) { - /* Requires static_remotekey, but we always set that */ - feature_set_or(ld->our_features, - take(feature_set_for_feature(NULL, - OPTIONAL_FEATURE(OPT_ANCHORS_ZERO_FEE_HTLC_TX)))); + /* FIXME: deprecated_apis! */ return NULL; } @@ -1469,8 +1466,7 @@ static void register_opts(struct lightningd *ld) " channels."); opt_register_early_noarg("--experimental-anchors", opt_set_anchor_zero_fee_htlc_tx, ld, - "EXPERIMENTAL: enable option_anchors_zero_fee_htlc_tx" - " to open zero-fee-anchor channels"); + opt_hidden); clnopt_witharg("--announce-addr-dns", OPT_EARLY|OPT_SHOWBOOL, opt_set_bool_arg, opt_show_bool, &ld->announce_dns, @@ -1789,6 +1785,13 @@ void handle_early_opts(struct lightningd *ld, int argc, char *argv[]) else ld->config = mainnet_config; + /* No anchors if we're elements */ + if (chainparams->is_elements) { + feature_set_sub(ld->our_features, + feature_set_for_feature(tmpctx, + OPTIONAL_FEATURE(OPT_ANCHORS_ZERO_FEE_HTLC_TX))); + } + /* Set the ln_port given from chainparams */ ld->config.ip_discovery_port = chainparams->ln_port; diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index f264d1943fc8..6a63c6c3927e 100644 --- a/tests/test_bookkeeper.py +++ b/tests/test_bookkeeper.py @@ -355,8 +355,7 @@ def test_bookkeeping_missed_chans_leases(node_factory, bitcoind): opts = {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'plugin': str(coin_mvt_plugin), - 'disable-plugin': 'bookkeeper', - 'experimental-anchors': None} + 'disable-plugin': 'bookkeeper'} l1, l2 = node_factory.get_nodes(2, opts=opts) diff --git a/tests/test_closing.py b/tests/test_closing.py index caa2df2d352f..4d7e2fc2ea84 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -361,8 +361,10 @@ def feerate_for(target, minimum=0, maximum=10000000): orate = feerate_for(21000) # closing fee negotiation starts at 21000 prate = feerate_for(20000) # closing fee negotiation starts at 20000 - opener, peer = node_factory.line_graph(2, opts=[{'feerates': (orate, orate, orate, orate)}, - {'feerates': (prate, prate, prate, prate)}]) + opener, peer = node_factory.line_graph(2, opts=[{'feerates': (orate, orate, orate, orate), + 'dev-force-features': "-23"}, + {'feerates': (prate, prate, prate, prate), + 'dev-force-features': "-23"}]) opener_id = opener.info['id'] peer_id = peer.info['id'] @@ -501,8 +503,8 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams, anchors): # Feerates identical so we don't get gratuitous commit to update them opts = {'dev-disable-commit-after': 1, 'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # FIXME: | for dicts was added in Python 3.9 apparently. l1, l2 = node_factory.line_graph(2, opts=[{**opts, **{'may_fail': True, @@ -632,8 +634,8 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams, anchors) opts = {'dev-disable-commit-after': 3, 'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # First we need to get funds to l2, so suppress after second. # Feerates identical so we don't get gratuitous commit to update them @@ -764,11 +766,9 @@ def test_channel_lease_falls_behind(node_factory, bitcoind): their blockheight, the lessor fails the channel ''' opts = [{'funder-policy': 'match', 'funder-policy-mod': 100, - 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'experimental-anchors': None}, + 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100}, {'funder-policy': 'match', 'funder-policy-mod': 100, - 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'experimental-anchors': None}] + 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100}] l1, l2, = node_factory.get_nodes(2, opts=opts) amount = 500000 @@ -805,8 +805,7 @@ def test_channel_lease_post_expiry(node_factory, bitcoind, chainparams): opts = {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'may_reconnect': True, 'plugin': coin_mvt_plugin, - 'dev-no-reconnect': None, - 'experimental-anchors': None} + 'dev-no-reconnect': None} l1, l2, = node_factory.get_nodes(2, opts=opts) @@ -909,8 +908,7 @@ def test_channel_lease_unilat_closes(node_factory, bitcoind): ''' opts = {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'funder-lease-requests-only': False, - 'experimental-anchors': None} + 'funder-lease-requests-only': False} l1, l2, l3 = node_factory.get_nodes(3, opts=opts) # Allow l2 some warnings @@ -1020,12 +1018,10 @@ def test_channel_lease_lessor_cheat(node_factory, bitcoind, chainparams): opts = [{'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'may_reconnect': True, 'allow_warning': True, - 'experimental-anchors': None, 'plugin': balance_snaps}, {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'may_reconnect': True, 'allow_broken_log': True, - 'experimental-anchors': None, 'plugin': balance_snaps}] l1, l2, = node_factory.get_nodes(2, opts=opts) @@ -1192,21 +1188,25 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams, anchors): coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') balance_snaps = os.path.join(os.getcwd(), 'tests/plugins/balance_snaps.py') + opts = {'may_reconnect': True, + 'dev-no-reconnect': None} + if anchors: + commitfee = 3755 + else: + commitfee = 11005 + opts = {**opts, 'dev-force-features': "-23"} + l1, l2, l3, l4 = node_factory.line_graph(4, opts=[{'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC'], - 'may_reconnect': True, - 'dev-no-reconnect': None}, + **opts}, {'plugin': [coin_mvt_plugin, balance_snaps], 'disable-mpp': None, - 'dev-no-reconnect': None, - 'may_reconnect': True, + **opts, 'allow_broken_log': True}, {'plugin': [coin_mvt_plugin, balance_snaps], - 'dev-no-reconnect': None, - 'may_reconnect': True, + **opts, 'allow_broken_log': True}, - {'dev-no-reconnect': None, - 'may_reconnect': True}], + opts], wait_for_announce=True) channel_id = first_channel_id(l2, l3) @@ -1257,7 +1257,7 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams, anchors): # reconnect with l1, which will fulfill the payment l2.rpc.connect(l1.info['id'], 'localhost', l1.port) - l2.daemon.wait_for_log('got commitsig .*: feerate 11005, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed') + l2.daemon.wait_for_log('got commitsig .*: feerate {}, blockheight: 0, 0 added, 1 fulfilled, 0 failed, 0 changed'.format(commitfee)) # l2 moves on for closed l3 bitcoind.generate_block(1) @@ -1403,9 +1403,9 @@ def test_penalty_htlc_tx_timeout(node_factory, bitcoind, chainparams, anchors): 'allow_broken_log': True, } ] - if anchors: + if anchors is False: for opt in opts: - opt['experimental-anchors'] = None + opt['dev-force-features'] = "-23" l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=opts) @@ -1586,8 +1586,8 @@ def test_penalty_rbf_normal(node_factory, bitcoind, executor, chainparams, ancho coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') to_self_delay = 10 opts = {'dev-disable-commit-after': 1} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # l1 is the thief, which causes our honest upstanding lightningd # code to break, so l1 can fail. @@ -1938,8 +1938,8 @@ def test_onchain_timeout(node_factory, bitcoind, executor, chainparams, anchors) coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') opts = {'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # HTLC 1->2, 1 fails just after it's irrevocably committed disconnects = ['+WIRE_REVOKE_AND_ACK*3', 'permfail'] @@ -2063,8 +2063,8 @@ def test_onchain_middleman_simple(node_factory, bitcoind, chainparams, anchors): coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') opts = {'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # HTLC 1->2->3, 1->2 goes down after 2 gets preimage from 3. disconnects = ['-WIRE_UPDATE_FULFILL_HTLC', 'permfail'] @@ -2198,8 +2198,8 @@ def test_onchain_middleman_their_unilateral_in(node_factory, bitcoind, chainpara coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') opts = {'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1_disconnects = ['=WIRE_UPDATE_FULFILL_HTLC', 'permfail'] l2_disconnects = ['-WIRE_UPDATE_FULFILL_HTLC'] @@ -2328,8 +2328,8 @@ def test_onchain_their_unilateral_out(node_factory, bitcoind, chainparams, ancho # We track channel balances, to verify that accounting is ok. coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') opts = {'plugin': coin_mvt_plugin} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail'] @@ -3364,8 +3364,8 @@ def test_closing_higherfee(node_factory, bitcoind, executor, anchors): opts = {'may_reconnect': True, 'dev-no-reconnect': None, 'feerates': (7500, 7500, 7500, 7500)} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" # We change the feerate before it starts negotiating close, so it aims # for *higher* than last commit tx. @@ -3739,10 +3739,8 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): # We want an outstanding HTLC for l1, so it uses anchor to push. # Set feerates to lowball for now. l1, l2 = node_factory.line_graph(2, opts=[{'feerates': (1000,) * 4, - 'experimental-anchors': None, 'min-emergency-msat': 546000}, {'feerates': (1000,) * 4, - 'experimental-anchors': None, 'disconnect': ['-WIRE_UPDATE_FAIL_HTLC']}]) assert 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names'] @@ -3821,9 +3819,9 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): def test_htlc_no_force_close(node_factory, bitcoind, anchors): """l2<->l3 force closes while an HTLC is in flight from l1, but l2 can't timeout because the feerate has spiked. It should do so anyway.""" opts = [{}, {}, {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC']}] - if anchors: + if anchors is False: for opt in opts: - opt['experimental-anchors'] = None + opt['dev-force-features'] = "-23" l1, l2, l3 = node_factory.line_graph(3, opts=opts) @@ -3933,10 +3931,8 @@ def test_closing_minfee(node_factory, bitcoind): def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): """Test that we use anchor on peer's commit to CPFP tx""" l1, l2, l3 = node_factory.line_graph(3, opts=[{}, - {'experimental-anchors': None, - 'min-emergency-msat': 546000}, - {'experimental-anchors': None, - 'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC']}], + {'min-emergency-msat': 546000}, + {'disconnect': ['-WIRE_UPDATE_FULFILL_HTLC']}], wait_for_announce=True) # We splinter l2's funds so it's forced to use more than one UTXO to push. diff --git a/tests/test_connection.py b/tests/test_connection.py index b5518d99a1ca..29f912aa1599 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -388,9 +388,9 @@ def test_opening_tiny_channel(node_factory, anchors): {'min-capacity-sat': l2_min_capacity, 'dev-no-reconnect': None}, {'min-capacity-sat': l3_min_capacity, 'dev-no-reconnect': None}, {'min-capacity-sat': l4_min_capacity, 'dev-no-reconnect': None}] - if anchors: + if anchors is False: for opt in opts: - opt['experimental-anchors'] = None + opt['dev-force-features'] = "-23" l1, l2, l3, l4 = node_factory.get_nodes(4, opts=opts) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) @@ -2061,8 +2061,8 @@ def test_multifunding_feerates(node_factory, bitcoind, anchors): commitment_tx_feerate = str(commitment_tx_feerate_int) + 'perkw' opts = {'log-level': 'debug'} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1, l2, l3 = node_factory.get_nodes(3, opts=opts) l1.fundwallet(1 << 26) @@ -2472,18 +2472,16 @@ def test_fee_limits(node_factory, bitcoind): if 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names']: fee = 1255 - range = '2000-75000' else: fee = 258 - range = '1875-75000' - l1.daemon.wait_for_log(f'Received WARNING .*: update_fee {fee} outside range {range}') + l1.daemon.wait_for_log(f'Received WARNING .*: update_fee {fee} outside range 2000-75000') # They hang up on *us* l1.daemon.wait_for_log('Peer transient failure in CHANNELD_NORMAL: channeld: Owning subdaemon channeld died') # Disconnects, but does not error. Make sure it's noted in their status though. # FIXME: does not happen for l1! # assert 'update_fee 253 outside range 1875-75000' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'][0] - assert f'update_fee {fee} outside range {range}' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0] + assert f'update_fee {fee} outside range 2000-75000' in only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['status'][0] assert only_one(l2.rpc.listpeerchannels()['channels'])['feerate']['perkw'] != fee # Make l2 accept those fees, and it should recover. @@ -2537,6 +2535,7 @@ def test_fee_limits(node_factory, bitcoind): l1.rpc.close(chan) +@unittest.skipIf(TEST_NETWORK != 'regtest', 'Assumes anchors') def test_update_fee_dynamic(node_factory, bitcoind): # l1 has no fee estimates to start. l1 = node_factory.get_node(options={'log-level': 'io', @@ -2552,8 +2551,14 @@ def test_update_fee_dynamic(node_factory, bitcoind): with pytest.raises(RpcError, match='Cannot estimate fees'): l1.fundchannel(l2, 10**6) - # Explicit feerate works. - l1.fundchannel(l2, 10**6, feerate='10000perkw') + # Explicit feerate does not work still (doesn't apply for anchors!) + # We could make it, but we need a separate commitment feerate for + # anchors vs non-anchors, so easier after anchors are compulsory. + with pytest.raises(RpcError, match='Cannot estimate fees'): + l1.fundchannel(l2, 10**6, feerate='10000perkw') + + l1.set_feerates((2000, 2000, 2000, 2000)) + l1.fundchannel(l2, 10**6) l1.set_feerates((15000, 11000, 7500, 3750)) @@ -2905,18 +2910,11 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): with pytest.raises(RpcError, match=r'Cannot estimate fees'): l1.rpc.fundchannel(l2.info['id'], 10**6, 'slow') - # Can with manual feerate. + # With anchors, not even with manual feerate! l1.rpc.withdraw(l2.rpc.newaddr()['bech32'], 10000, '1500perkb') - l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw', minconf=0) - - # Make sure we clean up cahnnel for later attempt. - l1.daemon.wait_for_log('sendrawtx exit 0') - l1.rpc.dev_fail(l2.info['id']) - l1.daemon.wait_for_log('Failing due to dev-fail command') - l1.wait_for_channel_onchain(l2.info['id']) - bitcoind.generate_block(6) - wait_for(lambda: only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['state'] == 'ONCHAIN') - wait_for(lambda: only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])['state'] == 'ONCHAIN') + if TEST_NETWORK == 'regtest': + with pytest.raises(RpcError, match=r'Cannot estimate fees'): + l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw', minconf=0) # But can accept incoming connections. l1.rpc.connect(l2.info['id'], 'localhost', l2.port) @@ -3559,10 +3557,17 @@ def test_wumbo_channels(node_factory, bitcoind): @pytest.mark.openchannel('v2') @pytest.mark.parametrize("anchors", [False, True]) def test_channel_features(node_factory, bitcoind, anchors): - if anchors: - opts = {'experimental-anchors': None} + if TEST_NETWORK == 'regtest': + if anchors is False: + opts = {'dev-force-features': "-23"} + else: + opts = {} else: - opts = {} + # We have to force this ON for elements! + if anchors is False: + opts = {} + else: + opts = {'dev-force-features': "+23"} l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts) bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], 0.1) diff --git a/tests/test_misc.py b/tests/test_misc.py index f7d4c8f3a23b..a244a4de8f1d 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1590,8 +1590,8 @@ def test_ipv4_and_ipv6(node_factory): def test_feerates(node_factory, anchors): opts = {'log-level': 'io', 'dev-no-fake-fees': True} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1 = node_factory.get_node(options=opts, start=False) l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { @@ -2043,8 +2043,8 @@ def mock_fail(*args): def test_bitcoind_feerate_floor(node_factory, bitcoind, anchors): """Don't return a feerate less than minrelaytxfee/mempoolminfee.""" opts = {} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1 = node_factory.get_node(options=opts) assert l1.rpc.feerates('perkb') == { @@ -2254,6 +2254,7 @@ def test_list_features_only(node_factory): 'option_payment_secret/even', 'option_basic_mpp/odd', 'option_support_large_channel/odd', + 'option_anchors_zero_fee_htlc_tx/odd', 'option_route_blinding/odd', 'option_shutdown_anysegwit/odd', 'option_channel_type/odd', diff --git a/tests/test_opening.py b/tests/test_opening.py index 80f3ed050248..1569aff10f9e 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -22,8 +22,7 @@ def find_next_feerate(node, peer): @pytest.mark.openchannel('v2') def test_queryrates(node_factory, bitcoind): - opts = {'dev-no-reconnect': None, - 'experimental-anchors': None} + opts = {'dev-no-reconnect': None} l1, l2 = node_factory.get_nodes(2, opts=opts) @@ -581,7 +580,6 @@ def test_v2_rbf_liquidity_ad(node_factory, bitcoind, chainparams): opts = {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'experimental-anchors': None, 'may_reconnect': True} l1, l2 = node_factory.get_nodes(2, opts=opts) @@ -1554,13 +1552,11 @@ def test_inflight_dbload(node_factory, bitcoind): disconnects = ["@WIRE_COMMITMENT_SIGNED"] opts = [{'experimental-dual-fund': None, 'dev-no-reconnect': None, - 'may_reconnect': True, 'disconnect': disconnects, - 'experimental-anchors': None}, + 'may_reconnect': True, 'disconnect': disconnects}, {'experimental-dual-fund': None, 'dev-no-reconnect': None, 'may_reconnect': True, 'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', - 'lease-fee-basis': 100, - 'experimental-anchors': None}] + 'lease-fee-basis': 100}] l1, l2 = node_factory.get_nodes(2, opts=opts) @@ -1657,8 +1653,12 @@ def test_zeroconf_open(bitcoind, node_factory): # and use their own mindepth=6, while l2 uses mindepth=2 from the # plugin ret = l1.rpc.fundchannel(l2.info['id'], 'all', mindepth=0) - assert ret['channel_type'] == {'bits': [12, 50], 'names': ['static_remotekey/even', 'zeroconf/even']} - assert only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['channel_type'] == {'bits': [12, 50], 'names': ['static_remotekey/even', 'zeroconf/even']} + if TEST_NETWORK == 'regtest': + channel_type = {'bits': [12, 22, 50], 'names': ['static_remotekey/even', 'anchors_zero_fee_htlc_tx/even', 'zeroconf/even']} + else: + channel_type = {'bits': [12, 50], 'names': ['static_remotekey/even', 'zeroconf/even']} + assert ret['channel_type'] == channel_type + assert only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['channel_type'] == channel_type assert l1.db.query('SELECT minimum_depth FROM channels WHERE minimum_depth != 1') == [{'minimum_depth': 0}] assert l2.db.query('SELECT minimum_depth FROM channels') == [{'minimum_depth': 0}] @@ -1737,7 +1737,7 @@ def test_zeroconf_public(bitcoind, node_factory, chainparams): assert('short_channel_id' not in l2chan) # Channel is "proposed" - chan_val = 993888000 if chainparams['elements'] else 996363000 + chan_val = 993888000 if chainparams['elements'] else 970073000 l1_mvts = [ {'type': 'chain_mvt', 'credit_msat': chan_val, 'debit_msat': 0, 'tags': ['channel_proposed', 'opener']}, {'type': 'channel_mvt', 'credit_msat': 0, 'debit_msat': 20000000, 'tags': ['pushed'], 'fees_msat': '0msat'}, @@ -1875,12 +1875,10 @@ def test_v2_replay_bookkeeping(node_factory, bitcoind): opts = [{'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'rescan': 10, 'funding-confirms': 6, 'may_reconnect': True, - 'experimental-anchors': None}, + 'rescan': 10, 'funding-confirms': 6, 'may_reconnect': True}, {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'may_reconnect': True, - 'experimental-anchors': None}] + 'may_reconnect': True}] l1, l2, = node_factory.get_nodes(2, opts=opts) amount = 500000 @@ -1938,12 +1936,10 @@ def test_buy_liquidity_ad_check_bookkeeping(node_factory, bitcoind): opts = [{'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'rescan': 10, 'disable-plugin': 'bookkeeper', - 'funding-confirms': 6, 'may_reconnect': True, - 'experimental-anchors': None}, + 'funding-confirms': 6, 'may_reconnect': True}, {'funder-policy': 'match', 'funder-policy-mod': 100, 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, - 'may_reconnect': True, - 'experimental-anchors': None}] + 'may_reconnect': True}] l1, l2, = node_factory.get_nodes(2, opts=opts) amount = 500000 @@ -2434,7 +2430,7 @@ def test_no_anchor_liquidity_ads(node_factory, bitcoind): 'lease-fee-base-sat': '100sat', 'lease-fee-basis': 100, 'may_reconnect': True, 'funder-lease-requests-only': False} l1_opts = l2_opts.copy() - l1_opts['experimental-anchors'] = None + l2_opts['dev-no-anchors'] = None l1, l2 = node_factory.get_nodes(2, opts=[l1_opts, l2_opts]) feerate = 2000 @@ -2465,8 +2461,8 @@ def test_no_anchor_liquidity_ads(node_factory, bitcoind): @pytest.mark.parametrize("anchors", [False, True]) def test_commitment_feerate(bitcoind, node_factory, anchors): opts = {} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1, l2 = node_factory.get_nodes(2, opts=opts) @@ -2516,8 +2512,7 @@ def test_commitment_feerate(bitcoind, node_factory, anchors): @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd has different tx costs') def test_anchor_min_emergency(bitcoind, node_factory): - l1, l2 = node_factory.line_graph(2, opts={'experimental-anchors': None}, - fundchannel=False) + l1, l2 = node_factory.line_graph(2, fundchannel=False) addr = l1.rpc.newaddr()['bech32'] bitcoind.rpc.sendtoaddress(addr, 5000000 / 10**8) diff --git a/tests/test_pay.py b/tests/test_pay.py index ade0abfe2052..dbd28035807e 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -755,8 +755,8 @@ def test_wait_sendpay(node_factory, executor): def test_sendpay_cant_afford(node_factory, anchors): # Set feerates the same so we don't have to wait for update. opts = {'feerates': (15000, 15000, 15000, 15000)} - if anchors: - opts['experimental-anchors'] = None + if anchors is False: + opts['dev-force-features'] = "-23" l1, l2 = node_factory.line_graph(2, fundamount=10**6, opts=opts) @@ -2686,17 +2686,16 @@ def test_lockup_drain(node_factory, bitcoind): l2.pay(l1, total // 2) +@unittest.skipIf(TEST_NETWORK != 'regtest', 'Assumes anchors') def test_htlc_too_dusty_outgoing(node_factory, bitcoind, chainparams): """ Try to hit the 'too much dust' limit, should fail the HTLC """ - feerate = 10000 # elements txs are bigger so they become dusty faster - max_dust_limit_sat = 100000 if chainparams['elements'] else 50000 - non_dust_htlc_val_sat = 20000 if chainparams['elements'] else 10000 - htlc_val_sat = 10000 if chainparams['elements'] else 5000 + max_dust_limit_sat = 1000 if chainparams['elements'] else 500 + non_dust_htlc_val_sat = 2000 if chainparams['elements'] else 1000 + htlc_val_sat = 250 l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True, - 'feerates': (feerate, feerate, feerate, feerate), 'max-dust-htlc-exposure-msat': '{}sat'.format(max_dust_limit_sat), 'allow_warning': True}) diff --git a/tests/utils.py b/tests/utils.py index 2f9e903ffd5f..6a109b80915a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -43,6 +43,9 @@ def expected_peer_features(extra=[]): if EXPERIMENTAL_SPLICING: features += [35] # option_quiesce features += [163] # option_experimental_splice + if TEST_NETWORK != 'liquid-regtest': + # Anchors, except for elements + features += [23] return hex_bits(features + extra) @@ -57,6 +60,9 @@ def expected_node_features(extra=[]): if EXPERIMENTAL_SPLICING: features += [35] # option_quiesce features += [163] # option_experimental_splice + if TEST_NETWORK != 'liquid-regtest': + # Anchors, except for elements + features += [23] return hex_bits(features + extra) From 16ab95daa1357e6fddc2dbf84909e1f3bb63c945 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Feb 2024 14:43:29 +1030 Subject: [PATCH 7/8] pytest: fix bad gossip flake in test_closing_specified_destination. Signed-off-by: Rusty Russell --- tests/test_closing.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_closing.py b/tests/test_closing.py index 4d7e2fc2ea84..062c4bbcc000 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -303,6 +303,12 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams): wait_for(lambda: len(n.rpc.listchannels()['channels']) == 6) wait_for(lambda: ['alias' in node for node in n.rpc.listnodes()['nodes']] == [True] * 4) + # If we don't wait for gossip to propagate, then we can get bad gossip msgs if it + # propagates after close! + for n in l1, l2, l3, l4: + wait_for(lambda: len(n.rpc.listchannels()['channels']) == 6) + wait_for(lambda: ['alias' in node for node in n.rpc.listnodes()['nodes']] == [True] * 4) + addr = chainparams['example_addr'] l1.rpc.close(chan12, None, addr) l1.rpc.call('close', {'id': chan13, 'destination': addr}) From 28070b25ff0e1481362e8e63f8fbc0a84faca854 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 7 Feb 2024 09:17:15 +1030 Subject: [PATCH 8/8] pytest: fix flake in test_htlc_in_timeout with anchors. If we try to reuse the same UTXO for the HTLC as the anchor, it will clash. One block later we can spend the anchor change, all good. Signed-off-by: Rusty Russell --- tests/test_misc.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/test_misc.py b/tests/test_misc.py index a244a4de8f1d..fa6e45ddce0e 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -359,7 +359,14 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor): # We hit deadline (we give 1 block grace), then mined another. assert blocks2 == -2 - bitcoind.generate_block(1, wait_for_mempool=txid) + # If we try to reuse the same output as we used for the anchor spend, then + # bitcoind can reject it. In that case we'll try again after we get change + # from anchor spend. + if txid not in bitcoind.rpc.getrawmempool(): + bitcoind.generate_block(1) + bitcoind.generate_block(1, wait_for_mempool=1) + else: + bitcoind.generate_block(1) rawtx, txid, blocks = l1.wait_for_onchaind_tx('OUR_DELAYED_RETURN_TO_WALLET', 'OUR_HTLC_TIMEOUT_TX/DELAYED_OUTPUT_TO_US') @@ -424,7 +431,16 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor): _, txid, blocks = l2.wait_for_onchaind_tx('OUR_HTLC_SUCCESS_TX', 'OUR_UNILATERAL/THEIR_HTLC') assert blocks == 0 - bitcoind.generate_block(1, wait_for_mempool=txid) + + # If we try to reuse the same output as we used for the anchor spend, then + # bitcoind can reject it. In that case we'll try again after we get change + # from anchor spend. + if txid not in bitcoind.rpc.getrawmempool(): + bitcoind.generate_block(1) + bitcoind.generate_block(1, wait_for_mempool=1) + else: + bitcoind.generate_block(1) + rawtx, txid, blocks = l2.wait_for_onchaind_tx('OUR_DELAYED_RETURN_TO_WALLET', 'OUR_HTLC_SUCCESS_TX/DELAYED_OUTPUT_TO_US') assert blocks == 4