Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
19 changes: 19 additions & 0 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -4224,6 +4224,16 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
}
}
else if (type == WIRE_TX_INIT_RBF) {
/* BOLT #2:
* The sender:
* - MUST NOT send `tx_init_rbf` if `option_zeroconf`
* has been negotiated.
*/
if (channel_type_has(peer->channel->type, OPT_ZEROCONF))
peer_failed_warn(peer->pps, &peer->channel_id,
"Peer sent tx_init_rbf but channel"
" uses option_zeroconf");

if (!fromwire_tx_init_rbf(tmpctx, inmsg,
&channel_id,
&locktime,
Expand Down Expand Up @@ -5001,6 +5011,15 @@ static void handle_splice_init(struct peer *peer, const u8 *inmsg)
wire_sync_write(MASTER_FD, take(msg));
return;
}
if (last_inflight(peer)
&& channel_type_has(peer->channel->type, OPT_ZEROCONF)) {
msg = towire_channeld_splice_state_error(NULL,
"Cannot RBF splice:"
" channel uses"
" option_zeroconf");
wire_sync_write(MASTER_FD, take(msg));
return;
}

status_debug("Getting handle_splice_init psbt version %d (RBF?: %s)",
peer->splicing->current_psbt->version,
Expand Down
2 changes: 1 addition & 1 deletion lightningd/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ struct channel *new_unsaved_channel(struct peer *peer,
channel->state = DUALOPEND_OPEN_INIT;
channel->owner = NULL;
channel->reestablished = false;
channel->minimum_depth = ld->config.funding_confirms;
memset(&channel->billboard, 0, sizeof(channel->billboard));
channel->billboard.transient = tal_fmt(channel, "%s",
"Empty channel init'd");
Expand Down Expand Up @@ -1309,4 +1310,3 @@ const u8 *channel_update_for_error(const tal_t *ctx,
/* FIXME: Call directly from callers */
return channel_gossip_update_for_error(ctx, channel);
}

17 changes: 17 additions & 0 deletions lightningd/channel_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,16 @@ static void handle_splice_lookup_tx(struct lightningd *ld,
}

tx = wallet_transaction_get(tmpctx, ld->wallet, &txid);
if (!tx
&& channel->funding_psbt
&& bitcoin_txid_eq(&channel->funding.txid, &txid)) {
/* Zeroconf funding can be used for splicing before topology
* has indexed the funding tx, so reconstruct it from the
* persisted funding PSBT if needed. */
tx = bitcoin_tx_with_psbt(tmpctx,
clone_psbt(tmpctx,
channel->funding_psbt));
}

if (!tx) {
channel_internal_error(channel,
Expand Down Expand Up @@ -699,6 +709,13 @@ static enum watch_result splice_depth_cb(struct lightningd *ld,
return DELETE_WATCH;
}

/* Reorged out? OK, we're not committed yet.
* But for zero-conf channels (minimum_depth == 0), depth 0 means
* we should send splice_locked immediately per BOLT #2. */
if (depth == 0 && inflight->channel->minimum_depth != 0) {
return KEEP_WATCHING;
}

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 is not the right place to be trying to do zero-conf. Instead we should initiate splice_locked in channeld right after sending tx_signatures which occurs in resume_splice_negotiation.

This needs to be done with extra care because this flow can occur from initiate, accepter, or during the reestablish flow. We need to make sure it's behaving correctly in each of these flows.

Probably the best approach is to take the code in handle_funding_depth that confirms the splice and move it out into it's own function so that we can additionally call it from resume_splice_negotiation.

It would be very important to duplicate the tests in test_splicing_disconnect.py with this new zero conf splice setting enabled.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 7bd121c.

I moved zero-conf splice_locked initiation into resume_splice_negotiation() so we send it immediately after each local tx_signatures write, which covers the initiator, accepter, and reestablish paths in one place. For the reconnect cases I also persist i_sent_sigs, cache early splice_locked until the local signature path is complete, and treat channel_ready during zero-conf reestablish as an implied peer splice_locked once we have already sent ours.

I also duplicated the disconnect regressions for zero-conf in tests/test_splicing_disconnect.py and re-ran the zero-conf happy-path splice test. Verified with targeted runs of test_splice_zeroconf, test_splice_disconnect_sig_zeroconf, and test_splice_disconnect_commit_zeroconf.

if (inflight->channel->owner) {
log_debug(inflight->channel->log, "splice_depth_cb: sending funding depth scid: %s",
fmt_short_channel_id(tmpctx, *inflight->scid));
Expand Down
79 changes: 70 additions & 9 deletions lightningd/dual_open_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS)
payload->psbt,
payload->our_shutdown_scriptpubkey,
our_shutdown_script_wallet_index,
channel->minimum_depth,
payload->rates);

subd_send_msg(dualopend, take(msg));
Expand All @@ -732,6 +733,7 @@ openchannel2_hook_deserialize(struct openchannel2_payload *payload,
}

const jsmntok_t *t_result = json_get_member(buffer, toks, "result");
const jsmntok_t *t_mindepth = json_get_member(buffer, toks, "mindepth");
if (!t_result)
fatal("Plugin returned an invalid response to the"
" openchannel2 hook: %.*s",
Expand Down Expand Up @@ -780,6 +782,15 @@ openchannel2_hook_deserialize(struct openchannel2_payload *payload,
else
payload->our_shutdown_scriptpubkey = shutdown_script;

if (t_mindepth != NULL) {
json_to_u32(buffer, t_mindepth,
&payload->channel->minimum_depth);
log_debug(dualopend->ld->log,
"Setting mindepth=%u for this channel as requested by "
"the openchannel2 hook",
payload->channel->minimum_depth);
}


struct amount_msat fee_base, fee_max_base;
/* deserialized may be called multiple times */
Expand Down Expand Up @@ -1769,6 +1780,16 @@ static void send_funding_tx(struct channel *channel,
sendfunding_done, cs);
}

static void maybe_signal_zeroconf_lockin(struct channel *channel)
{
if (channel->minimum_depth != 0)
return;

log_debug(channel->log, "Zero-conf funding: signaling immediate lock-in");
subd_send_msg(channel->owner,
take(towire_dualopend_depth_reached(NULL, 0)));
}

static void handle_peer_tx_sigs_sent(struct subd *dualopend,
const int *fds,
const u8 *msg)
Expand Down Expand Up @@ -1847,6 +1868,7 @@ static void handle_peer_tx_sigs_sent(struct subd *dualopend,
DUALOPEND_AWAITING_LOCKIN,
REASON_UNKNOWN,
"Sigs exchanged, waiting for lock-in");
maybe_signal_zeroconf_lockin(channel);

/* Mimic the old behavior, notify a channel has been opened,
* for the accepter side */
Expand Down Expand Up @@ -1933,6 +1955,7 @@ static void handle_channel_locked(struct subd *dualopend,
const u8 *msg)
{
struct channel *channel = dualopend->channel;
struct channel_inflight *inflight;
struct peer_fd *peer_fd;

if (!fromwire_dualopend_channel_locked(msg)) {
Expand All @@ -1943,7 +1966,6 @@ static void handle_channel_locked(struct subd *dualopend,
}
peer_fd = new_peer_fd_arr(tmpctx, fds);

assert(channel->scid);
assert(channel->remote_channel_ready);

log_debug(channel->log, "Lockin complete state %s",
Expand All @@ -1958,15 +1980,26 @@ static void handle_channel_locked(struct subd *dualopend,
CHANNELD_NORMAL,
REASON_UNKNOWN,
"Lockin complete");
channel_record_open(channel,
short_channel_id_blocknum(*channel->scid),
true);
lockin_has_completed(channel, true);

inflight = channel_current_inflight(channel);
if (inflight && inflight->funding_psbt) {
tal_free(channel->funding_psbt);
channel->funding_psbt = clone_psbt(channel,
inflight->funding_psbt);
wallet_channel_save(dualopend->ld->wallet, channel);
}

/* Empty out the inflights */
wallet_channel_clear_inflights(dualopend->ld->wallet, channel);

/* That freed watchers in inflights: now watch funding tx */
channel_watch_depth(dualopend->ld, short_channel_id_blocknum(*channel->scid), channel);
/* Zeroconf channels still need to discover their on-chain scid later. */
if (channel->scid)
channel_watch_depth(dualopend->ld,
short_channel_id_blocknum(*channel->scid),
channel);
else
channel_watch_funding(dualopend->ld, channel);
channel_watch_funding_out(dualopend->ld, channel);

/* FIXME: LND sigs/update_fee msgs? */
Expand Down Expand Up @@ -2193,6 +2226,7 @@ static void handle_peer_tx_sigs_msg(struct subd *dualopend,
DUALOPEND_AWAITING_LOCKIN,
REASON_UNKNOWN,
"Sigs exchanged, waiting for lock-in");
maybe_signal_zeroconf_lockin(channel);

/* Mimic the old behavior, notify a channel has been opened,
* for the accepter side */
Expand Down Expand Up @@ -3030,6 +3064,7 @@ static struct command_result *openchannel_init(struct command *cmd,
const struct wally_psbt *psbt,
u32 feerate_per_kw_funding,
u32 feerate_per_kw,
u32 minimum_depth,
const u8 *our_upfront_shutdown_script,
bool announce_channel,
const struct lease_rates *rates,
Expand Down Expand Up @@ -3059,6 +3094,7 @@ static struct command_result *openchannel_init(struct command *cmd,
channel->opener = LOCAL;
channel->open_attempt = oa = new_channel_open_attempt(channel);
channel->channel_flags = OUR_CHANNEL_FLAGS;
channel->minimum_depth = minimum_depth;
oa->funding = amount;
oa->cmd = cmd;

Expand Down Expand Up @@ -3124,6 +3160,7 @@ struct openchannel_init_info {
struct node_id *id;
struct amount_sat *amount, *request_amt;
struct wally_psbt *psbt;
u32 *mindepth;
u32 *feerate_per_kw_funding, *feerate_per_kw;
const u8 *our_upfront_shutdown_script;
bool *announce_channel;
Expand Down Expand Up @@ -3157,6 +3194,7 @@ static void openchannel_init_after_sync(struct chain_topology *topo,
*info->request_amt,
info->psbt,
*info->feerate_per_kw_funding, *info->feerate_per_kw,
*info->mindepth,
info->our_upfront_shutdown_script,
*info->announce_channel,
info->rates, info->ctype);
Expand All @@ -3179,6 +3217,7 @@ static struct command_result *json_openchannel_init(struct command *cmd,
p_req("initialpsbt", param_psbt, &info->psbt),
p_opt("commitment_feerate", param_feerate, &info->feerate_per_kw),
p_opt("funding_feerate", param_feerate, &info->feerate_per_kw_funding),
p_opt("mindepth", param_u32, &info->mindepth),
p_opt_def("announce", param_bool, &info->announce_channel, true),
p_opt("close_to", param_bitcoin_address, &info->our_upfront_shutdown_script),
p_opt_def("request_amt", param_sat, &info->request_amt, AMOUNT_SAT(0)),
Expand Down Expand Up @@ -3237,6 +3276,30 @@ static struct command_result *json_openchannel_init(struct command *cmd,
info->ctype = desired_channel_type(info, cmd->ld->our_features,
peer->their_features);

if (channel_type_has(info->ctype, OPT_ZEROCONF)) {
if (info->mindepth && *info->mindepth != 0) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Cannot set non-zero mindepth for zero-conf channel_type");
}
if (!info->mindepth) {
info->mindepth = tal(info, u32);
*info->mindepth = 0;
}
} else if (info->mindepth && *info->mindepth == 0) {
if (!feature_negotiated(cmd->ld->our_features,
peer->their_features,
OPT_ZEROCONF)) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Cannot set mindepth=0: peer does not support zero-conf channels");
}
info->ctype = channel_type_dup(info, info->ctype);
channel_type_set_zeroconf(info->ctype);
}

if (!info->mindepth)
info->mindepth = tal_dup(info, u32,
&cmd->ld->config.funding_confirms);

if (!cmd->ld->dev_any_channel_type &&
!channel_type_accept(tmpctx,
info->ctype->features,
Expand Down Expand Up @@ -3293,6 +3356,7 @@ static struct command_result *json_openchannel_init(struct command *cmd,
*info->request_amt,
info->psbt,
*info->feerate_per_kw_funding, *info->feerate_per_kw,
*info->mindepth,
info->our_upfront_shutdown_script,
*info->announce_channel,
info->rates, info->ctype);
Expand Down Expand Up @@ -4171,9 +4235,6 @@ bool peer_start_dualopend(struct peer *peer,
* considers reasonable to avoid double-spending of the
* funding transaction.
*/
/* FIXME: We should override this to 0 in the openchannel2 hook of we want zeroconf*/
channel->minimum_depth = peer->ld->config.funding_confirms;

msg = towire_dualopend_init(NULL, chainparams,
peer->ld->our_features,
peer->their_features,
Expand Down
3 changes: 3 additions & 0 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -2287,6 +2287,9 @@ void update_channel_from_inflight(struct lightningd *ld,
channel->lease_chan_max_msat = inflight->lease_chan_max_msat;
channel->lease_chan_max_ppt = inflight->lease_chan_max_ppt;

tal_free(channel->funding_psbt);
channel->funding_psbt = clone_psbt(channel, inflight->funding_psbt);

tal_free(channel->blockheight_states);
channel->blockheight_states = new_height_states(channel,
channel->opener,
Expand Down
1 change: 1 addition & 0 deletions openingd/dualopend.c
Original file line number Diff line number Diff line change
Expand Up @@ -2548,6 +2548,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
&tx_state->psbt,
&state->upfront_shutdown_script[LOCAL],
&state->local_upfront_shutdown_wallet_index,
&state->minimum_depth,
&tx_state->rates))
master_badmsg(WIRE_DUALOPEND_GOT_OFFER_REPLY, msg);

Expand Down
1 change: 1 addition & 0 deletions openingd/dualopend_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ msgdata,dualopend_got_offer_reply,psbt,wally_psbt,
msgdata,dualopend_got_offer_reply,shutdown_len,u16,
msgdata,dualopend_got_offer_reply,our_shutdown_scriptpubkey,?u8,shutdown_len
msgdata,dualopend_got_offer_reply,our_shutdown_wallet_index,?u32,
msgdata,dualopend_got_offer_reply,mindepth,u32,
# must go last because of embedded tu32
msgdata,dualopend_got_offer_reply,lease_rates,?lease_rates,

Expand Down
2 changes: 2 additions & 0 deletions plugins/spender/openchannel.c
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,8 @@ openchannel_init_dest(struct multifundchannel_destination *dest)
mfc->feerate_str);
}
json_add_bool(req->js, "announce", dest->announce);
if (dest->mindepth)
json_add_u32(req->js, "mindepth", *dest->mindepth);
if (dest->close_to_str)
json_add_string(req->js, "close_to", dest->close_to_str);

Expand Down
6 changes: 5 additions & 1 deletion tests/plugins/zeroconf-selective.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

@plugin.hook('openchannel')
def on_openchannel(openchannel, plugin, **kwargs):
plugin.log(repr(openchannel))
mindepth = int(plugin.options['zeroconf_mindepth']['value'])

if openchannel['id'] == plugin.options['zeroconf_allow']['value'] or plugin.options['zeroconf_allow']['value'] == 'any':
Expand All @@ -19,6 +18,11 @@ def on_openchannel(openchannel, plugin, **kwargs):
return {'result': 'continue'}


@plugin.hook('openchannel2')
def on_openchannel2(openchannel2, plugin, **kwargs):
return on_openchannel(openchannel2, plugin, **kwargs)


plugin.add_option(
'zeroconf_allow',
'03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f',
Expand Down
11 changes: 8 additions & 3 deletions tests/test_gossip.py
Original file line number Diff line number Diff line change
Expand Up @@ -2401,9 +2401,14 @@ def test_gossip_force_broadcast_channel_msgs(node_factory, bitcoind):
del tally['query_channel_range']
del tally['ping']
del tally['gossip_filter']
assert tally == {'channel_announce': 1,
'channel_update': 1,
'node_announce': 1}
# We can see l2 replay the shared channel_announcement while the final
# announcement is propagating. Allow a single duplicate.
assert tally in ({'channel_announce': 1,
'channel_update': 1,
'node_announce': 1},
{'channel_announce': 2,
'channel_update': 1,
'node_announce': 1})

# Make sure l1 sees l2's channel update
wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2)
Expand Down
Loading
Loading