Skip to content

Commit

Permalink
splice: signer must be informed of splice params
Browse files Browse the repository at this point in the history 
The signer needs to know when the splice operation starts and the
splice parameters for each splice transaction candidate.

The channel establishment v2 (dual funding) code path already
notifies the signer via the hsmd API hsmd_ready_channel calls
However, the splicing code path does not.

Link: ElementsProject#6723
Suggested-by: @devrandom
Co-Developed-by: Ken Sedgwick <[email protected]>
Signed-off-by: Vincenzo Palazzo <[email protected]>
  • Loading branch information
@ksedgwic @vincenzopalazzo
ksedgwic authored and vincenzopalazzo committed Oct 3, 2023
1 parent bc9333a commit bf3a9bb
Showing 1 changed file with 56 additions and 5 deletions.
61 changes: 56 additions & 5 deletions channeld/channeld.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3441,6 +3441,56 @@ static struct inflight *inflights_new(struct peer *peer)
return inf;
}

static void update_hsmd_with_splice(struct peer *peer, struct inflight *inflight,
const bool accepter_side)
{
u8 *msg;
struct amount_msat push_value;
struct amount_msat remote_old_vale;

/* We calculcate the `push_value` to send to the
* hsmd, that is the remote amount in the channel
* after the splice. */
if (accepter_side)
push_value = amount_msat(peer->splicing->opener_relative);

This comment has been minimized.

Sign in to view

Copy link
@ksedgwic

ksedgwic Oct 3, 2023

Author

I think we want the same value as out[TX_ACCEPTER] as computed in check_balances:
https://github.com/vincenzopalazzo/lightning/blob/macros/remote-signer-with-splicing/channeld/channeld.c#L2998

Also, the xxx_relative values are signed, so they can't be directly assigned to an unsigned. The check_balances routine starts with the current "owed" value (never negative) and adds (the possibly negative) "relative" value to get the post-splice "output", which is the push_val

This comment has been minimized.

Sign in to view

Copy link
@vincenzopalazzo

vincenzopalazzo Oct 3, 2023

Owner

lso, the xxx_relative values are signed, so they can't be directly assigned to an unsigned. The check_balances routine starts with the current "owed" value (never negative) and adds (the possibly negative) "relative" value to get the post-splice "output", which is the push_val

This is an amazing review, thanks! I was just thinking with the amount_msat was smart enough to do this.

I think we want the same value as out[TX_ACCEPTER] as computed in check_balances:
https://github.com/vincenzopalazzo/lightning/blob/macros/remote-signer-with-splicing/channeld/channeld.c#L2998

Mh nice pointer, I was skipping this logic because it is really purely undocumented, but I spent some time moving code around, what do you think is we move this logic inside a new method? 059feff

Looks like that the our logic then will be really easy to implement
else
push_value = amount_msat(peer->splicing->accepter_relative);

/* Take the remote channel balance (before splicing) */
remote_old_vale = peer->channel->view[REMOTE].owed[REMOTE];
if (!amount_msat_add(&push_value, push_value, remote_old_vale))
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"Unable to add field amount %s to rolling"
" total %s",
fmt_amount_msat(tmpctx, push_value),
fmt_amount_msat(tmpctx, remote_old_vale));

/* local_upfront_shutdown_script, local_upfront_shutdown_wallet_index,
* remote_upfront_shutdown_script aren't allowed to change, so we
* don't need to gather them */
msg = towire_hsmd_ready_channel(
NULL,
peer->channel->opener == LOCAL,
inflight->amnt,
push_value,
&inflight->outpoint.txid,
inflight->outpoint.n,
peer->channel->config[LOCAL].to_self_delay,
/*local_upfront_shutdown_script*/ NULL,
/*local_upfront_shutdown_wallet_index*/ NULL,
&peer->channel->basepoints[REMOTE],
&peer->channel->funding_pubkey[REMOTE],
peer->channel->config[REMOTE].to_self_delay,
/*remote_upfront_shutdown_script*/ NULL,
peer->channel->type);

wire_sync_write(HSM_FD, take(msg));
msg = wire_sync_read(tmpctx, HSM_FD);
if (!fromwire_hsmd_ready_channel_reply(msg))
status_failed(STATUS_FAIL_HSM_IO, "Bad ready_channel_reply %s",
tal_hex(tmpctx, msg));
}

/* ACCEPTER side of the splice. Here we handle all the accepter's steps for the
* splice. Since the channel must be in STFU mode we block the daemon here until
* the splice is finished or aborted. */
Expand Down Expand Up @@ -3519,7 +3569,6 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
* The receiver of `splice_ack`:
* - MUST begin splice negotiation.
*/

ictx->next_update_fn = next_splice_step;
ictx->desired_psbt = NULL;
ictx->pause_when_complete = false;
Expand Down Expand Up @@ -3577,6 +3626,8 @@ static void splice_accepter(struct peer *peer, const u8 *inmsg)
new_inflight->last_tx = NULL;
new_inflight->i_am_initiator = false;

update_hsmd_with_splice(peer, new_inflight, true);

update_view_from_inflights(peer);

peer->splice_state->count++;
Expand Down Expand Up @@ -3811,6 +3862,8 @@ static void splice_initiator_user_finalized(struct peer *peer)
new_inflight->last_tx = NULL;
new_inflight->i_am_initiator = true;

update_hsmd_with_splice(peer, new_inflight, false);

update_view_from_inflights(peer);

peer->splice_state->count++;
Expand Down Expand Up @@ -5160,8 +5213,7 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg)

if (depth < peer->channel->minimum_depth) {
peer->depth_togo = peer->channel->minimum_depth - depth;
}
else {
} else {
peer->depth_togo = 0;

/* For splicing we only update the short channel id on mutual
Expand Down Expand Up @@ -5206,8 +5258,7 @@ static void handle_funding_depth(struct peer *peer, const u8 *msg)
peer_write(peer->pps, take(msg));

peer->channel_ready[LOCAL] = true;
}
else if(splicing && !peer->splice_state->locked_ready[LOCAL]) {
} else if(splicing && !peer->splice_state->locked_ready[LOCAL]) {
assert(scid);

msg = towire_splice_locked(NULL, &peer->channel_id);
Expand Down

0 comments on commit bf3a9bb

Please sign in to comment.