diff --git a/common/fee_states.c b/common/fee_states.c index 3d1a8976e664..1626ea824c0b 100644 --- a/common/fee_states.c +++ b/common/fee_states.c @@ -172,7 +172,9 @@ u32 marginal_feerate(u32 current_feerate) if (current_feerate == 0) return 0; #endif - assert(current_feerate >= minfeerate); + /* This could happen in future if we celebrate sub-sat summer! */ + if (current_feerate < minfeerate) + current_feerate = minfeerate; if (current_feerate > maxfeerate) return current_feerate * 1.1; diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 25544f906e27..775658e1f365 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -381,6 +381,11 @@ static void watch_for_unconfirmed_txs(struct lightningd *ld, /* Mutual recursion via timer. */ static void next_updatefee_timer(struct chain_topology *topo); +bool unknown_feerates(const struct chain_topology *topo) +{ + return tal_count(topo->feerates[0]) == 0; +} + static u32 interp_feerate(const struct feerate_est *rates, u32 blockcount) { const struct feerate_est *before = NULL, *after = NULL; diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 8515cb5bdaf3..ff0a448bdad3 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -183,6 +183,9 @@ u32 smoothed_feerate_for_deadline(const struct chain_topology *topo, u32 blockco /* Get feerate to hit this *block number*. */ u32 feerate_for_target(const struct chain_topology *topo, u64 deadline); +/* Has our feerate estimation failed altogether? */ +bool unknown_feerates(const struct chain_topology *topo); + /* Get range of feerates to insist other side abide by for normal channels. * If we have to guess, sets *unknown to true, otherwise false. */ u32 feerate_min(struct lightningd *ld, bool *unknown); diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index a605a528fe76..e3b3bf95648e 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -2090,6 +2090,15 @@ static void accepter_got_offer(struct subd *dualopend, return; } + /* Don't allow opening if we don't know any fees; even if + * ignore-feerates is set. */ + if (unknown_feerates(dualopend->ld->topology)) { + subd_send_msg(dualopend, + take(towire_dualopend_fail(NULL, "Cannot accept channel: feerates unknown"))); + tal_free(payload); + return; + } + /* As a convenience to the plugin, we provide our current known * min + max feerates. Ideally, the plugin will fail to * contribute funds if the peer's feerate range is outside of @@ -3046,6 +3055,7 @@ static struct command_result *openchannel_init(struct command *cmd, { u32 *our_upfront_shutdown_script_wallet_index; u32 found_wallet_index; + u32 anchor_feerate; struct channel *channel; struct open_attempt *oa; int fds[2]; @@ -3093,12 +3103,19 @@ static struct command_result *openchannel_init(struct command *cmd, } else our_upfront_shutdown_script_wallet_index = NULL; + /* 0 from this means "unknown" */ + anchor_feerate = unilateral_feerate(cmd->ld->topology, true); + if (anchor_feerate == 0) { + anchor_feerate = get_feerate_floor(cmd->ld->topology); + assert(anchor_feerate); + } + oa->open_msg = towire_dualopend_opener_init(oa, psbt, amount, oa->our_upfront_shutdown_script, our_upfront_shutdown_script_wallet_index, feerate_per_kw, - unilateral_feerate(cmd->ld->topology, true), + anchor_feerate, feerate_per_kw_funding, channel->channel_flags, amount_sat_is_zero(request_amt) ? @@ -3803,7 +3820,7 @@ static struct command_result *json_queryrates(struct command *cmd, struct peer *peer; struct channel *channel; u32 *feerate_per_kw_funding; - u32 *feerate_per_kw; + u32 *feerate_per_kw, anchor_feerate; struct amount_sat *amount, *request_amt; struct wally_psbt *psbt; struct open_attempt *oa; @@ -3891,12 +3908,19 @@ static struct command_result *json_queryrates(struct command *cmd, } else our_upfront_shutdown_script_wallet_index = NULL; + /* 0 from this means "unknown" */ + anchor_feerate = unilateral_feerate(cmd->ld->topology, true); + if (anchor_feerate == 0) { + anchor_feerate = get_feerate_floor(cmd->ld->topology); + assert(anchor_feerate); + } + oa->open_msg = towire_dualopend_opener_init(oa, psbt, *amount, oa->our_upfront_shutdown_script, our_upfront_shutdown_script_wallet_index, *feerate_per_kw, - unilateral_feerate(cmd->ld->topology, true), + anchor_feerate, *feerate_per_kw_funding, channel->channel_flags, amount_sat_is_zero(*request_amt) ? diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index ae302630f071..c6f06071e29e 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -871,6 +871,16 @@ static void opening_got_offer(struct subd *openingd, return; } + /* Don't allow opening if we don't know any fees; even if + * ignore-feerates is set. */ + if (unknown_feerates(openingd->ld->topology)) { + subd_send_msg(openingd, + take(towire_openingd_got_offer_reply(NULL, "Cannot accept channel: feerates unknown", + NULL, NULL, NULL, 0))); + tal_free(payload); + return; + } + tal_add_destructor2(openingd, openchannel_payload_remove_openingd, payload); plugin_hook_call_openchannel(openingd->ld, NULL, payload); } diff --git a/tests/test_connection.py b/tests/test_connection.py index dece3e1a87d2..2aef856860ff 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -2884,7 +2884,8 @@ def mock_donothing(r): def test_no_fee_estimate(node_factory, bitcoind, executor): - l1 = node_factory.get_node(start=False, options={'dev-no-fake-fees': True}) + l1 = node_factory.get_node(start=False, options={'dev-no-fake-fees': True}, + may_reconnect=True) # Fail any fee estimation requests until we allow them further down l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { @@ -2892,7 +2893,7 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): }) l1.start() - l2 = node_factory.get_node() + l2 = node_factory.get_node(may_reconnect=True) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) # Can't fund a channel. @@ -2929,9 +2930,19 @@ 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, '2000perkw', minconf=0) - # But can accept incoming connections. + # Can accept incoming connections, can't allow incoming channels l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + with pytest.raises(RpcError, match=r'feerates unknown'): + l2.fundchannel(l1, 10**6) + + # Re-enable fees for a moment so we can fund channel. + l1.set_feerates((15000, 11000, 7500, 3750), True) l2.fundchannel(l1, 10**6) + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { + 'error': {"errors": ["Insufficient data or no feerate found"], "blocks": 0} + }) + l1.restart() + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) # Can do HTLCs. l2.pay(l1, 10**5) @@ -2943,8 +2954,14 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): sync_blockheight(bitcoind, [l1, l2]) # Can do unilateral close. - l2.rpc.connect(l1.info['id'], 'localhost', l1.port) + l1.set_feerates((15000, 11000, 7500, 3750), True) l2.fundchannel(l1, 10**6) + l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', { + 'error': {"errors": ["Insufficient data or no feerate found"], "blocks": 0} + }) + l1.restart() + l2.rpc.connect(l1.info['id'], 'localhost', l1.port) + l2.pay(l1, 10**9 // 2) l1.rpc.dev_fail(l2.info['id']) l1.daemon.wait_for_log('Failing due to dev-fail command') @@ -2954,18 +2971,6 @@ def test_no_fee_estimate(node_factory, bitcoind, executor): bitcoind.generate_block(100) sync_blockheight(bitcoind, [l1, l2]) - # Start estimatesmartfee. - l1.set_feerates((15000, 11000, 7500, 3750), True) - - # Can now fund a channel (as a test, use slow feerate). - l1.rpc.connect(l2.info['id'], 'localhost', l2.port) - sync_blockheight(bitcoind, [l1]) - l1.rpc.fundchannel(l2.info['id'], 10**6, 'slow') - - # Can withdraw (use urgent feerate). `minconf` may be needed depending on - # the previous `fundchannel` selecting all confirmed outputs. - l1.rpc.withdraw(l2.rpc.newaddr('bech32')['bech32'], 'all', 'urgent', minconf=0) - def test_opener_feerate_reconnect(node_factory, bitcoind): # l1 updates fees, then reconnect so l2 retransmits commitment_signed. diff --git a/tests/test_opening.py b/tests/test_opening.py index 13863df45b92..4954c31b87b4 100644 --- a/tests/test_opening.py +++ b/tests/test_opening.py @@ -2929,6 +2929,31 @@ def test_zeroconf_withhold(node_factory, bitcoind, stay_withheld, mutual_close): wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['state'] == 'AWAITING_UNILATERAL') +@pytest.mark.openchannel('v1') +@pytest.mark.openchannel('v2') +def test_opening_incoming_unknown_feerates(node_factory, bitcoind): + """ + Don't allow incoming channels if we can't estimate feerates. + """ + nofee_opts = {'ignore-fee-limits': True, + 'feerates': None, + 'dev-no-fake-fees': True} + + l1, l2 = node_factory.get_nodes(2, opts=[{}, nofee_opts]) + + l1.fundwallet(FUNDAMOUNT) + + # Connect peers + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + + # Verify fee estimation is failing + l2.daemon.wait_for_log('Unable to estimate any fees') + + # Open channel l1 <-> l2: l2 should refuse! + with pytest.raises(RpcError, match=r'They sent.*Cannot accept channel: feerates unknown'): + l1.rpc.fundchannel(l2.info['id'], 100000) + + def test_zeroconf_withhold_htlc_failback(node_factory, bitcoind): """Test that CLTV timeout on a withheld channel fails HTLCs back upstream without force-close.""" zeroconf_plugin = str(Path(__file__).parent / "plugins" / "zeroconf-selective.py")