Skip to content

Commit

Permalink
onchaind: remove now-unused direct tx creation.
Browse files Browse the repository at this point in the history 
Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
@rustyrussell
rustyrussell committed Apr 7, 2023
1 parent 9496e9f commit c1bc4d0
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 262 deletions.
78 changes: 0 additions & 78 deletions lightningd/onchain_control.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
198 changes: 25 additions & 173 deletions onchaind/onchaind.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -1572,22 +1426,21 @@ static void tx_new_depth(struct tracked_output **outs,
continue;
}

/* Otherwise, is this something we have a pending
* resolution for? */
/* Does it match this output? */
if (!bitcoin_txid_eq(&outs[i]->outpoint.txid, txid))
continue;

outs[i]->depth = depth;

/* Are we supposed to ignore it now? */
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]);

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]);
}
&& 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);
}
}
}
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 0 additions & 8 deletions onchaind/onchaind_wire.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions onchaind/test/run-grind_feerate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down

0 comments on commit c1bc4d0

Please sign in to comment.