Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce mem and cpu usage of onchaind for long-lived channels #4250

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions bitcoin/tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ bool bitcoin_tx_check(const struct bitcoin_tx *tx)
if (written != tal_bytelen(newtx))
return false;

tal_free(newtx);
return true;
}

Expand Down
10 changes: 10 additions & 0 deletions common/daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ static void add_steal_notifiers(const tal_t *root)
for (const tal_t *i = tal_first(root); i; i = tal_next(i))
add_steal_notifiers(i);
}

static void remove_steal_notifiers(void)
{
/* We remove this from root, assuming everything else freed. */
tal_del_notifier(NULL, add_steal_notifier);
}
#endif

void daemon_setup(const char *argv0,
Expand Down Expand Up @@ -155,6 +161,10 @@ void daemon_setup(const char *argv0,
void daemon_shutdown(void)
{
common_shutdown();

#if DEVELOPER && BACKTRACE_SUPPORTED
remove_steal_notifiers();
#endif
}

void daemon_maybe_debug(char *argv[])
Expand Down
71 changes: 71 additions & 0 deletions onchaind/onchaind.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <bitcoin/feerate.h>
#include <bitcoin/psbt.h>
#include <bitcoin/script.h>
#include <ccan/asort/asort.h>
#include <ccan/crypto/shachain/shachain.h>
#include <ccan/mem/mem.h>
#include <ccan/tal/str/str.h>
Expand Down Expand Up @@ -142,6 +143,55 @@ static bool wally_tx_output_scripteq(const struct wally_tx_output *out,
return memeq(out->script, out->script_len, script, tal_bytelen(script));
}

/* The feerate for the HTLC txs (which we grind) are the same as the
* feerate for the main tx. However, there may be dust HTLCs which
* were added to the fee, so we can only estimate a maximum feerate */
static void trim_maximum_feerate(struct amount_sat funding,
const struct tx_parts *commitment)
{
size_t weight;
struct amount_sat fee = funding;

/* FIXME: This doesn't work for elements? */
if (chainparams->is_elements)
return;
Comment on lines +155 to +157
Copy link
Member

Choose a reason for hiding this comment

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

Good question, I don't think it should be too much of an issue, given that we currently only have one main-asset input, which also makes the below if (!amount_asset_is_main(&amt)) always false, but better keep it in in case we eventually add multi-input and multi-asset channels.

Was there a concrete error that was triggered on elements, or is this more of a precaution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we had a -1ULL satoshis value. Then added this test and this function caused an onchaind failure (presumably by giving the wrong result).


weight = bitcoin_tx_core_weight(tal_count(commitment->inputs),
tal_count(commitment->outputs));

/* BOLT #3:
* ## Commitment Transaction
*...
* * `txin[0]` script bytes: 0
* * `txin[0]` witness: `0 <signature_for_pubkey1> <signature_for_pubkey2>`
*/
/* Account for witness (1 byte count + 1 empty + sig + sig) */
assert(tal_count(commitment->inputs) == 1);
weight += bitcoin_tx_input_weight(false, 1 + 1 + 2 * bitcoin_tx_input_sig_weight());

for (size_t i = 0; i < tal_count(commitment->outputs); i++) {
struct amount_asset amt;
weight += bitcoin_tx_output_weight(commitment->outputs[i]
->script_len);

amt = wally_tx_output_get_amount(commitment->outputs[i]);
if (!amount_asset_is_main(&amt))
continue;
if (!amount_sat_sub(&fee, fee, amount_asset_to_sat(&amt))) {
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"Unable to subtract fee");
}
}

status_debug("reducing max_possible_feerate from %u...",
max_possible_feerate);
/* This is naive, but simple. */
while (amount_sat_greater(amount_tx_fee(max_possible_feerate, weight),
fee))
max_possible_feerate--;
status_debug("... to %u", max_possible_feerate);
}

static void send_coin_mvt(struct chain_coin_mvt *mvt TAKES)
{
wire_sync_write(REQ_FD,
Expand Down Expand Up @@ -2263,6 +2313,12 @@ static size_t resolve_our_htlc_ourcommit(struct tracked_output *out,
/* These htlcs are all possibilities, but signature will only match
* one with the correct cltv: check which that is. */
for (i = 0; i < tal_count(matches); i++) {
/* Skip over duplicate HTLCs, since we only need one. */
if (i > 0
&& (htlcs[matches[i]].cltv_expiry
== htlcs[matches[i-1]].cltv_expiry))
continue;

/* BOLT #5:
*
* ## HTLC Output Handling: Local Commitment, Local Offers
Expand Down Expand Up @@ -3640,6 +3696,16 @@ static void handle_unknown_commitment(const struct tx_parts *tx,
wait_for_resolved(outs);
}

static int cmp_htlc_cltv(const struct htlc_stub *a,
const struct htlc_stub *b, void *unused)
{
if (a->cltv_expiry < b->cltv_expiry)
return -1;
else if (a->cltv_expiry > b->cltv_expiry)
return 1;
return 0;
}

int main(int argc, char *argv[])
{
setup_locale();
Expand Down Expand Up @@ -3731,6 +3797,9 @@ int main(int argc, char *argv[])
master_badmsg(WIRE_ONCHAIND_HTLC, msg);
}

/* Sort by CLTV, so matches are in CLTV order (and easy to skip dups) */
asort(htlcs, tal_count(htlcs), cmp_htlc_cltv, NULL);

outs = tal_arr(ctx, struct tracked_output *, 0);
wally_tx_input_get_txid(tx->inputs[0], &tmptxid);
new_tracked_output(&outs, &tmptxid,
Expand All @@ -3747,6 +3816,8 @@ int main(int argc, char *argv[])
type_to_string(tmpctx, struct pubkey,
&old_remote_per_commit_point));

trim_maximum_feerate(funding, tx);

/* BOLT #5:
*
* There are three ways a channel can end:
Expand Down
15 changes: 15 additions & 0 deletions onchaind/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,21 @@ update-mocks: $(ONCHAIND_TEST_SRC:%=update-mocks/%)

$(ONCHAIND_TEST_PROGRAMS): $(ONCHAIND_TEST_COMMON_OBJS) $(BITCOIN_OBJS)

# This needs many more objs:
onchaind/test/run-onchainstress: \
common/htlc_tx.o \
common/derive_basepoints.o \
common/daemon.o \
common/htlc_wire.o \
common/initial_commit_tx.o \
common/key_derive.o \
common/keyset.o \
common/permute_tx.o \
common/subdaemon.o \
common/wallet.o \
wire/towire.o \
wire/fromwire.o

# Test objects depend on ../ src and headers.
$(ONCHAIND_TEST_OBJS): $(ONCHAIND_HEADERS) $(ONCHAIND_SRC)

Expand Down
Binary file added onchaind/test/onchainstress-data.gz
Binary file not shown.
224 changes: 224 additions & 0 deletions onchaind/test/run-onchainstress.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
#include "../onchaind.c"
#include "../onchaind_wiregen.c"
#include "../onchaind_wire.c"
#include "../../hsmd/hsmd_wiregen.c"
#include <ccan/str/hex/hex.h>
#include <common/dev_disconnect.h>
#include <common/onionreply.h>
#include <err.h>
#include <zlib.h>

volatile bool logging_io;
struct backtrace_state *backtrace_state;

/* AUTOGENERATED MOCKS START */
/* Generated stub for dup_onionreply */
struct onionreply *dup_onionreply(const tal_t *ctx UNNEEDED,
const struct onionreply *r TAKES UNNEEDED)
{ fprintf(stderr, "dup_onionreply called!\n"); abort(); }
/* Generated stub for fromwire_bip32_key_version */
void fromwire_bip32_key_version(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
struct bip32_key_version *version UNNEEDED)
{ fprintf(stderr, "fromwire_bip32_key_version called!\n"); abort(); }
/* Generated stub for fromwire_chain_coin_mvt */
void fromwire_chain_coin_mvt(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct chain_coin_mvt *mvt UNNEEDED)
{ fprintf(stderr, "fromwire_chain_coin_mvt called!\n"); abort(); }
/* Generated stub for fromwire_ext_key */
void fromwire_ext_key(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct ext_key *bip32 UNNEEDED)
{ fprintf(stderr, "fromwire_ext_key called!\n"); abort(); }
/* Generated stub for fromwire_node_id */
void fromwire_node_id(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct node_id *id UNNEEDED)
{ fprintf(stderr, "fromwire_node_id called!\n"); abort(); }
/* Generated stub for fromwire_onionreply */
struct onionreply *fromwire_onionreply(const tal_t *ctx UNNEEDED,
const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
{ fprintf(stderr, "fromwire_onionreply called!\n"); abort(); }
/* Generated stub for fromwire_utxo */
struct utxo *fromwire_utxo(const tal_t *ctx UNNEEDED, const u8 **ptr UNNEEDED, size_t *max UNNEEDED)
{ fprintf(stderr, "fromwire_utxo called!\n"); abort(); }
/* Generated stub for master_badmsg */
void master_badmsg(u32 type_expected UNNEEDED, const u8 *msg)
{ fprintf(stderr, "master_badmsg called!\n"); abort(); }
/* Generated stub for memleak_find_allocations */
struct htable *memleak_find_allocations(const tal_t *ctx UNNEEDED,
const void *exclude1 UNNEEDED,
const void *exclude2 UNNEEDED)
{ fprintf(stderr, "memleak_find_allocations called!\n"); abort(); }
/* Generated stub for memleak_get */
const void *memleak_get(struct htable *memtable UNNEEDED, const uintptr_t **backtrace UNNEEDED)
{ fprintf(stderr, "memleak_get called!\n"); abort(); }
/* Generated stub for memleak_init */
void memleak_init(void)
{ fprintf(stderr, "memleak_init called!\n"); abort(); }
/* Generated stub for memleak_remove_region */
void memleak_remove_region(struct htable *memtable UNNEEDED,
const void *p UNNEEDED, size_t bytelen UNNEEDED)
{ fprintf(stderr, "memleak_remove_region called!\n"); abort(); }
/* Generated stub for new_coin_penalty_sat */
struct chain_coin_mvt *new_coin_penalty_sat(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *txid UNNEEDED,
const struct bitcoin_txid *out_txid UNNEEDED,
u32 vout UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_sat amount UNNEEDED)
{ fprintf(stderr, "new_coin_penalty_sat called!\n"); abort(); }
/* Generated stub for new_coin_withdrawal */
struct chain_coin_mvt *new_coin_withdrawal(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *tx_txid UNNEEDED,
const struct bitcoin_txid *out_txid UNNEEDED,
u32 vout UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_msat amount UNNEEDED)
{ fprintf(stderr, "new_coin_withdrawal called!\n"); abort(); }
/* Generated stub for status_failed */
void status_failed(enum status_failreason code UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "status_failed called!\n"); abort(); }
/* Generated stub for status_vfmt */
void status_vfmt(enum log_level level UNNEEDED,
const struct node_id *peer UNNEEDED,
const char *fmt UNNEEDED, va_list ap UNNEEDED)
{ fprintf(stderr, "status_vfmt called!\n"); abort(); }
/* Generated stub for towire_bip32_key_version */
void towire_bip32_key_version(u8 **cursor UNNEEDED, const struct bip32_key_version *version UNNEEDED)
{ fprintf(stderr, "towire_bip32_key_version called!\n"); abort(); }
/* Generated stub for towire_ext_key */
void towire_ext_key(u8 **pptr UNNEEDED, const struct ext_key *bip32 UNNEEDED)
{ fprintf(stderr, "towire_ext_key called!\n"); abort(); }
/* Generated stub for towire_node_id */
void towire_node_id(u8 **pptr UNNEEDED, const struct node_id *id UNNEEDED)
{ fprintf(stderr, "towire_node_id called!\n"); abort(); }
/* Generated stub for towire_onionreply */
void towire_onionreply(u8 **cursor UNNEEDED, const struct onionreply *r UNNEEDED)
{ fprintf(stderr, "towire_onionreply called!\n"); abort(); }
/* Generated stub for towire_utxo */
void towire_utxo(u8 **pptr UNNEEDED, const struct utxo *utxo UNNEEDED)
{ fprintf(stderr, "towire_utxo called!\n"); abort(); }
/* Generated stub for version */
const char *version(void)
{ fprintf(stderr, "version called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */

#if DEVELOPER
/* Generated stub for dev_disconnect_init */
void dev_disconnect_init(int fd UNNEEDED)
{ fprintf(stderr, "dev_disconnect_init called!\n"); abort(); }
#endif

/* Noops */
void status_setup_sync(int fd UNNEEDED)
{
}
void status_fmt(enum log_level level,
const struct node_id *peer,
const char *fmt, ...)
{
}
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{
return (void *)ptr;
}
void peer_billboard(bool perm UNNEEDED, const char *fmt UNNEEDED, ...)
{
}
struct chain_coin_mvt *new_coin_chain_fees(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *tx_txid UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_msat amount UNNEEDED)
{
return NULL;
}

/* Generated stub for new_coin_chain_fees_sat */
struct chain_coin_mvt *new_coin_chain_fees_sat(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *tx_txid UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_sat amount UNNEEDED)
{
return NULL;
}
/* Generated stub for new_coin_journal_entry */
struct chain_coin_mvt *new_coin_journal_entry(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *txid UNNEEDED,
const struct bitcoin_txid *out_txid UNNEEDED,
u32 vout UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_msat amount UNNEEDED,
bool is_credit UNNEEDED)
{
return NULL;
}
struct chain_coin_mvt *new_coin_onchain_htlc_sat(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *txid UNNEEDED,
const struct bitcoin_txid *out_txid UNNEEDED,
u32 vout UNNEEDED,
struct sha256 payment_hash UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_sat amount UNNEEDED,
bool is_credit UNNEEDED)
{
return NULL;
}
struct chain_coin_mvt *new_coin_withdrawal_sat(const tal_t *ctx UNNEEDED,
const char *account_name UNNEEDED,
const struct bitcoin_txid *tx_txid UNNEEDED,
const struct bitcoin_txid *out_txid UNNEEDED,
u32 vout UNNEEDED,
u32 blockheight UNNEEDED,
struct amount_sat amount UNNEEDED)
{
return NULL;
}
void towire_chain_coin_mvt(u8 **pptr UNNEEDED, const struct chain_coin_mvt *mvt UNNEEDED)
{
}

bool wire_sync_write(int fd, const void *msg TAKES)
{
if (taken(msg))
tal_free(msg);
return true;
}

u8 *wire_sync_read(const tal_t *ctx, int fd)
{
char line[5000];
u8 *ret;
static gzFile stream;
size_t hexlen;

/* Don't run this under valgrind in CI: takes too long! */
if (getenv("VALGRIND") && streq(getenv("VALGRIND"), "1"))
goto exit;

if (!stream) {
stream = gzopen("onchaind/test/onchainstress-data.gz", "rb");
if (!stream)
err(1, "opening onchaind/test/onchainstress-data.gz");
}

do {
if (!gzgets(stream, line, sizeof(line)))
goto exit;
} while (!strstarts(line, "read "));

/* Ignore prefix and \n at end */
hexlen = strlen(line) - strlen("read ") - 1;
ret = tal_arr(ctx, u8, hex_data_size(hexlen));
if (!hex_decode(line + strlen("read "), hexlen, ret, tal_bytelen(ret)))
errx(1, "Bad hex string '%s'", line);
return ret;

exit:
daemon_shutdown();
/* Free top-level ctx pointer! */
while (tal_first(NULL))
tal_free(tal_first(NULL));
exit(0);
}