splicescript: Turn on bitcoin addresses in splice script by ddustin · Pull Request #8991 · ElementsProject/lightning

ddustin · 2026-03-25T21:30:54Z

Some work to enable bitcoin address support in splice script, with automatically empowers spliceout to receive an address as a destination. This (IMO super cool) feature allows people to pay onchain bitcoin addresses from their lightning balance.

While we were, we spotted a rare corner case involving a small memleak and fixed it.

We also fixed a few issues with how bitcoin addresses were handled in splice script.

Then we enabled bitcoin addresses in splice script and turned on the tests.

Super excited for this one!

This new test tests sending to a static amount in a bitcoin address inside splice script.

If we’re aborting a script attempt and that abort fails (for example if the connection is lost midway), we were leaving the callback payload `abort_pkg` attached to the plugin. This fix releases the `abort_pkg` when the abort fails.

Some bugs were existing in the bitcoin addr handling, `strcmp`’s result was handled backwards. The “Cannot fund from bitcoin address” check was also inverted, checking `in_sat` when it was meant to be checking `out_sat`. This value meaning sats going “in” to the action or “out” of the action. Also I moved the bitcoin addr handling into its own function, and had it occur later in the process. In paticular we needed it to happen *after* ppm calculations were complete to support dynamic amounts on the bitcoin address.

And also enable the tests for bitcoin addresses Changelog-Added: Added the ability to `spliceout` to a bitcoin address directly.

Andezion · 2026-07-22T00:07:09Z

    assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


-@pytest.mark.xfail(strict=True)


The xfail(strict=True) removal from test_splice_out_address is correct, but the test only covers a fixed absolute amount to the address - the percent-based path enabled by point 4 (in_ppm -> address) is now reachable code but has zero test coverage?

574e859 removes @pytest.mark.xfail(strict=True) in two places, not one: tests/test_splice.py:656 on test_splice_out_address, the test added by 6aa09d2, and tests/test_splice.py:688 on test_easy_splice_out_address, which was already on master.

The first one does use a static amount. Its script is *:? -> 100000+fee; 100000 -> <addr>, so that action parses to in_sat = 100000, in_ppm = 0.

The second one does not. It calls l1.rpc.spliceout("*:?", "100000", destination=addr), and spliceout is a wrapper that writes the script itself, plugins/spender/splice.c:2292:

script = tal_fmt(NULL, "%s -> %s + fee; 100%% -> %s", channel, amount, destination);

100%% emits a literal 100%, so the generated script is *:? -> 100000 + fee; 100% -> <addr>. The left side of a segment is the inbound amount (common/splice_script.c:2540, itr->in_ppm = tokens[i]->left->ppm), so that action parses to in_ppm = 1000000, in_sat = 0. That is exactly the bitcoin_address && in_ppm combination the first deleted guard rejected, and it is what the whole spliceout RPC path uses.

Easy one to miss: the % never appears in the Python, it is generated in the C wrapper.

@nGoline's comment is pointing out that the percent-based path enabled by point 4 is covered in one of the two tests that are turned on in this PR.

Andezion · 2026-07-22T00:09:46Z

 						    " fee");
 			paying_fee_count++;
 		}
-		if (action->bitcoin_address && action->in_ppm)


Nice! No remaining guard exists, but none is needed

handle_bitcoin_addrs() appends the output with action->in_sat unconditionally (splice.c:1692). The wallet path immediately below (splice.c:1761) checks chainparams->dust_limit and folds dust into the fee instead of creating the output. There is no equivalent for addresses.

That was survivable while only static amounts were reachable, since the user picked the number. With in_ppm allowed, a small percentage or a share that rounds to zero produces a dust or 0-sat output. channeld does not dust-check splice outputs, so this does not surface as a JSONRPC2_INVALID_PARAMS, it surfaces as the peer failing the negotiation (BOLT#2 tx_add_output: MUST fail the negotiation if the sats amount is less than the dust_limit) or as a transaction that will not relay, with the channel left holding an inflight splice.

So on the removal of the bitcoin_address && in_ppm guard: agreed it should go, but I would replace it with a dust check in handle_bitcoin_addrs() rather than nothing.

nGoline

c9b4ac4 mixes two real bug fixes with a function extraction and a reordering of when the output is added. The reordering is the part with behaviour risk. Splitting it out would make it bisectable on its own.

nGoline · 2026-08-06T13:07:33Z

 						    " fee");
 			paying_fee_count++;
 		}
-		if (action->bitcoin_address && action->in_ppm)


handle_bitcoin_addrs() appends the output with action->in_sat unconditionally (splice.c:1692). The wallet path immediately below (splice.c:1761) checks chainparams->dust_limit and folds dust into the fee instead of creating the output. There is no equivalent for addresses.

