Fix crash with dual opening when we cannot estimate fees

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;
 

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;

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);

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) ?

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);
 }

tests/test_connection.py

@@ -2884,15 +2884,16 @@ 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', {
         'error': {"errors": ["Insufficient data or no feerate found"], "blocks": 0}
     })
     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.

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")