splice: Make feerate opening + feerate_offset

Splicing feerate was being derived by `default_feerate` which was actually `unilateral_feerate`, which is incorrect.

Instead we define an explicit `splice_feerate` in the chain topology that is `opening` + `feerate_offset`.

This slipped through testing because regtest isn’t a great test of fees we see in the wild.

Changelog-Fixed: Fixes high fee issue that caused `splicein` and `spliceout` to abort.

Author: ddustin

channeld/channeld.c

@@ -3600,9 +3600,9 @@ static struct amount_sat check_balances(struct peer *peer,
 	/* As a safeguard max feerate is checked (only) locally, if it's
 	 * particularly high we fail and tell the user but allow them to
 	 * override with `splice_force_feerate` */
-	max_accepter_fee = amount_tx_fee(peer->feerate_opening,
+	max_accepter_fee = amount_tx_fee(peer->feerate_max,
 					 calc_weight(TX_ACCEPTER, psbt, false));
-	max_initiator_fee = amount_tx_fee(peer->feerate_opening,
+	max_initiator_fee = amount_tx_fee(peer->feerate_max,
 					  calc_weight(TX_INITIATOR, psbt, opener));
 
 	if (opener) {
@@ -3620,10 +3620,13 @@ static struct amount_sat check_balances(struct peer *peer,
 							   false);
 		wire_sync_write(MASTER_FD, take(msg));
 		splice_abort(peer, NULL,
-				 "%s fee (%s) was too low, must be at least %s",
+				 "%s fee (%s) was too low, must be at least %s"
+				 " weight: %"PRIu64", splicing->feerate_per_kw: %"PRIu32,
 				 opener ? "Our" : "Your",
 				 fmt_amount_msat(tmpctx, initiator_fee),
-				 fmt_amount_sat(tmpctx, min_initiator_fee));
+				 fmt_amount_sat(tmpctx, min_initiator_fee),
+				 calc_weight(TX_INITIATOR, psbt, false),
+				 peer->splicing->feerate_per_kw);
 	}
 	if (!peer->splicing->force_feerate && opener
 		&& amount_msat_greater_sat(initiator_fee, max_initiator_fee)) {
@@ -3660,7 +3663,7 @@ static struct amount_sat check_balances(struct peer *peer,
 			     fmt_amount_msat(tmpctx, accepter_fee),
 			     fmt_amount_sat(tmpctx, min_accepter_fee),
 			     calc_weight(TX_INITIATOR, psbt, false),
-			     peer->feerate_opening);
+			     peer->feerate_max);
 	}
 	if (!peer->splicing->force_feerate && !opener
 		&& amount_msat_greater_sat(accepter_fee, max_accepter_fee)) {

lightningd/chaintopology.c

@@ -617,6 +617,15 @@ u32 opening_feerate(struct chain_topology *topo)
 				    conversions[FEERATE_OPENING].blockcount);
 }
 