That was survivable while only static amounts were reachable, since the user picked the number. With in_ppm allowed, a small percentage or a share that rounds to zero produces a dust or 0-sat output. channeld does not dust-check splice outputs, so this does not surface as a JSONRPC2_INVALID_PARAMS, it surfaces as the peer failing the negotiation (BOLT#2 tx_add_output: MUST fail the negotiation if the sats amount is less than the dust_limit) or as a transaction that will not relay, with the channel left holding an inflight splice.

So on the removal of the bitcoin_address && in_ppm guard: agreed it should go, but I would replace it with a dust check in handle_bitcoin_addrs() rather than nothing.

nGoline · 2026-08-06T13:07:50Z

    assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


-@pytest.mark.xfail(strict=True)


574e859 removes @pytest.mark.xfail(strict=True) in two places, not one: tests/test_splice.py:656 on test_splice_out_address, the test added by 6aa09d2, and tests/test_splice.py:688 on test_easy_splice_out_address, which was already on master.

The first one does use a static amount. Its script is *:? -> 100000+fee; 100000 -> <addr>, so that action parses to in_sat = 100000, in_ppm = 0.

The second one does not. It calls l1.rpc.spliceout("*:?", "100000", destination=addr), and spliceout is a wrapper that writes the script itself, plugins/spender/splice.c:2292:

script = tal_fmt(NULL, "%s -> %s + fee; 100%% -> %s", channel, amount, destination);

100%% emits a literal 100%, so the generated script is *:? -> 100000 + fee; 100% -> <addr>. The left side of a segment is the inbound amount (common/splice_script.c:2540, itr->in_ppm = tokens[i]->left->ppm), so that action parses to in_ppm = 1000000, in_sat = 0. That is exactly the bitcoin_address && in_ppm combination the first deleted guard rejected, and it is what the whole spliceout RPC path uses.

Easy one to miss: the % never appears in the Python, it is generated in the C wrapper.

nGoline · 2026-08-06T13:33:30Z

 	if (!splice_cmd->fee_calculated) {

 		result = handle_fee_and_ppm(cmd, splice_cmd);
 		if (result)
 			return result;

 		splice_cmd->fee_calculated = true;
 	}

+	result = handle_bitcoin_addrs(cmd, splice_cmd);


calc_weight() (splice.c:865) counts psbt->num_outputs, simulates one output per channel action, and simulates a single P2TR output if any wallet action wants funds. Nothing simulates an address output.

Before c9b4ac4 the address output was appended in execute_splice(), which runs before continue_splice(), so it was in the psbt by the time handle_fee_and_ppm() called calc_weight(). Now handle_bitcoin_addrs() runs after it, so at fee time the psbt is empty and the output is invisible to the estimate.

The leftover the plugin arranges is amount_tx_fee(feerate, calc_weight(...)). channeld compares that leftover against amount_tx_fee(peer->splicing->feerate_per_kw, calc_weight(TX_INITIATOR, psbt, false)) (channeld/channeld.c:3595 and 3617), computed over the final psbt, which does contain the address output. By my reading that is short by bitcoin_tx_output_weight() of the address scriptpubkey, 172 WU for P2TR.

In test_splice_out_address this cancels out by accident, and the reason is the difference between the two scripts.

calculate_amounts() (common/splice_script.c:2429) appends an implicit <remainder>% -> wallet segment, but only when !left_wilds && left_used_ppm < 1000000. The script-form test is 100000 -> addr, a literal amount, so left_used_ppm stays 0 and a wallet action is appended. That makes add_wallet_output true in calc_weight(), which simulates a P2TR wallet output that then never gets created, because the remainder is zero and the dust branch at splice.c:1761 drops it. 172 WU of phantom output offsets 172 WU of missing output.

The spliceout test is 100% -> addr, so left_used_ppm is already 1000000, no wallet segment is appended, and I cannot see what offsets it there. CI is green, so I am probably missing something. Can you confirm where that case is covered?

Either way, calc_weight() should have an explicit case for action->bitcoin_address rather than the estimate being correct by coincidence. Right now changing the address type (P2WPKH is 22 bytes of script, not 34) or the wallet-segment heuristic silently changes the fee.

nGoline · 2026-08-06T13:35:12Z

+					       " address is %s while"
+					       " address from script is"
+					       " %s",
+					       bitcoin_address ?: "NULL",


This looks like dead code. The !bitcoin_address case returned on the previous if (!bitcoin_address).

nGoline · 2026-08-06T13:36:12Z

-			add_to_debug_log(splice_cmd,
-					 "execute_splice-load_btcaddress");


debug_log is user-visible output of dev-splice, and every other step in this flow logs one. Worth re-adding as handle_bitcoin_addrs-load_btcaddress.

nGoline · 2026-08-06T13:38:59Z

 	if (!added) {
 		plugin_log(cmd->plugin, LOG_DBG,
 			   "No channels were stfu'ed, skipping to unreserve"
 			   " (psbt:%p)", splice_cmd->psbt);
 		return abort_get_result(cmd, NULL, NULL, NULL, abort_pkg);
 	}


Going through the code and this caught my eye. Might worth fixing in this PR.
When no channels were stfu'ed, the abort_channels req is built and then never sent, and abort_get_result() is called directly. That unsent req still holds abort_pkg as its errcb arg, which 33b6782 now frees. It is never invoked so nothing breaks today, but it is a dangling arg one refactor away from a use-after-free.
Building the req after the added count, or clearing req->arg, would remove the footgun.

abort_get_result, defined above, calls into a json_request which uses the new handler which frees abort_pkg in the error case.

nGoline · 2026-08-06T13:40:48Z

+    wait_for(lambda: len(l1.rpc.listfunds()['channels']) == 1)
+
+    end_wallet_balance = Millisatoshi(bkpr_account_balance(l1, 'wallet'))
+    assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


This only asserts the wallet balance moved, which holds because the address is l1's own. It would still pass if the funds landed at a different l1 address. Asserting the requested address appears in listfunds()['outputs'] would actually pin down that the scriptpubkey is the one that was asked for.

Instead of routing the funds to l1's onchain wallet, route them to l2's and verify the wallet balance difference there.

That would confirm the splice worked.

Could also add a check of the channel's balance (or at least check that the balance changes by at least that much)

niftynei

ACK 574e859

niftynei · 2026-08-09T20:27:52Z

+    wait_for(lambda: len(l1.rpc.listfunds()['channels']) == 1)
+
+    end_wallet_balance = Millisatoshi(bkpr_account_balance(l1, 'wallet'))
+    assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


Instead of routing the funds to l1's onchain wallet, route them to l2's and verify the wallet balance difference there.

That would confirm the splice worked.

Could also add a check of the channel's balance (or at least check that the balance changes by at least that much)

niftynei · 2026-08-09T20:35:30Z

+			return do_fail(cmd, splice_cmd,
+				       JSONRPC2_INVALID_PARAMS,
+				       "Bitcoin address"
+				       " unrecognized");


nit: print the unrecognized bitcoin address

niftynei · 2026-08-09T20:36:03Z

+			return do_fail(cmd, splice_cmd,
+				       JSONRPC2_INVALID_PARAMS,
+				       "Bitcoin scriptpubkey failed"
+				       " reencoding for address");


nit: add failing scriptpubkey to error message

niftynei · 2026-08-09T20:37:00Z

+					       " address from script is"
+					       " %s",
+					       bitcoin_address ?: "NULL",
+					       action->bitcoin_address));


