Skip to content

splicescript: Turn on bitcoin addresses in splice script #8991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ddustin wants to merge 4 commits into
base: master
Choose a base branch
from
+122 −70
Open

splicescript: Turn on bitcoin addresses in splice script #8991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6aa09d2
splicescript: Add test for splice out to btc addr
ddustin Mar 25, 2026
33b6782
splicescript: Fix rare memleak of aborting an abort
ddustin Mar 25, 2026
c9b4ac4
splicescript: Fix bugs in bitcoin addr handling
ddustin Mar 25, 2026
574e859
splicescript: Turn on bitcoin addresses in splice script
ddustin Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 90 additions & 69 deletions plugins/spender/splice.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ static struct command_result *unreserve_get_result(struct command *cmd,
return make_error(cmd, abort_pkg, "unreserve_get_result");
}

static struct command_result *free_abort_pkg_and_forward(struct command *cmd,
const char *methodname,
const char *buf,
const jsmntok_t *result,
struct abort_pkg *abort_pkg)
{
tal_free(abort_pkg);
return forward_error(cmd, methodname, buf, result, NULL);
}

static struct command_result *abort_get_result(struct command *cmd,
const char *methodname,
const char *buf,
Expand All @@ -163,7 +173,8 @@ static struct command_result *abort_get_result(struct command *cmd,
return make_error(cmd, abort_pkg, "abort_get_result");

req = jsonrpc_request_start(cmd, "unreserveinputs",
unreserve_get_result, forward_error,
unreserve_get_result,
free_abort_pkg_and_forward,
abort_pkg);

json_add_psbt(req->js, "psbt", splice_cmd->psbt);
Expand Down Expand Up @@ -193,7 +204,9 @@ static struct command_result *do_fail(struct command *cmd,
abort_pkg->code = code;

req = jsonrpc_request_start(cmd, "abort_channels",
abort_get_result, forward_error, abort_pkg);
abort_get_result,
free_abort_pkg_and_forward,
abort_pkg);

added = 0;
json_array_start(req->js, "channel_ids");
Expand Down Expand Up @@ -1620,6 +1633,77 @@ static struct command_result *handle_fee_and_ppm(struct command *cmd,
return NULL;
}

/* Fund out to bitcoin addresses */
static struct command_result *handle_bitcoin_addrs(struct command *cmd,
struct splice_cmd *splice_cmd)
{
struct splice_script_result *action;
struct splice_cmd_action_state *state;
struct wally_psbt_output *output;
char *bitcoin_address;
u64 serial_id;
u8 *scriptpubkey;

for (size_t i = 0; i < tal_count(splice_cmd->actions); i++) {
action = splice_cmd->actions[i];
state = splice_cmd->states[i];
if (!action->bitcoin_address)
continue;
if (state->state != SPLICE_CMD_NONE)
continue;

if (!amount_sat_is_zero(action->out_sat))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Cannot fund from bitcoin"
" address");
if (!decode_scriptpubkey_from_addr(cmd, chainparams,
action->bitcoin_address,
&scriptpubkey))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Bitcoin address"
" unrecognized");

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: print the unrecognized bitcoin address


/* Reencode scriptpubkey to addr for verification */
bitcoin_address = encode_scriptpubkey_to_addr(tmpctx,
chainparams,
scriptpubkey,
tal_bytelen(scriptpubkey));
if (!bitcoin_address)
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Bitcoin scriptpubkey failed"
" reencoding for address");

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add failing scriptpubkey to error message


if (0 != strcmp(bitcoin_address, action->bitcoin_address))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
tal_fmt(tmpctx,
"Bitcoin scriptpubkey"
" failed validation for"
" address. Reencoded"
" address is %s while"
" address from script is"
" %s",
bitcoin_address ?: "NULL",

@nGoline nGoline Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

action->bitcoin_address));

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


output = psbt_append_output(splice_cmd->psbt,
scriptpubkey,
action->in_sat);

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

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

psbt_output_set_serial_id(splice_cmd->psbt, output,
serial_id);

state->state = SPLICE_CMD_DONE;
}

return NULL;
}

