From c1bc4d0ead93ec1b0401edb3024488245a00e025 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 6 Apr 2023 09:03:25 +0930 Subject: [PATCH] onchaind: remove now-unused direct tx creation. Signed-off-by: Rusty Russell --- lightningd/onchain_control.c | 78 ------------ onchaind/onchaind.c | 198 ++++-------------------------- onchaind/onchaind_wire.csv | 8 -- onchaind/test/run-grind_feerate.c | 3 - 4 files changed, 25 insertions(+), 262 deletions(-) diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 8ef25f9b8..2f9a912fd 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -282,80 +282,6 @@ static void handle_onchain_log_coin_move(struct channel *channel, const u8 *msg) tal_free(mvt); } -/** handle_onchain_broadcast_rbf_tx_cb - * - * @brief suppresses the rebroadcast of a - * transaction. - * - * @desc when using the `bitcoin_tx` function, - * if a callback is not given, the transaction - * will be rebroadcast automatically by - * chaintopology. - * However, in the case of an RBF transaction - * from `onchaind`, `onchaind` will periodically - * create a new, higher-fee replacement, thus - * `onchaind` will trigger rebroadcast (with a - * higher fee) by itself, which the `lightningd` - * chaintopology should not repeat. - * This callback exists to suppress the - * rebroadcast behavior of chaintopology. - * - * @param channel - the channel for which the - * transaction was broadcast. - * @param success - whether the tx was broadcast. - * @param err - the error received from the - * underlying sendrawtx. - */ -static void handle_onchain_broadcast_rbf_tx_cb(struct channel *channel, - bool success, - const char *err) -{ - /* Victory is boring. */ - if (success) - return; - - /* Failure is unusual but not broken: it is possible that just - * as we were about to broadcast, a new block came in which - * contains a previous version of the transaction, thus - * causing the higher-fee replacement to fail broadcast. - * - * ...or it could be a bug in onchaind which prevents it from - * successfully RBFing out the transaction, in which case we - * should log it for devs to check. - */ - log_unusual(channel->log, - "Broadcast of RBF tx failed, " - "did a new block just come in? " - "error: %s", - err); -} - -static void handle_onchain_broadcast_tx(struct channel *channel, - const u8 *msg) -{ - struct bitcoin_tx *tx; - struct wallet *w = channel->peer->ld->wallet; - bool is_rbf; - - if (!fromwire_onchaind_broadcast_tx(msg, msg, &tx, &is_rbf)) { - channel_internal_error(channel, "Invalid onchain_broadcast_tx"); - return; - } - - tx->chainparams = chainparams; - - wallet_transaction_add(w, tx->wtx, 0, 0); - - /* We don't really care if it fails, we'll respond via watch. */ - /* If the onchaind signals this as RBF-able, then we also - * set allowhighfees, as the transaction may be RBFed into - * high feerates as protection against the MAD-HTLC attack. */ - broadcast_tx(channel->peer->ld->topology, channel, - tx, NULL, is_rbf, 0, - is_rbf ? &handle_onchain_broadcast_rbf_tx_cb : NULL, - NULL, NULL); -} - static void handle_onchain_unwatch_tx(struct channel *channel, const u8 *msg) { struct bitcoin_txid txid; @@ -1254,10 +1180,6 @@ static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds U handle_onchain_init_reply(sd->channel, msg); break; - case WIRE_ONCHAIND_BROADCAST_TX: - handle_onchain_broadcast_tx(sd->channel, msg); - break; - case WIRE_ONCHAIND_UNWATCH_TX: handle_onchain_unwatch_tx(sd->channel, msg); break; diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 81c1084be..50817b518 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -93,12 +93,9 @@ static u32 min_relay_feerate; /* If we broadcast a tx, or need a delay to resolve the output. */ struct proposed_resolution { - /* flag indicating we are a modern resolution, sent to lightningd. */ - bool via_lightningd; - /* Obsolete: if we created tx ourselves: */ - const struct bitcoin_tx *tx; /* Once we had lightningd create tx, here's what it told us * witnesses were (we ignore sigs!). */ + /* NULL if answer is to simply ignore it. */ const struct onchain_witness_element **welements; /* Non-zero if this is CSV-delayed. */ u32 depth_required; @@ -346,38 +343,6 @@ static void record_to_them_htlc_fulfilled(struct tracked_output *out, &out->payment_hash))); } -static void record_ignored_wallet_deposit(struct tracked_output *out) -{ - struct bitcoin_outpoint outpoint; - - /* FIXME: Would be clearer to omit the txid field, BUT the - * tests seem to assume it's there, and things break */ - if (!out->proposal->tx) - memset(&outpoint.txid, 0, sizeof(outpoint.txid)); - else - bitcoin_txid(out->proposal->tx, &outpoint.txid); - /* Every spend tx we construct has a single output. */ - outpoint.n = 0; - - enum mvt_tag tag = TO_WALLET; - if (out->tx_type == OUR_HTLC_TIMEOUT_TX - || out->tx_type == OUR_HTLC_SUCCESS_TX) - tag = HTLC_TX; - else if (out->tx_type == THEIR_REVOKED_UNILATERAL) - tag = PENALTY; - else if (out->tx_type == OUR_UNILATERAL - || out->tx_type == THEIR_UNILATERAL) { - if (out->output_type == OUR_HTLC) - tag = HTLC_TIMEOUT; - } - if (out->output_type == DELAYED_OUTPUT_TO_US) - tag = CHANNEL_TO_US; - - /* Record the in/out through the channel */ - record_channel_deposit(out, out->tx_blockheight, tag); - record_channel_withdrawal(&outpoint.txid, out, 0, IGNORED); -} - static void record_anchor(struct tracked_output *out) { enum mvt_tag *tags = new_tag_arr(NULL, ANCHOR); @@ -391,7 +356,6 @@ static void record_anchor(struct tracked_output *out) static void record_coin_movements(struct tracked_output *out, u32 blockheight, - const struct bitcoin_tx *tx, const struct bitcoin_txid *txid) { /* For 'timeout' htlcs, we re-record them as a deposit @@ -692,48 +656,11 @@ static void handle_spend_created(struct tracked_output *out, const u8 *msg) ignore_output(out); } -/* For old-style outputs where we've made our own txs. */ -static void proposal_meets_depth(struct tracked_output *out) -{ - assert(out->proposal); - /* If we simply wanted to ignore it after some depth */ - if (!out->proposal->tx) { - ignore_output(out); - - if (out->proposal->tx_type == THEIR_HTLC_TIMEOUT_TO_THEM) - record_external_deposit(out, out->tx_blockheight, - HTLC_TIMEOUT); - - return; - } - - status_debug("Broadcasting %s (%s) to resolve %s/%s", - tx_type_name(out->proposal->tx_type), - type_to_string(tmpctx, struct bitcoin_tx, out->proposal->tx), - tx_type_name(out->tx_type), - output_type_name(out->output_type)); - - wire_sync_write( - REQ_FD, - take(towire_onchaind_broadcast_tx( - NULL, out->proposal->tx, false))); - - /* Don't wait for this if we're ignoring the tiny payment. */ - if (out->proposal->tx_type == IGNORING_TINY_PAYMENT) { - ignore_output(out); - record_ignored_wallet_deposit(out); - } - - /* Otherwise we will get a callback when it's in a block. */ -} - static struct proposed_resolution *new_proposed_resolution(struct tracked_output *out, unsigned int block_required, enum tx_type tx_type) { struct proposed_resolution *proposal = tal(out, struct proposed_resolution); - proposal->via_lightningd = true; - proposal->tx = NULL; proposal->tx_type = tx_type; proposal->depth_required = block_required - out->tx_blockheight; @@ -801,68 +728,6 @@ static void propose_ignore(struct tracked_output *out, ignore_output(out); } -static bool is_valid_sig(const u8 *e) -{ - struct bitcoin_signature sig; - return signature_from_der(e, tal_count(e), &sig); -} - -/* We ignore things which look like signatures. */ -static bool input_similar(const struct wally_tx_input *i1, - const struct wally_tx_input *i2) -{ - u8 *s1, *s2; - - if (!memeq(i1->txhash, WALLY_TXHASH_LEN, i2->txhash, WALLY_TXHASH_LEN)) - return false; - - if (i1->index != i2->index) - return false; - - if (!scripteq(i1->script, i2->script)) - return false; - - if (i1->sequence != i2->sequence) - return false; - - if (i1->witness->num_items != i2->witness->num_items) - return false; - - for (size_t i = 0; i < i1->witness->num_items; i++) { - /* Need to wrap these in `tal_arr`s since the primitives - * except to be able to call tal_bytelen on them */ - s1 = tal_dup_arr(tmpctx, u8, i1->witness->items[i].witness, - i1->witness->items[i].witness_len, 0); - s2 = tal_dup_arr(tmpctx, u8, i2->witness->items[i].witness, - i2->witness->items[i].witness_len, 0); - - if (scripteq(s1, s2)) - continue; - - if (is_valid_sig(s1) && is_valid_sig(s2)) - continue; - return false; - } - - return true; -} - -static bool resolved_by_our_tx(const struct bitcoin_tx *tx, - const struct tx_parts *tx_parts) -{ - /* Our proposal can change as feerates change. Input - * comparison (ignoring signatures) works pretty well. */ - if (tal_count(tx_parts->inputs) != tx->wtx->num_inputs) - return false; - - for (size_t i = 0; i < tal_count(tx_parts->inputs); i++) { - if (!input_similar(tx_parts->inputs[i], - &tx->wtx->inputs[i])) - return false; - } - return true; -} - /* Do any of these tx_parts spend this outpoint? If so, return it */ static const struct wally_tx_input * which_input_spends(const struct tx_parts *tx_parts, @@ -905,23 +770,17 @@ static bool onchain_witness_element_matches(const struct onchain_witness_element static bool resolved_by_proposal(struct tracked_output *out, const struct tx_parts *tx_parts) { - /* Old case: we made the tx ourselves, so we compare that. */ - if (out->proposal->tx) { - if (!resolved_by_our_tx(out->proposal->tx, tx_parts)) - return false; - } else { - const struct wally_tx_input *input; + const struct wally_tx_input *input; - /* If there's no TX associated, it's not us. */ - if (!out->proposal->welements) - return false; - input = which_input_spends(tx_parts, &out->outpoint); - if (!input) - return false; - if (!onchain_witness_element_matches(out->proposal->welements, - input)) - return false; - } + /* If there's no TX associated, it's not us. */ + if (!out->proposal->welements) + return false; + + input = which_input_spends(tx_parts, &out->outpoint); + if (!input) + return false; + if (!onchain_witness_element_matches(out->proposal->welements, input)) + return false; out->resolved = tal(out, struct resolution); out->resolved->txid = tx_parts->txid; @@ -1369,7 +1228,6 @@ static void output_spent(struct tracked_output ***outs, tx_blockheight); record_coin_movements(out, tx_blockheight, - out->proposal->tx, &tx_parts->txid); return; } @@ -1560,10 +1418,6 @@ static void tx_new_depth(struct tracked_output **outs, } for (i = 0; i < tal_count(outs); i++) { - /* Update output depth. */ - if (bitcoin_txid_eq(&outs[i]->outpoint.txid, txid)) - outs[i]->depth = depth; - /* Is this tx resolving an output? */ if (outs[i]->resolved) { if (bitcoin_txid_eq(&outs[i]->resolved->txid, txid)) { @@ -1572,22 +1426,21 @@ static void tx_new_depth(struct tracked_output **outs, continue; } - /* Otherwise, is this something we have a pending - * resolution for? */ - if (outs[i]->proposal - && bitcoin_txid_eq(&outs[i]->outpoint.txid, txid) - && depth >= outs[i]->proposal->depth_required) { - if (outs[i]->proposal->via_lightningd) { - if (!outs[i]->proposal->welements) { - ignore_output(outs[i]); + /* Does it match this output? */ + if (!bitcoin_txid_eq(&outs[i]->outpoint.txid, txid)) + continue; - if (outs[i]->proposal->tx_type == THEIR_HTLC_TIMEOUT_TO_THEM) - record_external_deposit(outs[i], outs[i]->tx_blockheight, - HTLC_TIMEOUT); - } - } else { - proposal_meets_depth(outs[i]); - } + outs[i]->depth = depth; + + /* Are we supposed to ignore it now? */ + if (outs[i]->proposal + && depth >= outs[i]->proposal->depth_required + && !outs[i]->proposal->welements) { + ignore_output(outs[i]); + + if (outs[i]->proposal->tx_type == THEIR_HTLC_TIMEOUT_TO_THEM) + record_external_deposit(outs[i], outs[i]->tx_blockheight, + HTLC_TIMEOUT); } } } @@ -1833,7 +1686,6 @@ static void wait_for_resolved(struct tracked_output **outs) /* We send these, not receive! */ case WIRE_ONCHAIND_INIT_REPLY: - case WIRE_ONCHAIND_BROADCAST_TX: case WIRE_ONCHAIND_UNWATCH_TX: case WIRE_ONCHAIND_EXTRACTED_PREIMAGE: case WIRE_ONCHAIND_MISSING_HTLC_OUTPUT: diff --git a/onchaind/onchaind_wire.csv b/onchaind/onchaind_wire.csv index ca3eadadf..62c6719e8 100644 --- a/onchaind/onchaind_wire.csv +++ b/onchaind/onchaind_wire.csv @@ -67,14 +67,6 @@ msgdata,onchaind_htlcs,htlc,htlc_stub,num_htlcs msgdata,onchaind_htlcs,tell_if_missing,bool,num_htlcs msgdata,onchaind_htlcs,tell_immediately,bool,num_htlcs -# onchaind->master: Send out a tx. -# If is_rbf is false then master should rebroadcast the tx. -# If is_rbf is true then onchaind is responsible for rebroadcasting -# it with a higher fee. -msgtype,onchaind_broadcast_tx,5003 -msgdata,onchaind_broadcast_tx,tx,bitcoin_tx, -msgdata,onchaind_broadcast_tx,is_rbf,bool, - # master->onchaind: Notifier that an output has been spent by input_num of tx. msgtype,onchaind_spent,5004 msgdata,onchaind_spent,tx,tx_parts, diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index c780903ce..273b4ea2b 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -281,9 +281,6 @@ u8 *towire_onchaind_annotate_txin(const tal_t *ctx UNNEEDED, const struct bitcoi /* Generated stub for towire_onchaind_annotate_txout */ u8 *towire_onchaind_annotate_txout(const tal_t *ctx UNNEEDED, const struct bitcoin_outpoint *outpoint UNNEEDED, enum wallet_tx_type type UNNEEDED) { fprintf(stderr, "towire_onchaind_annotate_txout called!\n"); abort(); } -/* Generated stub for towire_onchaind_broadcast_tx */ -u8 *towire_onchaind_broadcast_tx(const tal_t *ctx UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, bool is_rbf UNNEEDED) -{ fprintf(stderr, "towire_onchaind_broadcast_tx called!\n"); abort(); } /* Generated stub for towire_onchaind_dev_memleak_reply */ u8 *towire_onchaind_dev_memleak_reply(const tal_t *ctx UNNEEDED, bool leak UNNEEDED) { fprintf(stderr, "towire_onchaind_dev_memleak_reply called!\n"); abort(); }