+u32 splice_feerate(struct chain_topology *topo, struct lightningd *ld)
+{
+	u32 rate = opening_feerate(topo);
+	rate += ld->config.feerate_offset;
+	if (rate > feerate_max(ld, NULL))
+		rate = feerate_max(ld, NULL);
+	return rate;
+}
+
 u32 mutual_close_feerate(struct chain_topology *topo)
 {
 	if (topo->ld->force_feerates)
@@ -717,14 +726,9 @@ static struct command_result *json_feerates(struct command *cmd,
 	if (rate)
 		json_add_num(response, "penalty",
 			     feerate_to_style(rate, *style));
-	rate = unilateral_feerate(topo, true);
-	if (rate) {
-		rate += cmd->ld->config.feerate_offset;
-		if (rate > feerate_max(cmd->ld, NULL))
-			rate = feerate_max(cmd->ld, NULL);
-		json_add_num(response, "splice",
-			     feerate_to_style(rate, *style));
-	}
+	rate = splice_feerate(topo, cmd->ld);
+	if (rate)
+		json_add_num(response, "splice", feerate_to_style(rate, *style));
 
 	json_add_u64(response, "min_acceptable",
 		     feerate_to_style(feerate_min(cmd->ld, NULL), *style));

lightningd/chaintopology.h

@@ -196,6 +196,7 @@ u32 feerate_max(struct lightningd *ld, bool *unknown);
 
 /* These return 0 if unknown */
 u32 opening_feerate(struct chain_topology *topo);
+u32 splice_feerate(struct chain_topology *topo, struct lightningd *ld);
 u32 mutual_close_feerate(struct chain_topology *topo);
 u32 unilateral_feerate(struct chain_topology *topo, bool option_anchors);
 /* For onchain resolution. */

lightningd/channel_control.c

@@ -81,7 +81,7 @@ void channel_update_feerates(struct lightningd *ld, const struct channel *channe
 	u32 min_feerate, max_feerate;
 	bool anchors = channel_type_has_anchors(channel->type);
 	u32 feerate = default_feerate(ld, channel, (channel->opener == LOCAL));
-	u32 feerate_splice = default_feerate(ld, channel, true);
+	u32 feerate_splice = splice_feerate(ld->topology, ld);
 
 	/* Nothing to do if we don't know feerate. */
 	if (!feerate)
@@ -1900,7 +1900,7 @@ bool peer_start_channeld(struct channel *channel,
 		tal_arr_expand(&inflights, infcopy);
 	}
 
-	feerate_splice = default_feerate(ld, channel, true);
+	feerate_splice = splice_feerate(ld->topology, ld);
 
 	initmsg = towire_channeld_init(tmpctx,
 				       chainparams,
@@ -2347,7 +2347,7 @@ static struct command_result *json_splice_init(struct command *cmd,
 
 	if (!feerate_per_kw) {
 		feerate_per_kw = tal(cmd, u32);
-		*feerate_per_kw = default_feerate(cmd->ld, channel, true);
+		*feerate_per_kw = splice_feerate(cmd->ld->topology, cmd->ld);
 	}
 
 	if (!initialpsbt)
@@ -2720,7 +2720,7 @@ static struct command_result *json_dev_feerate(struct command *cmd,
 				       feerate_max(cmd->ld, NULL),
 				       penalty_feerate(cmd->ld->topology),
 				       opening_feerate(cmd->ld->topology),
-				       default_feerate(cmd->ld, channel, true));
+				       splice_feerate(cmd->ld->topology, cmd->ld));
 	subd_send_msg(channel->owner, take(msg));
 
 	response = json_stream_success(cmd);

tests/test_splice.py

@@ -488,9 +488,9 @@ def test_script_two_chan_splice_inout(node_factory, bitcoind):
     chan_id2 = l2.get_channel_id(l3)
 
     # move sats from chan 2 into chan 1
-    # By adding 10000 from wallet, the fee will be taken from this and the
+    # By adding 100000 from wallet, the fee will be taken from this and the
     # extra placed back into the wallet by default
-    result = l2.rpc.splice(f"wallet -> 10000; 100000 -> {chan_id1}; {chan_id2} -> 100000")
+    result = l2.rpc.splice(f"wallet -> 100000; 100000 -> {chan_id1}; {chan_id2} -> 100000")
 
     l3.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
     l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
@@ -639,15 +639,15 @@ def execute_script(node_factory, bitcoind, script, expected_balances=None, fee_m
 @pytest.mark.openchannel('v2')
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
 def test_script_two_chan_splice_b(node_factory, bitcoind):
-    execute_script(node_factory, bitcoind, "wallet -> 10000; {} -> 100000; {} -> 100000",
+    execute_script(node_factory, bitcoind, "wallet -> 100000; {} -> 100000; {} -> 100000",
                    [500000 - 100000, 500000 - 100000])
 
 
 @pytest.mark.openchannel('v1')
 @pytest.mark.openchannel('v2')
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
 def test_script_two_chan_splice_c(node_factory, bitcoind):
-    execute_script(node_factory, bitcoind, "wallet -> 10000; 100000 -> {}; {} -> 100000",
+    execute_script(node_factory, bitcoind, "wallet -> 100000; 100000 -> {}; {} -> 100000",
                    [500000 + 100000, 500000 - 100000])
 
 
@@ -711,8 +711,8 @@ def test_script_two_chan_splice_j(node_factory, bitcoind):
 @pytest.mark.openchannel('v2')
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
 def test_script_two_chan_splice_k(node_factory, bitcoind):
-    execute_script(node_factory, bitcoind, "{} -> 10000; 1000 -> {}",
-                   [500000 - 10000, 500000 + 1000])
+    execute_script(node_factory, bitcoind, "{} -> 100000; 1000 -> {}",
+                   [500000 - 100000, 500000 + 1000])
 
 
 @pytest.mark.openchannel('v1')
@@ -807,8 +807,8 @@ def test_script_two_chan_splice_v(node_factory, bitcoind):
 @pytest.mark.openchannel('v2')
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
 def test_script_two_chan_splice_x(node_factory, bitcoind):
-    execute_script(node_factory, bitcoind, "* -> wallet; * -> {}; {} -> 100000",
-                   [500000 + 50000, 500000 - 100000], [-0.5, 0])
+    execute_script(node_factory, bitcoind, "* -> wallet; * -> {}; {} -> 200000",
+                   [500000 + 100000 + 1, 500000 - 200000], [-0.5, 0])  # (+1 to track rounded sat)
 
 
 @pytest.mark.openchannel('v1')

tests/test_splicing.py

@@ -17,7 +17,7 @@ def test_splice(node_factory, bitcoind):
     chan_id = l1.get_channel_id(l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -59,7 +59,7 @@ def test_two_chan_splice_in(node_factory, bitcoind):
     chan_id2 = l2.get_channel_id(l3)
 
     # add extra sats to pay fee
-    funds_result = l2.rpc.fundpsbt("205790sat", 0, 0, excess_as_change=True)
+    funds_result = l2.rpc.fundpsbt("211722sat", 0, 0, excess_as_change=True)
 
     # Intiate splices to both channels
     result = l2.rpc.splice_init(chan_id1, 100000, funds_result['psbt'])
@@ -130,8 +130,8 @@ def test_splice_rbf(node_factory, bitcoind):
 
     funds_result = l1.rpc.addpsbtoutput(100000)
 
-    # Pay with fee by subtracting 5000 from channel balance
-    result = l1.rpc.splice_init(chan_id, -105000, funds_result['psbt'])
+    # Pay with fee by subtracting 5801 from channel balance
+    result = l1.rpc.splice_init(chan_id, -105801, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
     assert(result['commitments_secured'] is False)
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -151,7 +151,7 @@ def test_splice_rbf(node_factory, bitcoind):
     funds_result = l1.rpc.addpsbtoutput(100000)
 
     # Pay with fee by subtracting 5790 from channel balance
-    result = l1.rpc.splice_init(chan_id, -105790, funds_result['psbt'])
+    result = l1.rpc.splice_init(chan_id, -111722, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
     assert(result['commitments_secured'] is False)
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -189,7 +189,7 @@ def test_splice_nosign(node_factory, bitcoind):
     chan_id = l1.get_channel_id(l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -214,7 +214,7 @@ def test_splice_gossip(node_factory, bitcoind):
     pre_splice_scid = first_scid(l1, l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -277,7 +277,7 @@ def test_splice_listnodes(node_factory, bitcoind):
     chan_id = l1.get_channel_id(l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -314,8 +314,8 @@ def test_splice_out(node_factory, bitcoind):
 
     funds_result = l1.rpc.addpsbtoutput(100000)
 
-    # Pay with fee by subjtracting 5000 from channel balance
-    result = l1.rpc.splice_init(chan_id, -105000, funds_result['psbt'])
+    # Pay with fee by subjtracting 5801 from channel balance
+    result = l1.rpc.splice_init(chan_id, -105801, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
     assert(result['commitments_secured'] is False)
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -372,7 +372,7 @@ def test_invalid_splice(node_factory, bitcoind):
     assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 0
 
     # Now we do a real splice to confirm everything works after restart
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -408,7 +408,7 @@ def test_commit_crash_splice(node_factory, bitcoind):
 
     chan_id = l1.get_channel_id(l2)
 
-    result = l1.rpc.splice_init(chan_id, -105000, l1.rpc.addpsbtoutput(100000)['psbt'])
+    result = l1.rpc.splice_init(chan_id, -105801, l1.rpc.addpsbtoutput(100000)['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
     assert(result['commitments_secured'] is False)
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -467,7 +467,7 @@ def test_splice_stuck_htlc(node_factory, bitcoind, executor):
     chan_id = l1.get_channel_id(l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
@@ -513,7 +513,7 @@ def test_route_by_old_scid(node_factory, bitcoind):
     route = l1.rpc.getroute(l3.info['id'], 10000000, 1, cltv=16)['route']
 
     # Do a splice
-    funds_result = l2.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l2.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
     chan_id = l2.get_channel_id(l3)
     result = l2.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l2.rpc.splice_update(chan_id, result['psbt'])
@@ -533,7 +533,7 @@ def test_route_by_old_scid(node_factory, bitcoind):
 
     # Let's splice again, so the original scid is two behind the times.
     l3.fundwallet(200000)
-    funds_result = l3.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l3.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
     chan_id = l3.get_channel_id(l2)
     result = l3.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l3.rpc.splice_update(chan_id, result['psbt'])
@@ -566,7 +566,7 @@ def test_splice_unannounced(node_factory, bitcoind):
     chan_id = l1.get_channel_id(l2)
 
     # add extra sats to pay fee
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True)
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])
     assert(result['commitments_secured'] is False)

tests/test_splicing_insane.py

@@ -9,7 +9,7 @@ def make_pending_splice(node_factory):
 
     chan_id = l1.get_channel_id(l2)
 
-    funds_result = l1.rpc.fundpsbt("105790sat", 0, 0, excess_as_change=True)
+    funds_result = l1.rpc.fundpsbt("107527sat", 0, 0, excess_as_change=True)
 
     result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt'])
     result = l1.rpc.splice_update(chan_id, result['psbt'])