suggestion: is there a more global place in CLN we can enshrine the "decode/recode address" logic and use it more places? Seems like a good check on wallets in general

niftynei · 2026-08-09T20:38:07Z

+					    action->in_sat);
+
+		serial_id = psbt_new_output_serial(splice_cmd->psbt,
+				       		   TX_INITIATOR);


are you always the initiator? what happens if splice scripts gets added to funder? (future problems?)

niftynei · 2026-08-09T20:43:29Z

    assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


-@pytest.mark.xfail(strict=True)


@nGoline's comment is pointing out that the percent-based path enabled by point 4 is covered in one of the two tests that are turned on in this PR.

ddustin added 4 commits March 25, 2026 17:14

splicescript: Add test for splice out to btc addr

6aa09d2

This new test tests sending to a static amount in a bitcoin address inside splice script.

splicescript: Fix rare memleak of aborting an abort

33b6782

If we’re aborting a script attempt and that abort fails (for example if the connection is lost midway), we were leaving the callback payload `abort_pkg` attached to the plugin. This fix releases the `abort_pkg` when the abort fails.

splicescript: Turn on bitcoin addresses in splice script

574e859

And also enable the tests for bitcoin addresses Changelog-Added: Added the ability to `spliceout` to a bitcoin address directly.

ddustin added this to the v26.06 milestone May 12, 2026

madelinevibes requested a review from niftynei June 4, 2026 12:29

madelinevibes added the Status::Ready for Review The work has been completed and is now awaiting evaluation or approval. label Jun 4, 2026

madelinevibes modified the milestones: v26.06, v26.09 Jun 4, 2026

madelinevibes assigned niftynei Jun 15, 2026

Andezion approved these changes Jul 22, 2026

View reviewed changes

nGoline requested changes Aug 6, 2026

View reviewed changes

niftynei reviewed Aug 9, 2026

View reviewed changes

		assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


		@pytest.mark.xfail(strict=True)

		add_to_debug_log(splice_cmd,
		"execute_splice-load_btcaddress");

Uh oh!

Conversation

ddustin commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nGoline left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niftynei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants