splice: Add test for splice-out and fix errors

Added a test for splicing out that exposed some behavior and code glitches that are addressed in this commit.

Also added documentation for how to do a splice out .

ChangeLog-Fixed: Added docs, testing, and some fixes related to splicing out.

Author: ddustin

channeld/channeld.c

@@ -61,6 +61,8 @@
 	((msg) == WIRE_SPLICE || \
 	(msg) == WIRE_SPLICE_ACK)
 
+#define SAT_MIN(a, b) (amount_sat_less((a), (b)) ? (a) : (b))
+
 struct peer {
 	struct per_peer_state *pps;
 	bool channel_ready[NUM_SIDES];
@@ -2907,7 +2909,7 @@ static size_t calc_weight(enum tx_role role, const struct wally_psbt *psbt)
 				weight += psbt_input_get_weight(psbt, i);
 
 	for (size_t i = 0; i < psbt->num_outputs; i++)
-		if (is_initiators_serial(&psbt->inputs[i].unknowns)) {
+		if (is_initiators_serial(&psbt->outputs[i].unknowns)) {
 			if (role == TX_INITIATOR)
 				weight += psbt_output_get_weight(psbt, i);
 		}
@@ -2928,7 +2930,7 @@ static struct amount_sat check_balances(struct peer *peer,
 {
 	struct amount_sat min_initiator_fee, min_accepter_fee,
 			  max_initiator_fee, max_accepter_fee,
-			  funding_amount_res;
+			  funding_amount_res, min_multiplied;
 	struct amount_msat funding_amount,
 			   initiator_fee, accepter_fee;
 	struct amount_msat in[NUM_TX_ROLES], out[NUM_TX_ROLES];
@@ -2977,45 +2979,23 @@ static struct amount_sat check_balances(struct peer *peer,
 	 *   While we're, here, adjust the output counts by splice amount.
 	 */
 
-	if(peer->splicing->opener_relative > 0) {
-		if (!amount_msat_add_sat(&funding_amount, funding_amount,
-					amount_sat((u64)peer->splicing->opener_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to add opener funding");
-		if (!amount_msat_add_sat(&out[TX_INITIATOR], out[TX_INITIATOR],
-					amount_sat((u64)peer->splicing->opener_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to add opener funding to out amnt.");
-	} else {
-		if (!amount_msat_sub_sat(&funding_amount, funding_amount,
-					amount_sat((u64)-peer->splicing->opener_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to sub opener funding");
-		if (!amount_msat_sub_sat(&out[TX_INITIATOR], out[TX_INITIATOR],
-					amount_sat((u64)peer->splicing->opener_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to sub opener funding from out amnt.");
-	}
+	if (!amount_msat_add_sat_s64(&funding_amount, funding_amount,
+				peer->splicing->opener_relative))
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "Unable to add opener funding");
+	if (!amount_msat_add_sat_s64(&out[TX_INITIATOR], out[TX_INITIATOR],
+				peer->splicing->opener_relative))
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "Unable to add opener funding to out amnt.");
 
-	if(peer->splicing->accepter_relative > 0) {
-		if (!amount_msat_add_sat(&funding_amount, funding_amount,
-					amount_sat((u64)peer->splicing->accepter_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to add accepter funding");
-		if (!amount_msat_add_sat(&out[TX_ACCEPTER], out[TX_ACCEPTER],
-					amount_sat((u64)peer->splicing->accepter_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to add accepter funding to out amnt.");
-	} else {
-		if (!amount_msat_sub_sat(&funding_amount, funding_amount,
-					amount_sat((u64)-peer->splicing->accepter_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to subtract accepter funding");
-		if (!amount_msat_sub_sat(&out[TX_ACCEPTER], out[TX_ACCEPTER],
-					amount_sat((u64)-peer->splicing->accepter_relative)))
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "Unable to sub accepter funding from out amnt.");
-	}
+	if (!amount_msat_add_sat_s64(&funding_amount, funding_amount,
+				peer->splicing->accepter_relative))
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "Unable to add accepter funding");
+	if (!amount_msat_add_sat_s64(&out[TX_ACCEPTER], out[TX_ACCEPTER],
+				peer->splicing->accepter_relative))
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "Unable to add accepter funding to out amnt.");
 
 	if (amount_msat_less(in[TX_INITIATOR], out[TX_INITIATOR])) {
 		msg = towire_channeld_splice_funding_error(NULL, in[TX_INITIATOR],
@@ -3064,6 +3044,14 @@ static struct amount_sat check_balances(struct peer *peer,
 	max_initiator_fee = amount_tx_fee(peer->feerate_max,
 					  calc_weight(TX_INITIATOR, psbt));
 
+	/* Sometimes feerate_max is some absurdly high value, in that case we
+	 * give a fee warning based of a multiple of the min value. */
+	amount_sat_mul(&min_multiplied, min_accepter_fee, 5);
+	max_accepter_fee = SAT_MIN(min_multiplied, max_accepter_fee);
+
+	amount_sat_mul(&min_multiplied, min_initiator_fee, 5);
+	max_initiator_fee = SAT_MIN(min_multiplied, max_initiator_fee);
+
 	/* Check initiator fee */
 	if (amount_msat_less_sat(initiator_fee, min_initiator_fee)) {
 		msg = towire_channeld_splice_feerate_error(NULL, initiator_fee,

channeld/inflight.c

@@ -12,8 +12,15 @@ struct inflight *fromwire_inflight(const tal_t *ctx, const u8 **cursor, size_t *
 	inflight->amnt = fromwire_amount_sat(cursor, max);
 	inflight->psbt = fromwire_wally_psbt(inflight, cursor, max);
 	inflight->splice_amnt = fromwire_s64(cursor, max);
-	inflight->last_tx = fromwire_bitcoin_tx(inflight, cursor, max);
-	fromwire_bitcoin_signature(cursor, max, &inflight->last_sig);
+	int has_tx = fromwire_u8(cursor, max);
+	if(has_tx) {
+		inflight->last_tx = fromwire_bitcoin_tx(inflight, cursor, max);
+		fromwire_bitcoin_signature(cursor, max, &inflight->last_sig);
+	}
+	else {
+		inflight->last_tx = NULL;
+		memset(&inflight->last_sig, 0, sizeof(inflight->last_sig));
+	}
 	inflight->i_am_initiator = fromwire_bool(cursor, max);
 
 	return inflight;
@@ -25,8 +32,11 @@ void towire_inflight(u8 **pptr, const struct inflight *inflight)
 	towire_amount_sat(pptr, inflight->amnt);
 	towire_wally_psbt(pptr, inflight->psbt);
 	towire_s64(pptr, inflight->splice_amnt);
-	towire_bitcoin_tx(pptr, inflight->last_tx);
-	towire_bitcoin_signature(pptr, &inflight->last_sig);
+	towire_u8(pptr, inflight->last_tx ? 1 : 0);
+	if(inflight->last_tx) {
+		towire_bitcoin_tx(pptr, inflight->last_tx);
+		towire_bitcoin_signature(pptr, &inflight->last_sig);
+	}
 	towire_bool(pptr, inflight->i_am_initiator);
 }
 

doc/lightning-splice_init.7.md

@@ -29,29 +29,58 @@ our\_bytes\_in\_splice\_tx / 1000.
 provided looks too high. This is to protect against accidentally setting your
 fee higher than intended. Set `force_feerate` to true to skip this saftey check.
 
+Note you may need to add a double dash (\-\-) after splice\_init if using a negative
+*relative\_amount* so it is not interpretted as a command modifier. For example:
+```shell
+lightning-cli splice_init -- $CHANNEL_ID -100000
+```
+
 Here is an example set of splice commands that will splice in 100,000 sats to
 the first channel that comes out of `listpeerchannels`. The example assumes
 you already have at least one confirmed channel.
 ```shell
-RESULT=$(lightning-cli listpeerchannels)
-CHANNEL_ID=$(echo $RESULT| jq -r ".channels[0].channel_id")
-echo $RESULT
+RESULT=$(lightning-cli listpeerchannels);
+CHANNEL_ID=$(echo $RESULT| jq -r ".channels[0].channel_id");
+echo $RESULT;
+
+RESULT=$(lightning-cli fundpsbt -k satoshi=100000sat feerate=urgent startweight=800 excess_as_change=true);
+INITIALPSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
+
+RESULT=$(lightning-cli splice_init $CHANNEL_ID 100000 $INITIALPSBT);
+PSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
+
+RESULT=$(lightning-cli splice_update $CHANNEL_ID $PSBT);
+PSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
 
-RESULT=$(lightning-cli fundpsbt -k satoshi=100000sat feerate=urgent startweight=800 excess_as_change=true)
-INITIALPSBT=$(echo $RESULT | jq -r ".psbt")
-echo $RESULT
+RESULT=$(lightning-cli signpsbt -k psbt="$PSBT");
+PSBT=$(echo $RESULT | jq -r ".signed_psbt");
+echo $RESULT;
+
+lightning-cli splice_signed $CHANNEL_ID $PSBT
+```
+
+Here is an example set of splice commands that will splice out 100,000 sats from
+ first channel that comes out of `listpeerchannels`. The example assumes
+you already have at least one confirmed channel.
+```shell
+RESULT=$(lightning-cli listpeerchannels);
+CHANNEL_ID=$(echo $RESULT| jq -r ".channels[0].channel_id");
+echo $RESULT;
 
-RESULT=$(lightning-cli splice_init $CHANNEL_ID 100000 $INITIALPSBT)
-PSBT=$(echo $RESULT | jq -r ".psbt")
-echo $RESULT
+RESULT=$(lightning-cli recvpsbt 100000);
+INITIALPSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
 
-RESULT=$(lightning-cli splice_update $CHANNEL_ID $PSBT)
-PSBT=$(echo $RESULT | jq -r ".psbt")
-echo $RESULT
+RESULT=$(lightning-cli splice_init -- $CHANNEL_ID -100500 $INITIALPSBT);
+PSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
 
-RESULT=$(lightning-cli signpsbt -k psbt="$PSBT")
-PSBT=$(echo $RESULT | jq -r ".signed_psbt")
-echo $RESULT
+RESULT=$(lightning-cli splice_update $CHANNEL_ID $PSBT);
+PSBT=$(echo $RESULT | jq -r ".psbt");
+echo $RESULT;
 
 lightning-cli splice_signed $CHANNEL_ID $PSBT
 ```

lightningd/channel_control.c

@@ -1417,7 +1417,10 @@ bool peer_start_channeld(struct channel *channel,
 		infcopy->outpoint = inflight->funding->outpoint;
 		infcopy->amnt = inflight->funding->total_funds;
 		infcopy->splice_amnt = inflight->funding->splice_amnt;
-		infcopy->last_tx = tal_dup(infcopy, struct bitcoin_tx, inflight->last_tx);
+		if (inflight->last_tx)
+			infcopy->last_tx = tal_dup(infcopy, struct bitcoin_tx, inflight->last_tx);
+		else
+			infcopy->last_tx = NULL;
 		infcopy->last_sig = inflight->last_sig;
 		infcopy->i_am_initiator = inflight->i_am_initiator;
 		tal_wally_start();

tests/test_splicing.py

@@ -39,3 +39,38 @@ def test_splice(node_factory, bitcoind):
     # Check that the splice doesn't generate a unilateral close transaction
     time.sleep(5)
     assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0
+
+
+@pytest.mark.openchannel('v1')
+@pytest.mark.openchannel('v2')
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
+def test_splice_out(node_factory, bitcoind):
+    l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=True, opts={'experimental-splicing': None})
+
+    chan_id = l1.get_channel_id(l2)
+
+    funds_result = l1.rpc.recvpsbt(100000)
+
+    # Pay with fee by subjtracting 5000 from channel balance
+    result = l1.rpc.splice_init(chan_id, -105000, funds_result['psbt'])
+    result = l1.rpc.splice_update(chan_id, result['psbt'])
+    result = l1.rpc.splice_signed(chan_id, result['psbt'])
+
+    l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
+    l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE')
+
+    mempool = bitcoind.rpc.getrawmempool(True)
+    assert len(list(mempool.keys())) == 1
+    assert result['txid'] in list(mempool.keys())
+
+    bitcoind.generate_block(6, wait_for_mempool=1)
+
+    l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL')
+    l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL')
+
+    inv = l2.rpc.invoice(10**2, '3', 'no_3')
+    l1.rpc.pay(inv['bolt11'])
+
+    # Check that the splice doesn't generate a unilateral close transaction
+    time.sleep(5)
+    assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0