static struct command_result *continue_splice(struct command *cmd,
struct splice_cmd *splice_cmd)
{
Expand Down Expand Up @@ -1661,6 +1745,10 @@ static struct command_result *continue_splice(struct command *cmd,
splice_cmd->fee_calculated = true;
}

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

/* Only after fee calcualtion can we add wallet actions taking funds */
for (size_t i = 0; i < tal_count(splice_cmd->actions); i++) {
action = splice_cmd->actions[i];
Expand Down Expand Up @@ -1762,18 +1850,13 @@ static struct command_result *execute_splice(struct command *cmd,
struct splice_cmd *splice_cmd)
{
struct splice_script_result *action;
struct splice_cmd_action_state *state;
struct wally_psbt_output *output;
u64 serial_id;
int pays_fee;
u8 *scriptpubkey;

/* Basic validation */
pays_fee = 0;
for (size_t i = 0; i < tal_count(splice_cmd->actions); i++) {
int dest_count = 0;
action = splice_cmd->actions[i];
state = splice_cmd->states[i];

if (action->out_ppm && !action->onchain_wallet)
return do_fail(cmd, splice_cmd, JSONRPC2_INVALID_PARAMS,
Expand Down Expand Up @@ -1824,8 +1907,6 @@ static struct command_result *execute_splice(struct command *cmd,

for (size_t i = 0; i < tal_count(splice_cmd->actions); i++) {
action = splice_cmd->actions[i];
state = splice_cmd->states[i];
char *bitcoin_address;

/* `out_ppm` is the percent to take out of the action.
* If it is set to '*' we get a value of UINT32_MAX.
Expand All @@ -1842,61 +1923,11 @@ static struct command_result *execute_splice(struct command *cmd,
" feerate");
splice_cmd->feerate_per_kw = action->feerate_per_kw;
}

/* Fund out to bitcoin address */
if (action->bitcoin_address) {
if (!amount_sat_is_zero(action->in_sat))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Cannot fund from bitcoin"
" address");
if (!decode_scriptpubkey_from_addr(cmd, chainparams,
action->bitcoin_address,
&scriptpubkey))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Bitcoin address"
" unrecognized");

/* Reencode scriptpubkey to addr for verification */
bitcoin_address = encode_scriptpubkey_to_addr(tmpctx,
chainparams,
scriptpubkey,
tal_bytelen(scriptpubkey));
if (!bitcoin_address)
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Bitcoin scriptpubkey failed"
" reencoding for address");

if (!strcmp(bitcoin_address, action->bitcoin_address))
return do_fail(cmd, splice_cmd,
JSONRPC2_INVALID_PARAMS,
"Bitcoin scriptpubkey failed"
" validation for address");

output = psbt_append_output(splice_cmd->psbt,
scriptpubkey,
action->in_sat);

/* DTODO: support dynamic address payouts (percent) */

serial_id = psbt_new_output_serial(splice_cmd->psbt,
TX_INITIATOR);
psbt_output_set_serial_id(splice_cmd->psbt, output,
serial_id);

state->state = SPLICE_CMD_DONE;

add_to_debug_log(splice_cmd,
"execute_splice-load_btcaddress");
Comment on lines -1891 to -1892

@nGoline nGoline Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}

/* Set needed funds to the wallet contributions. */
for (size_t i = 0; i < tal_count(splice_cmd->actions); i++) {
action = splice_cmd->actions[i];
state = splice_cmd->states[i];
if (action->onchain_wallet
&& !amount_sat_is_zero(action->out_sat)) {
splice_cmd->needed_funds = action->out_sat;
Expand Down Expand Up @@ -2042,16 +2073,6 @@ validate_splice_cmd(struct splice_cmd *splice_cmd)
" fee");
paying_fee_count++;
}
if (action->bitcoin_address && action->in_ppm)

@Andezion Andezion Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! No remaining guard exists, but none is needed

@nGoline nGoline Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return command_fail(splice_cmd->cmd,
JSONRPC2_INVALID_PARAMS,
"Dynamic bitcoin address amounts"
" not supported for now");
if (action->bitcoin_address)
return command_fail(splice_cmd->cmd,
JSONRPC2_INVALID_PARAMS,
"Paying out to bitcoin addresses"
" not supported for now.");
}

return NULL;
Expand Down
33 changes: 32 additions & 1 deletion tests/test_splice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,38 @@ def test_easy_splice_out(node_factory, bitcoind, chainparams):
assert initial_wallet_balance + Millisatoshi(spliceamt * 1000) == end_wallet_balance


@pytest.mark.xfail(strict=True)

@Andezion Andezion Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@nGoline nGoline Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@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_address(node_factory, bitcoind, chainparams):
fundamt = 1000000

l1, l2 = node_factory.line_graph(2, fundamount=fundamt, wait_for_announce=True,
opts={'experimental-splicing': None})

initial_wallet_balance = Millisatoshi(bkpr_account_balance(l1, 'wallet'))

addr = l1.rpc.newaddr()['p2tr']

# Splice out 100k from first channel, putting result less fees into onchain wallet via addres
spliceamt = 100000
l1.rpc.splice(f"*:? -> {spliceamt}+fee; {spliceamt} -> {addr}")

bitcoind.generate_block(6, wait_for_mempool=1)
l2.daemon.wait_for_log(r'lightningd, splice_locked clearing inflights')

p1 = only_one(l1.rpc.listpeerchannels(peer_id=l2.info['id'])['channels'])
p2 = only_one(l2.rpc.listpeerchannels(l1.info['id'])['channels'])
assert 'inflight' not in p1
assert 'inflight' not in p2

wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 2)
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

@nGoline nGoline Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@niftynei niftynei Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)



@pytest.mark.openchannel('v1')
@pytest.mark.openchannel('v2')
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
Expand Down
Loading