From ebd0a3fd69f70d51bc186f79b47f494416259220 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 27 Jul 2023 13:13:33 -0700 Subject: [PATCH] interactive-tx: Renaming for clarity and cleaning up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New daemon process means we don’t have to deal with gossip, so that gets removed along with error cleanup and a refactoring of how we calculating PDBT diffs. --- common/interactivetx.c | 169 ++++++++++++++++++++++++----------------- common/interactivetx.h | 22 +++--- 2 files changed, 110 insertions(+), 81 deletions(-) diff --git a/common/interactivetx.c b/common/interactivetx.c index 8b63a9cb5052..b3cb21f18d51 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -8,8 +8,6 @@ #include #include #include -#include -#include #include #include #include @@ -17,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -24,7 +23,6 @@ #include #include #include -#include /* * BOLT-f53ca2301232db780843e894f55d95d512f297f9 #2: @@ -64,14 +62,13 @@ struct interactivetx_context *new_interactivetx_context(const tal_t *ctx, { struct interactivetx_context *ictx = tal(ctx, struct interactivetx_context); - ictx->ctx = NULL; ictx->our_role = our_role; ictx->pps = pps; ictx->channel_id = channel_id; ictx->tx_add_input_count = 0; ictx->tx_add_output_count = 0; - ictx->next_update = default_next_update; - ictx->current_psbt = NULL; + ictx->next_update_fn = default_next_update; + ictx->current_psbt = create_psbt(ictx, 0, 0, 0); ictx->desired_psbt = NULL; ictx->pause_when_complete = false; ictx->change_set = NULL; @@ -102,26 +99,14 @@ static u8 *read_next_msg(const tal_t *ctx, for (;;) { char *desc; bool warning; - struct channel_id actual; enum peer_wire t; - bool from_gossipd; /* Prevent runaway memory usage from many messages */ if (msg) tal_free(msg); /* This helper routine polls the peer. */ - msg = peer_or_gossip_sync_read(ctx, state->pps, &from_gossipd); - - /* Line should be in STFU mode and not receiving gossip */ - if (from_gossipd) { - *error = tal_fmt(ctx, "interactivetx got gossip but" - " should be in STFU mode."); - - tal_free(msg); - /* Return NULL so caller knows to stop negotiating. */ - return NULL; - } + msg = peer_read(ctx, state->pps); /* BOLT #1: * @@ -140,7 +125,7 @@ static u8 *read_next_msg(const tal_t *ctx, if (!desc) continue; - *error = tal_fmt(ctx, "They sent a %s: %s" + *error = tal_fmt(ctx, "They sent a %s: %s", warning ? "warning" : "error", desc); @@ -160,9 +145,11 @@ static u8 *read_next_msg(const tal_t *ctx, case WIRE_TX_REMOVE_OUTPUT: case WIRE_TX_COMPLETE: return msg; + case WIRE_TX_ABORT: + /* TODO */ case WIRE_TX_SIGNATURES: - case WIRE_FUNDING_LOCKED: - case WIRE_INIT_RBF: + case WIRE_CHANNEL_READY: + case WIRE_TX_INIT_RBF: case WIRE_OPEN_CHANNEL2: case WIRE_INIT: case WIRE_ERROR: @@ -184,7 +171,7 @@ static u8 *read_next_msg(const tal_t *ctx, case WIRE_GOSSIP_TIMESTAMP_FILTER: case WIRE_ONION_MESSAGE: case WIRE_ACCEPT_CHANNEL2: - case WIRE_ACK_RBF: + case WIRE_TX_ACK_RBF: case WIRE_CHANNEL_ANNOUNCEMENT: case WIRE_CHANNEL_UPDATE: case WIRE_NODE_ANNOUNCEMENT: @@ -197,6 +184,11 @@ static u8 *read_next_msg(const tal_t *ctx, case WIRE_PONG: case WIRE_SHUTDOWN: case WIRE_STFU: + case WIRE_PEER_STORAGE: + case WIRE_YOUR_PEER_STORAGE: + case WIRE_SPLICE: + case WIRE_SPLICE_ACK: + case WIRE_SPLICE_LOCKED: *error = tal_fmt(ctx, "Received invalid message from peer: %d", t); return NULL; @@ -211,7 +203,7 @@ static char *send_next(const tal_t *ctx, struct channel_id *cid = &ictx->channel_id; struct psbt_changeset *set = ictx->change_set; u64 serial_id; - u8 *msg = NULL; + u8 *msg; *finished = false; if (!set) @@ -219,35 +211,29 @@ static char *send_next(const tal_t *ctx, if (tal_count(set->added_ins) != 0) { const struct input_set *in = &set->added_ins[0]; - struct bitcoin_outpoint outpoint; u8 *prevtx; if (!psbt_get_serial_id(&in->input.unknowns, &serial_id)) - return "interactivetx ADD_INPUT PSBT has invalid serial_id."; + return "interactivetx ADD_INPUT PSBT has invalid" + " serial_id."; if (in->input.utxo) - prevtx = linearize_wtx(ctx, - in->input.utxo); + prevtx = linearize_wtx(ctx, in->input.utxo); else - return "interactivetx ADD_INPUT PSBT needs the previous transaction set."; - - memcpy(outpoint.txid.shad.sha.u.u8, - in->tx_input.txhash, - WALLY_TXHASH_LEN); - - outpoint.n = in->tx_input.index; + return "interactivetx ADD_INPUT PSBT needs the previous" + " transaction set."; msg = towire_tx_add_input(NULL, cid, serial_id, - prevtx, in->tx_input.index, - in->tx_input.sequence, - NULL); + prevtx, in->input.index, + in->input.sequence); tal_arr_remove(&set->added_ins, 0); } else if (tal_count(set->rm_ins) != 0) { if (!psbt_get_serial_id(&set->rm_ins[0].input.unknowns, &serial_id)) - return "interactivetx RM_INPUT PSBT has invalid serial_id."; + return "interactivetx RM_INPUT PSBT has invalid" + " serial_id."; msg = towire_tx_remove_input(NULL, cid, serial_id); @@ -262,11 +248,12 @@ static char *send_next(const tal_t *ctx, out = &set->added_outs[0]; if (!psbt_get_serial_id(&out->output.unknowns, &serial_id)) - return "interactivetx ADD_OUTPUT PSBT has invalid serial_id."; + return "interactivetx ADD_OUTPUT PSBT has invalid" + " serial_id."; - asset_amt = wally_tx_output_get_amount(&out->tx_output); + asset_amt = wally_psbt_output_get_amount(&out->output); sats = amount_asset_to_sat(&asset_amt); - script = wally_tx_output_get_script(ctx, &out->tx_output); + script = wally_psbt_output_get_script(ctx, &out->output); msg = towire_tx_add_output(NULL, cid, @@ -290,33 +277,74 @@ static char *send_next(const tal_t *ctx, if (!msg) return "Interactivetx send_next failed to build a message"; - sync_crypto_write(ictx->pps, take(msg)); + peer_write(ictx->pps, take(msg)); return NULL; tx_complete: - *finished = true; if (!ictx->pause_when_complete) { if (ictx->current_psbt->num_inputs > MAX_FUNDING_INPUTS) return tal_fmt(ctx, "Humbly refusing to `tx_complete` " "because we have too many inputs (%zu). " - "Limit is %zu." + "Limit is %d.", ictx->current_psbt->num_inputs, MAX_FUNDING_INPUTS); if (ictx->current_psbt->num_outputs > MAX_FUNDING_OUTPUTS) return tal_fmt(ctx, "Humbly refusing to `tx_complete` " "because we have too many outputs (%zu). " - "Limit is %zu." + "Limit is %d.", ictx->current_psbt->num_outputs, MAX_FUNDING_OUTPUTS); - msg = towire_tx_complete(ctx, cid); - sync_crypto_write(ictx->pps, msg); + msg = towire_tx_complete(NULL, cid); + peer_write(ictx->pps, take(msg)); } + *finished = true; return NULL; } +static struct psbt_changeset *get_changes(const tal_t *ctx, + struct interactivetx_context *ictx, + struct wally_psbt *next_psbt) +{ + u64 serial_id; + struct psbt_changeset *set = psbt_get_changeset(tmpctx, + ictx->current_psbt, + next_psbt); + + /* Remove duplicate serial_ids from the change set. */ + for (int i = 0; i < tal_count(set->added_ins); i++) { + struct bitcoin_outpoint point; + wally_psbt_input_get_outpoint(&set->added_ins[i].input, &point); + if (psbt_get_serial_id(&set->added_ins[i].input.unknowns, + &serial_id)) { + if (psbt_find_serial_input(ictx->current_psbt, + serial_id) != -1) + tal_arr_remove(&set->added_ins, i--); + else if (psbt_has_input(ictx->current_psbt, &point)) + tal_arr_remove(&set->added_ins, i--); + } + } + for (int i = 0; i < tal_count(set->added_outs); i++) + if (psbt_get_serial_id(&set->added_outs[i].output.unknowns, + &serial_id)) + if (psbt_find_serial_output(ictx->current_psbt, + serial_id) != -1) + tal_arr_remove(&set->added_outs, i--); + + return set; +} + +bool interactivetx_has_changes(struct interactivetx_context *ictx, + struct wally_psbt *next_psbt) +{ + struct psbt_changeset *set = get_changes(tmpctx, ictx, next_psbt); + + return tal_count(set->added_ins) || tal_count(set->rm_ins) + || tal_count(set->added_outs) || tal_count(set->rm_outs); +} + char *process_interactivetx_updates(const tal_t *ctx, struct interactivetx_context *ictx, bool *received_tx_complete) @@ -326,26 +354,21 @@ char *process_interactivetx_updates(const tal_t *ctx, char *error = NULL; struct wally_psbt *next_psbt; - if (ictx->current_psbt == NULL) - ictx->current_psbt = create_psbt(ictx, 0, 0, 0); - if (received_tx_complete) they_complete = *received_tx_complete; /* Build change_set and handle PSBT variables */ ictx->change_set = tal_free(ictx->change_set); - /* Call next_update or default to 'desired_psbt' */ - next_psbt = ictx->next_update(ictx, ictx); + /* Call next_update_fn or default to 'desired_psbt' */ + next_psbt = ictx->next_update_fn(ictx, ictx); - /* Returning NULL from next_update is the same as using `current_psbt` + /* Returning NULL from next_update_fn is the same as using `current_psbt` * with no changes -- both indicate no changes */ if (!next_psbt) next_psbt = ictx->current_psbt; - ictx->change_set = psbt_get_changeset(ictx, - ictx->current_psbt, - next_psbt); + ictx->change_set = get_changes(ctx, ictx, next_psbt); /* If current_psbt and next_psbt are the same, dont double free it! * Otherwise we advance `current_psbt` to `next_psbt` and begin @@ -353,7 +376,7 @@ char *process_interactivetx_updates(const tal_t *ctx, if (ictx->current_psbt != next_psbt) { /* psbt_get_changeset requires we keep the current_psbt until * we're done withh change_set */ - tal_steal(ictx->change_set, current_psbt); + tal_steal(ictx->change_set, ictx->current_psbt); ictx->current_psbt = next_psbt; } @@ -389,7 +412,7 @@ char *process_interactivetx_updates(const tal_t *ctx, t = fromwire_peektype(msg); switch (t) { case WIRE_TX_ADD_INPUT: { - const u8 *tx_bytes, *redeemscript; + const u8 *tx_bytes; u32 sequence; size_t len; struct bitcoin_tx *tx; @@ -399,9 +422,7 @@ char *process_interactivetx_updates(const tal_t *ctx, &serial_id, cast_const2(u8 **, &tx_bytes), - &outpoint.n, &sequence, - cast_const2(u8 **, - &redeemscript))) + &outpoint.n, &sequence)) return tal_fmt(ctx, "Parsing tx_add_input %s", tal_hex(ctx, msg)); @@ -435,7 +456,7 @@ char *process_interactivetx_updates(const tal_t *ctx, * the transaction */ if (psbt_find_serial_input(ictx->current_psbt, serial_id) != -1) - return tal_fmt(ctx, "Duplicate serial_id rcvd." + return tal_fmt(ctx, "Duplicate serial_id rcvd" " %"PRIu64, serial_id); /* Convert tx_bytes to a tx! */ @@ -459,7 +480,7 @@ char *process_interactivetx_updates(const tal_t *ctx, */ if (!is_segwit_output(ctx, &tx->wtx->outputs[outpoint.n], - redeemscript)) + NULL)) return tal_fmt(ctx, "Invalid tx sent. Not SegWit %s", type_to_string(ctx, @@ -491,7 +512,7 @@ char *process_interactivetx_updates(const tal_t *ctx, */ if (ictx->current_psbt->num_inputs + 1 > MAX_FUNDING_INPUTS) return tal_fmt(ctx, "Too many inputs. Have %zu," - " Max allowed %zu", + " Max allowed %d", ictx->current_psbt->num_inputs + 1, MAX_FUNDING_INPUTS); @@ -618,7 +639,7 @@ char *process_interactivetx_updates(const tal_t *ctx, */ if (ictx->current_psbt->num_outputs + 1 > MAX_FUNDING_OUTPUTS) return tal_fmt(ctx, "Too many inputs. Have %zu," - " Max allowed %zu", + " Max allowed %d", ictx->current_psbt->num_outputs + 1, MAX_FUNDING_OUTPUTS); @@ -676,6 +697,8 @@ char *process_interactivetx_updates(const tal_t *ctx, if (received_tx_complete) *received_tx_complete = true; break; + case WIRE_TX_ABORT: + /* Todo */ case WIRE_INIT: case WIRE_ERROR: case WIRE_WARNING: @@ -683,7 +706,7 @@ char *process_interactivetx_updates(const tal_t *ctx, case WIRE_ACCEPT_CHANNEL: case WIRE_FUNDING_CREATED: case WIRE_FUNDING_SIGNED: - case WIRE_FUNDING_LOCKED: + case WIRE_CHANNEL_READY: case WIRE_SHUTDOWN: case WIRE_CLOSING_SIGNED: case WIRE_UPDATE_ADD_HTLC: @@ -697,13 +720,12 @@ char *process_interactivetx_updates(const tal_t *ctx, case WIRE_CHANNEL_REESTABLISH: case WIRE_ANNOUNCEMENT_SIGNATURES: case WIRE_GOSSIP_TIMESTAMP_FILTER: - case WIRE_OBS2_ONION_MESSAGE: case WIRE_ONION_MESSAGE: case WIRE_TX_SIGNATURES: case WIRE_OPEN_CHANNEL2: case WIRE_ACCEPT_CHANNEL2: - case WIRE_INIT_RBF: - case WIRE_ACK_RBF: + case WIRE_TX_INIT_RBF: + case WIRE_TX_ACK_RBF: case WIRE_CHANNEL_ANNOUNCEMENT: case WIRE_CHANNEL_UPDATE: case WIRE_NODE_ANNOUNCEMENT: @@ -713,9 +735,12 @@ char *process_interactivetx_updates(const tal_t *ctx, case WIRE_REPLY_SHORT_CHANNEL_IDS_END: case WIRE_PING: case WIRE_PONG: - case WIRE_STFU: + case WIRE_PEER_STORAGE: + case WIRE_YOUR_PEER_STORAGE: case WIRE_SPLICE: case WIRE_SPLICE_ACK: + case WIRE_STFU: + case WIRE_SPLICE_LOCKED: return tal_fmt(ctx, "Unexpected wire message %s", tal_hex(ctx, msg)); } @@ -727,5 +752,7 @@ char *process_interactivetx_updates(const tal_t *ctx, /* Sort psbt! */ psbt_sort_by_serial_id(ictx->current_psbt); + tal_steal(ictx, ictx->current_psbt); + return NULL; } diff --git a/common/interactivetx.h b/common/interactivetx.h index 0d8a27588947..a63d01896dd9 100644 --- a/common/interactivetx.h +++ b/common/interactivetx.h @@ -26,9 +26,6 @@ enum tx_msgs { struct interactivetx_context { - /* Users can set this to their own context */ - void *ctx; - enum tx_role our_role; struct per_peer_state *pps; struct channel_id channel_id; @@ -45,16 +42,14 @@ struct interactivetx_context { * If no more changes are demanded, return NULL or current_psbt * unchanged to signal completion. */ - struct wally_psbt *(*next_update)(const tal_t *ctx, + struct wally_psbt *(*next_update_fn)(const tal_t *ctx, struct interactivetx_context *ictx); - /* Set this to the intial psbt. If NULL will be filled with an empty - * psbt. - */ + /* Set this to the intial psbt. Defaults to an empty PSBT. */ struct wally_psbt *current_psbt; /* Optional field for storing your side's desired psbt state, to be - * used inside 'next_update'. + * used inside 'next_update_fn'. */ struct wally_psbt *desired_psbt; @@ -74,8 +69,8 @@ struct interactivetx_context *new_interactivetx_context(const tal_t *ctx, struct channel_id channel_id); /* Blocks the thread until we run out of changes (and we send tx_complete), - * or an error occurs. If 'pause_when_complete' is set, this behavior changes - * and we return without sending tx_complete. + * or an error occurs. If 'pause_when_complete' on the `interactivetx_context` + * is set, this behavior changes and we return without sending tx_complete. * * If received_tx_complete is not NULL: * in -> true means we assume we've received tx_complete in a previous round. @@ -87,4 +82,11 @@ char *process_interactivetx_updates(const tal_t *ctx, struct interactivetx_context *ictx, bool *received_tx_complete); +/* If the given ictx would cause `process_interactivetx_updates to send tx + * changes when called. Returns true if an error occurs + * (call `process_interactivetx_updates` for a description of the error). + */ +bool interactivetx_has_changes(struct interactivetx_context *ictx, + struct wally_psbt *next_psbt); + #endif /* LIGHTNING_COMMON_INTERACTIVETX_H */