From 5f41d9247ef14d188ff04c8865dae5148a3574ec Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 19 Jul 2022 17:04:36 +0930 Subject: [PATCH] bkpr: properly account for onchain fees for channel closes onchain fees are weird at channel close because: - you may be missing an trimmed htlc (which went to fees) - the balance from close may have been rounded (msats cant land on chain) - the close might have been a past state and you've actually ended up with more money onchain than you had in the channel. wut This commit accounts for all of this appropriately, with some tests. channel_close.debit should equal onchain_fee.credit (for that txid) plus sum(chain_event.credit [wallet/channel_acct]). In the penalty case, channel_close.debit becomes channel_close.debit + penalty_adj.debit, i.e. channel-close.debit + (penalty_adj.debit) = onchain_fee.credit + sum(chain_event.credit [wallet/channel_acct]) --- plugins/bkpr/account_entry.c | 1 + plugins/bkpr/account_entry.h | 1 + plugins/bkpr/bookkeeper.c | 44 ++++- plugins/bkpr/recorder.c | 223 ++++++++++++++++++++----- plugins/bkpr/recorder.h | 11 ++ plugins/bkpr/test/run-bkpr_db.c | 19 +++ plugins/bkpr/test/run-recorder.c | 270 +++++++++++++++++++++++-------- tests/test_closing.py | 101 ++++++++++++ 8 files changed, 563 insertions(+), 107 deletions(-) diff --git a/plugins/bkpr/account_entry.c b/plugins/bkpr/account_entry.c index 10cd29a75..7179f761f 100644 --- a/plugins/bkpr/account_entry.c +++ b/plugins/bkpr/account_entry.c @@ -6,6 +6,7 @@ static const char *tags[] = { "journal_entry", + "penalty_adj", }; const char *account_entry_tag_str(enum account_entry_tag tag) diff --git a/plugins/bkpr/account_entry.h b/plugins/bkpr/account_entry.h index d0e608ba8..fed529847 100644 --- a/plugins/bkpr/account_entry.h +++ b/plugins/bkpr/account_entry.h @@ -5,6 +5,7 @@ #define NUM_ACCOUNT_ENTRY_TAGS (JOURNAL_ENTRY + 1) enum account_entry_tag { JOURNAL_ENTRY = 0, + PENALTY_ADJ = 1, }; /* Convert an enum into a string */ diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c index 6d74c2b7a..afe7b4717 100644 --- a/plugins/bkpr/bookkeeper.c +++ b/plugins/bkpr/bookkeeper.c @@ -701,6 +701,35 @@ listpeers_multi_done(struct command *cmd, return notification_handled(cmd); } +static char *do_account_close_checks(const tal_t *ctx, + struct chain_event *e, + struct account *acct) +{ + struct account *closed_acct; + + db_begin_transaction(db); + + /* If is an external acct event, might be close channel related */ + if (!is_channel_account(acct) && !e->spending_txid) + closed_acct = find_close_account(ctx, db, &e->outpoint.txid); + else + closed_acct = acct; + + if (closed_acct && closed_acct->closed_event_db_id) { + maybe_mark_account_onchain(db, closed_acct); + if (closed_acct->onchain_resolved_block > 0) { + char *err; + err = update_channel_onchain_fees(ctx, db, closed_acct); + if (err) + return err; + } + } + + db_commit_transaction(db); + + return NULL; +} + struct event_info { struct chain_event *ev; struct account *acct; @@ -757,10 +786,10 @@ listpeers_done(struct command *cmd, const char *buf, info->acct->name); /* Maybe mark acct as onchain resolved */ - db_begin_transaction(db); - if (info->acct->closed_event_db_id) - maybe_mark_account_onchain(db, info->acct); - db_commit_transaction(db); + err = do_account_close_checks(cmd, info->ev, info->acct); + if (err) + plugin_err(cmd->plugin, err); + return notification_handled(cmd); } @@ -1096,10 +1125,9 @@ parse_and_log_chain_move(struct command *cmd, } /* Maybe mark acct as onchain resolved */ - db_begin_transaction(db); - if (acct->closed_event_db_id) - maybe_mark_account_onchain(db, acct); - db_commit_transaction(db); + err = do_account_close_checks(cmd, e, acct); + if (err) + plugin_err(cmd->plugin, err); return notification_handled(cmd);; } diff --git a/plugins/bkpr/recorder.c b/plugins/bkpr/recorder.c index b00d353f6..f5bbc4bbe 100644 --- a/plugins/bkpr/recorder.c +++ b/plugins/bkpr/recorder.c @@ -11,11 +11,13 @@ #include #include #include +#include #include #include #include #include + static struct chain_event *stmt2chain_event(const tal_t *ctx, struct db_stmt *stmt) { struct chain_event *e = tal(ctx, struct chain_event); @@ -387,6 +389,37 @@ bool find_txo_chain(const tal_t *ctx, return is_complete; } +struct account *find_close_account(const tal_t *ctx, + struct db *db, + struct bitcoin_txid *txid) +{ + struct db_stmt *stmt; + struct account *close_acct; + char *acct_name; + + stmt = db_prepare_v2(db, SQL("SELECT" + " a.name" + " FROM chain_events e" + " LEFT OUTER JOIN accounts a" + " ON e.account_id = a.id" + " WHERE " + " e.tag = ?" + " AND e.spending_txid = ?")); + + db_bind_text(stmt, 0, mvt_tag_str(CHANNEL_CLOSE)); + db_bind_txid(stmt, 1, txid); + db_query_prepared(stmt); + + if (db_step(stmt)) { + acct_name = db_col_strdup(stmt, stmt, "a.name"); + close_acct = find_account(ctx, db, acct_name); + } else + close_acct = NULL; + + tal_free(stmt); + return close_acct; +} + void maybe_mark_account_onchain(struct db *db, struct account *acct) { const u8 *ctx = tal(NULL, u8); @@ -1255,6 +1288,148 @@ static void insert_chain_fees_diff(struct db *db, db_exec_prepared_v2(take(stmt)); } +char *update_channel_onchain_fees(const tal_t *ctx, + struct db *db, + struct account *acct) +{ + struct chain_event *close_ev, **events; + struct amount_msat onchain_amt; + + assert(acct->onchain_resolved_block); + close_ev = find_chain_event_by_id(ctx, db, + *acct->closed_event_db_id); + events = find_chain_events_bytxid(ctx, db, + close_ev->spending_txid); + + /* Starting balance is close-ev's debit amount */ + onchain_amt = AMOUNT_MSAT(0); + for (size_t i = 0; i < tal_count(events); i++) { + struct chain_event *ev = events[i]; + + /* Ignore: + - htlc_fufill (to me) + - anchors (already exlc from output) + - to_external (if !htlc_fulfill) + */ + if (is_channel_acct(ev) + && streq("htlc_fulfill", ev->tag)) + continue; + + if (streq("anchor", ev->tag)) + continue; + + /* Ignore stuff it's paid to + * the peer's account (external), + * except for fulfilled htlcs (which originated + * in our balance) */ + if (streq(ev->acct_name, EXTERNAL_ACCT) + && !streq("htlc_fulfill", ev->tag)) + continue; + + /* anything else we count? */ + if (!amount_msat_add(&onchain_amt, onchain_amt, + ev->credit)) + return tal_fmt(ctx, "Unable to add" + "onchain + %s's credit", + ev->tag); + } + + /* Was this an 'old state' tx, where we ended up + * with more sats than we had on record? */ + if (amount_msat_greater(onchain_amt, close_ev->debit)) { + struct channel_event *ev; + struct amount_msat diff; + + if (!amount_msat_sub(&diff, onchain_amt, + close_ev->debit)) + return tal_fmt(ctx, "Unable to sub" + "close debit from onchain_amt"); + /* Add in/out journal entries for it */ + ev = new_channel_event(ctx, + tal_fmt(tmpctx, "%s", + account_entry_tag_str(PENALTY_ADJ)), + diff, + AMOUNT_MSAT(0), + AMOUNT_MSAT(0), + close_ev->currency, + NULL, 0, + close_ev->timestamp); + log_channel_event(db, acct, ev); + ev = new_channel_event(ctx, + tal_fmt(tmpctx, "%s", + account_entry_tag_str(PENALTY_ADJ)), + AMOUNT_MSAT(0), + diff, + AMOUNT_MSAT(0), + close_ev->currency, + NULL, 0, + close_ev->timestamp); + log_channel_event(db, acct, ev); + } else { + struct amount_msat fees; + if (!amount_msat_sub(&fees, close_ev->debit, + onchain_amt)) + return tal_fmt(ctx, "Unable to sub" + "onchain sum from %s", + close_ev->tag); + + insert_chain_fees_diff(db, acct->db_id, + close_ev->spending_txid, + fees, close_ev->currency, + close_ev->timestamp); + } + + return NULL; +} + +static char *is_closed_channel_txid(const tal_t *ctx, struct db *db, + struct chain_event *ev, + struct bitcoin_txid *txid, + bool *is_channel_close_tx) +{ + struct account *acct; + struct chain_event *closed; + u8 *inner_ctx = tal(NULL, u8); + + /* Figure out if this is a channel close tx */ + acct = find_account(inner_ctx, db, ev->acct_name); + assert(acct); + + /* There's a separate process for figuring out + * our onchain fees for channel closures */ + if (!acct->closed_event_db_id) { + *is_channel_close_tx = false; + tal_free(inner_ctx); + return NULL; + } + + /* is the closed utxo the same as the one + * we're trying to find fees for now */ + closed = find_chain_event_by_id(inner_ctx, db, + *acct->closed_event_db_id); + if (!closed) { + *is_channel_close_tx = false; + tal_free(inner_ctx); + return tal_fmt(ctx, "Unable to find" + " db record (chain_evt)" + " with id %"PRIu64, + *acct->closed_event_db_id); + } + + if (!closed->spending_txid) { + *is_channel_close_tx = false; + tal_free(inner_ctx); + return tal_fmt(ctx, "Marked a closing" + " event that's not" + " actually a spend"); + } + + *is_channel_close_tx = + bitcoin_txid_eq(txid, closed->spending_txid); + tal_free(inner_ctx); + return NULL; +} + char *maybe_update_onchain_fees(const tal_t *ctx, struct db *db, struct bitcoin_txid *txid) { @@ -1278,40 +1453,19 @@ char *maybe_update_onchain_fees(const tal_t *ctx, struct db *db, goto finished; for (size_t i = 0; i < tal_count(events); i++) { + bool is_channel_close_tx; + err = is_closed_channel_txid(ctx, db, + events[i], txid, + &is_channel_close_tx); + + if (err) + goto finished; + + /* We skip channel close txs here! */ + if (is_channel_close_tx) + goto finished; + if (events[i]->spending_txid) { - struct account *acct; - /* Figure out if this is a channel close - * that we're not the opener for */ - acct = find_account(inner_ctx, db, - events[i]->acct_name); - assert(acct); - - /* If any of the spending_txid accounts are - * close accounts and we're not the opener, - * we end things */ - if (acct->closed_event_db_id && !acct->we_opened) { - struct chain_event *closed; - /* is the closed utxo the same as the one - * we're trying to find fees for now */ - closed = find_chain_event_by_id(inner_ctx, - db, *acct->closed_event_db_id); - if (!closed) { - err = tal_fmt(ctx, "Unable to find" - " db record (chain_evt)" - " with id %"PRIu64, - *acct->closed_event_db_id); - goto finished; - } - if (!closed->spending_txid) { - err = tal_fmt(ctx, "Marked a closing" - " event that's not" - " actually a spend"); - goto finished; - } - - if (bitcoin_txid_eq(txid, closed->spending_txid)) - goto finished; - } if (!amount_msat_add(&withdraw_msat, withdraw_msat, events[i]->debit)) { err = tal_fmt(ctx, "Overflow adding withdrawal debits for" @@ -1366,9 +1520,6 @@ char *maybe_update_onchain_fees(const tal_t *ctx, struct db *db, if (amount_msat_less(withdraw_msat, deposit_msat)) goto finished; - /* At this point, we have no way to know we've gotten all the data. - * But that's what the 'onchain_resolved_block' marker on - * accounts is for */ if (!amount_msat_sub(&fees_msat, withdraw_msat, deposit_msat)) { err = tal_fmt(ctx, "Err subtracting withdraw %s from deposit %s" " for txid %s", diff --git a/plugins/bkpr/recorder.h b/plugins/bkpr/recorder.h index 4989d5f12..5eb623a6c 100644 --- a/plugins/bkpr/recorder.h +++ b/plugins/bkpr/recorder.h @@ -106,6 +106,12 @@ struct account *find_account(const tal_t *ctx, struct db *db, const char *name); +/* Find the account that was closed by this txid. + * Returns NULL if none */ +struct account *find_close_account(const tal_t *ctx, + struct db *db, + struct bitcoin_txid *txid); + /* Some events update account information */ void maybe_update_account(struct db *db, struct account *acct, @@ -118,6 +124,11 @@ char *maybe_update_onchain_fees(const tal_t *ctx, struct db *db, struct bitcoin_txid *txid); +/* We calculate onchain fees for channel closes a bit different */ +char *update_channel_onchain_fees(const tal_t *ctx, + struct db *db, + struct account *acct); + /* Have all the outputs for this account's close tx * been resolved onchain? If so, update the account with the * highest blockheight that has a resolving tx in it. diff --git a/plugins/bkpr/test/run-bkpr_db.c b/plugins/bkpr/test/run-bkpr_db.c index 7bf7baad7..cab5ff9fb 100644 --- a/plugins/bkpr/test/run-bkpr_db.c +++ b/plugins/bkpr/test/run-bkpr_db.c @@ -29,11 +29,16 @@ void db_fatal(const char *fmt, ...) #include #include #include +#include +#include #include #include #include /* AUTOGENERATED MOCKS START */ +/* Generated stub for account_entry_tag_str */ +const char *account_entry_tag_str(enum account_entry_tag tag UNNEEDED) +{ fprintf(stderr, "account_entry_tag_str called!\n"); abort(); } /* Generated stub for daemon_maybe_debug */ void daemon_maybe_debug(char *argv[]) { fprintf(stderr, "daemon_maybe_debug called!\n"); abort(); } @@ -172,6 +177,20 @@ enum htlc_state last_fee_state(enum side opener UNNEEDED) /* Generated stub for log_level_name */ const char *log_level_name(enum log_level level UNNEEDED) { fprintf(stderr, "log_level_name called!\n"); abort(); } +/* Generated stub for mvt_tag_str */ +const char *mvt_tag_str(enum mvt_tag tag UNNEEDED) +{ fprintf(stderr, "mvt_tag_str called!\n"); abort(); } +/* Generated stub for new_channel_event */ +struct channel_event *new_channel_event(const tal_t *ctx UNNEEDED, + const char *tag UNNEEDED, + struct amount_msat credit UNNEEDED, + struct amount_msat debit UNNEEDED, + struct amount_msat fees UNNEEDED, + const char *currency UNNEEDED, + struct sha256 *payment_id STEALS UNNEEDED, + u32 part_id UNNEEDED, + u64 timestamp UNNEEDED) +{ fprintf(stderr, "new_channel_event called!\n"); abort(); } /* Generated stub for toks_alloc */ jsmntok_t *toks_alloc(const tal_t *ctx UNNEEDED) { fprintf(stderr, "toks_alloc called!\n"); abort(); } diff --git a/plugins/bkpr/test/run-recorder.c b/plugins/bkpr/test/run-recorder.c index 1693f089e..8480d5288 100644 --- a/plugins/bkpr/test/run-recorder.c +++ b/plugins/bkpr/test/run-recorder.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,9 @@ void db_fatal(const char *fmt, ...) /* AUTOGENERATED MOCKS START */ +/* Generated stub for account_entry_tag_str */ +const char *account_entry_tag_str(enum account_entry_tag tag UNNEEDED) +{ fprintf(stderr, "account_entry_tag_str called!\n"); abort(); } /* Generated stub for daemon_maybe_debug */ void daemon_maybe_debug(char *argv[]) { fprintf(stderr, "daemon_maybe_debug called!\n"); abort(); } @@ -179,6 +183,20 @@ enum htlc_state last_fee_state(enum side opener UNNEEDED) /* Generated stub for log_level_name */ const char *log_level_name(enum log_level level UNNEEDED) { fprintf(stderr, "log_level_name called!\n"); abort(); } +/* Generated stub for mvt_tag_str */ +const char *mvt_tag_str(enum mvt_tag tag UNNEEDED) +{ fprintf(stderr, "mvt_tag_str called!\n"); abort(); } +/* Generated stub for new_channel_event */ +struct channel_event *new_channel_event(const tal_t *ctx UNNEEDED, + const char *tag UNNEEDED, + struct amount_msat credit UNNEEDED, + struct amount_msat debit UNNEEDED, + struct amount_msat fees UNNEEDED, + const char *currency UNNEEDED, + struct sha256 *payment_id STEALS UNNEEDED, + u32 part_id UNNEEDED, + u64 timestamp UNNEEDED) +{ fprintf(stderr, "new_channel_event called!\n"); abort(); } /* Generated stub for toks_alloc */ jsmntok_t *toks_alloc(const tal_t *ctx UNNEEDED) { fprintf(stderr, "toks_alloc called!\n"); abort(); } @@ -322,6 +340,8 @@ static struct chain_event *make_chain_event(const tal_t *ctx, char *tag, struct amount_msat credit, struct amount_msat debit, + struct amount_msat output_val, + u32 blockheight, char outpoint_char, u32 outnum, /* Note that '*' is magic */ @@ -336,10 +356,10 @@ static struct chain_event *make_chain_event(const tal_t *ctx, ev->origin_acct = NULL; ev->credit = credit; ev->debit = debit; - ev->output_value = AMOUNT_MSAT(1000); + ev->output_value = output_val; ev->currency = "btc"; ev->timestamp = 1919191; - ev->blockheight = 1919191; + ev->blockheight = blockheight; memset(&ev->outpoint.txid, outpoint_char, sizeof(struct bitcoin_txid)); ev->outpoint.n = outnum; @@ -362,6 +382,7 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) struct account *wal_acct, *ext_acct; struct bitcoin_txid txid; struct onchain_fee **ofs; + u32 blockheight = 100000; memset(&node_id, 2, sizeof(struct node_id)); memset(&peer_id, 3, sizeof(struct node_id)); @@ -387,6 +408,8 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "withdrawal", AMOUNT_MSAT(0), AMOUNT_MSAT(1000), + AMOUNT_MSAT(1000), + blockheight, 'X', 0, '1')); maybe_update_onchain_fees(ctx, db, &txid); @@ -394,6 +417,8 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "deposit", AMOUNT_MSAT(200), AMOUNT_MSAT(0), + AMOUNT_MSAT(200), + blockheight, '1', 1, '*')); maybe_update_onchain_fees(ctx, db, &txid); @@ -401,6 +426,8 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "deposit", AMOUNT_MSAT(700), AMOUNT_MSAT(0), + AMOUNT_MSAT(700), + blockheight, '1', 0, '*')); maybe_update_onchain_fees(ctx, db, &txid); db_commit_transaction(db); @@ -432,9 +459,16 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) struct account *acct, *wal_acct, *ext_acct; struct onchain_fee **ofs, **ofs1; struct bitcoin_txid txid; + struct chain_event *ev; + enum mvt_tag *tags; + u32 close_output_count; + u32 blockheight = 100000; + char *err; memset(&node_id, 2, sizeof(struct node_id)); memset(&peer_id, 3, sizeof(struct node_id)); + /* to_us, to_them, 1 htlc, 2 anchors */ + close_output_count = 5; wal_acct = new_account(ctx, tal_fmt(ctx, "wallet"), &peer_id); ext_acct = new_account(ctx, tal_fmt(ctx, "external"), &peer_id); @@ -450,47 +484,88 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) /* Close a channel */ /* tag utxo_id vout txid debits credits acct_id * close XXXX 0 1111 1000 wallet - * delay 1111 1 2222 200 chan-1 - * htlc_tx 1111 2 3333 600 chan-1 - * to_them 1111 3 external - * to_wall 3333 0 500 wallet - * to_wall 2222 0 150 wallet + * delay 1111 1 200 chan-1 + * anchor 1111 0 30 chan-1 + * anchor 1111 4 30 external + * to_wall 1111 1 2222 200 chan-1 + * htlc_tim1111 2 600 chan-1 + * htlc_tim1111 2 3333 600 chan-1 + * to_them 1111 3 300 external + * deposit 2222 0 150 chan-1 + * htlc_tx 3333 0 450 chan-1 + * to_wall 3333 0 4444 450 chan-1 + * deposit 4444 0 350 wallet */ + tags = tal_arr(ctx, enum mvt_tag, 1); db_begin_transaction(db); - log_chain_event(db, wal_acct, - make_chain_event(ctx, "close", - AMOUNT_MSAT(0), - AMOUNT_MSAT(1000), - 'X', 0, '1')); + ev = make_chain_event(ctx, "channel_open", + AMOUNT_MSAT(1000), + AMOUNT_MSAT(0), + AMOUNT_MSAT(1660), + blockheight, + 'X', 0, '*'); + log_chain_event(db, acct, ev); + tags[0] = CHANNEL_OPEN; + maybe_update_account(db, acct, ev, tags, 0); + + ev = make_chain_event(ctx, "channel_close", + AMOUNT_MSAT(0), + AMOUNT_MSAT(1000), + AMOUNT_MSAT(1660), + blockheight, + 'X', 0, '1'); + log_chain_event(db, acct, ev); + + /* Update the account to have the right info! */ + tags[0] = CHANNEL_CLOSE; + maybe_update_account(db, acct, ev, tags, close_output_count); log_chain_event(db, acct, make_chain_event(ctx, "delayed_to_us", AMOUNT_MSAT(200), AMOUNT_MSAT(0), + AMOUNT_MSAT(200), + blockheight, '1', 1, '*')); memset(&txid, '1', sizeof(struct bitcoin_txid)); maybe_update_onchain_fees(ctx, db, &txid); - /* Should be one fees now */ + log_chain_event(db, wal_acct, + make_chain_event(ctx, "anchor", + AMOUNT_MSAT(30), + AMOUNT_MSAT(0), + AMOUNT_MSAT(30), + blockheight, + '1', 0, '*')); + log_chain_event(db, ext_acct, + make_chain_event(ctx, "anchor", + AMOUNT_MSAT(30), + AMOUNT_MSAT(0), + AMOUNT_MSAT(30), + blockheight, + '1', 4, '*')); + memset(&txid, '1', sizeof(struct bitcoin_txid)); + maybe_update_onchain_fees(ctx, db, &txid); + + /* Should be no fees yet */ ofs = list_chain_fees(ctx, db); CHECK_MSG(!db_err, db_err); - - CHECK(tal_count(ofs) == 1); - CHECK(ofs[0]->acct_db_id == acct->db_id); - CHECK(amount_msat_eq(ofs[0]->credit, AMOUNT_MSAT(800))); - CHECK(amount_msat_zero(ofs[0]->debit)); - CHECK(ofs[0]->update_count == 1); + CHECK(tal_count(ofs) == 0); log_chain_event(db, acct, - make_chain_event(ctx, "htlc_tx", + make_chain_event(ctx, "htlc_timeout", AMOUNT_MSAT(600), AMOUNT_MSAT(0), + AMOUNT_MSAT(600), + blockheight, '1', 2, '*')); log_chain_event(db, ext_acct, make_chain_event(ctx, "to_them", + AMOUNT_MSAT(300), AMOUNT_MSAT(0), - AMOUNT_MSAT(0), + AMOUNT_MSAT(300), + blockheight, '1', 3, '*')); memset(&txid, '1', sizeof(struct bitcoin_txid)); @@ -501,22 +576,90 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) /* txid 2222 */ db_begin_transaction(db); log_chain_event(db, acct, - make_chain_event(ctx, "withdraw", + make_chain_event(ctx, "to_wallet", AMOUNT_MSAT(0), AMOUNT_MSAT(200), + AMOUNT_MSAT(200), + blockheight + 1, '1', 1, '2')); log_chain_event(db, wal_acct, - make_chain_event(ctx, "to_wallet", + make_chain_event(ctx, "deposit", AMOUNT_MSAT(150), AMOUNT_MSAT(0), + AMOUNT_MSAT(150), + blockheight + 1, '2', 0, '*')); memset(&txid, '2', sizeof(struct bitcoin_txid)); maybe_update_onchain_fees(ctx, db, &txid); + CHECK(acct->onchain_resolved_block == 0); + + maybe_mark_account_onchain(db, acct); + CHECK(acct->onchain_resolved_block == 0); db_commit_transaction(db); CHECK_MSG(!db_err, db_err); - /* Expect: 3 onchain fee records, all for chan-1 */ + /* Expect: 1 onchain fee records, all for chan-1 */ + db_begin_transaction(db); + ofs = list_chain_fees(ctx, db); + ofs1 = account_onchain_fees(ctx, db, acct); + db_commit_transaction(db); + CHECK_MSG(!db_err, db_err); + + CHECK(tal_count(ofs) == tal_count(ofs1)); + CHECK(tal_count(ofs) == 1); + + /* txid 4444 */ + db_begin_transaction(db); + log_chain_event(db, wal_acct, + make_chain_event(ctx, "deposit", + AMOUNT_MSAT(350), + AMOUNT_MSAT(0), + AMOUNT_MSAT(350), + blockheight + 2, + '4', 0, '*')); + + memset(&txid, '4', sizeof(struct bitcoin_txid)); + maybe_update_onchain_fees(ctx, db, &txid); + + /* txid 3333 */ + log_chain_event(db, acct, + make_chain_event(ctx, "htlc_timeout", + AMOUNT_MSAT(0), + AMOUNT_MSAT(600), + AMOUNT_MSAT(600), + blockheight + 2, + '1', 2, '3')); + + maybe_mark_account_onchain(db, acct); + CHECK(acct->onchain_resolved_block == 0); + + log_chain_event(db, acct, + make_chain_event(ctx, "htlc_tx", + AMOUNT_MSAT(450), + AMOUNT_MSAT(0), + AMOUNT_MSAT(450), + blockheight + 2, + '3', 0, '*')); + + memset(&txid, '3', sizeof(struct bitcoin_txid)); + maybe_update_onchain_fees(ctx, db, &txid); + + log_chain_event(db, acct, + make_chain_event(ctx, "to_wallet", + AMOUNT_MSAT(0), + AMOUNT_MSAT(450), + AMOUNT_MSAT(450), + blockheight + 2, + '3', 0, '4')); + + memset(&txid, '4', sizeof(struct bitcoin_txid)); + maybe_update_onchain_fees(ctx, db, &txid); + + db_commit_transaction(db); + CHECK_MSG(!db_err, db_err); + + /* Expect: onchain fee records for tx except channel close */ db_begin_transaction(db); ofs = list_chain_fees(ctx, db); ofs1 = account_onchain_fees(ctx, db, acct); @@ -526,57 +669,33 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) CHECK(tal_count(ofs) == tal_count(ofs1)); CHECK(tal_count(ofs) == 3); - /* txid 3333 */ + /* Now we update the channel's onchain fees */ + CHECK(acct->onchain_resolved_block == 0); db_begin_transaction(db); - log_chain_event(db, acct, - make_chain_event(ctx, "htlc_tx", - AMOUNT_MSAT(0), - AMOUNT_MSAT(600), - '1', 2, '3')); - - log_chain_event(db, wal_acct, - make_chain_event(ctx, "to_wallet", - AMOUNT_MSAT(500), - AMOUNT_MSAT(0), - '3', 0, '*')); - memset(&txid, '3', sizeof(struct bitcoin_txid)); - maybe_update_onchain_fees(ctx, db, &txid); + maybe_mark_account_onchain(db, acct); + CHECK(acct->onchain_resolved_block == blockheight + 2); + err = update_channel_onchain_fees(ctx, db, acct); + CHECK_MSG(!err, err); + ofs = account_onchain_fees(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); - - /* Expect: 3 onchain fee records, all for chan-1 */ - db_begin_transaction(db); - ofs = list_chain_fees(ctx, db); - ofs1 = account_onchain_fees(ctx, db, acct); - db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); - - CHECK(tal_count(ofs) == tal_count(ofs1)); - CHECK(tal_count(ofs) == 4); /* Expect: fees as follows * - * chan-1, 1111, 800,-600 - * chan-1, 3333, 100 + * chan-1, 1111, 200 (anchor outs ignored) * chan-1, 2222, 50 + * chan-1, 3333, 150 + * chan-1, 4444, 100 */ + CHECK(tal_count(ofs) == 4); for (size_t i = 0; i < tal_count(ofs); i++) { CHECK(ofs[i]->acct_db_id == acct->db_id); CHECK(streq(ofs[i]->currency, "btc")); memset(&txid, '1', sizeof(struct bitcoin_txid)); if (bitcoin_txid_eq(&txid, &ofs[i]->txid)) { - CHECK(ofs[i]->update_count == 1 - || ofs[i]->update_count == 2); - - if (ofs[i]->update_count == 1) { - CHECK(amount_msat_eq(ofs[i]->credit, AMOUNT_MSAT(800))); - CHECK(amount_msat_zero(ofs[i]->debit)); - - } else { - CHECK(amount_msat_eq(ofs[i]->debit, AMOUNT_MSAT(600))); - CHECK(amount_msat_zero(ofs[i]->credit)); - } + CHECK(ofs[i]->update_count == 1); + CHECK(amount_msat_eq(ofs[i]->credit, AMOUNT_MSAT(200))); + CHECK(amount_msat_zero(ofs[i]->debit)); continue; } memset(&txid, '2', sizeof(struct bitcoin_txid)); @@ -587,6 +706,13 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) continue; } memset(&txid, '3', sizeof(struct bitcoin_txid)); + if (bitcoin_txid_eq(&txid, &ofs[i]->txid)) { + CHECK(ofs[i]->update_count == 1); + CHECK(amount_msat_eq(ofs[i]->credit, AMOUNT_MSAT(150))); + CHECK(amount_msat_zero(ofs[i]->debit)); + continue; + } + memset(&txid, '4', sizeof(struct bitcoin_txid)); if (bitcoin_txid_eq(&txid, &ofs[i]->txid)) { CHECK(ofs[i]->update_count == 1); CHECK(amount_msat_eq(ofs[i]->credit, AMOUNT_MSAT(100))); @@ -607,6 +733,7 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) struct account *acct, *acct2, *wal_acct, *ext_acct; struct bitcoin_txid txid; struct onchain_fee **ofs; + u32 blockheight = 100000; memset(&node_id, 2, sizeof(struct node_id)); memset(&peer_id, 3, sizeof(struct node_id)); @@ -641,17 +768,23 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "withdrawal", AMOUNT_MSAT(0), AMOUNT_MSAT(1000), + AMOUNT_MSAT(1000), + blockheight, 'X', 0, 'A')); log_chain_event(db, wal_acct, make_chain_event(ctx, "withdrawal", AMOUNT_MSAT(0), AMOUNT_MSAT(3001), + AMOUNT_MSAT(3001), + blockheight, 'Y', 0, 'A')); log_chain_event(db, acct, make_chain_event(ctx, "deposit", AMOUNT_MSAT(500), AMOUNT_MSAT(0), + AMOUNT_MSAT(500), + blockheight, 'A', 0, '*')); maybe_update_onchain_fees(ctx, db, &txid); @@ -659,6 +792,8 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "deposit", AMOUNT_MSAT(1000), AMOUNT_MSAT(0), + AMOUNT_MSAT(1000), + blockheight, 'A', 1, '*')); maybe_update_onchain_fees(ctx, db, &txid); @@ -666,6 +801,8 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "deposit", AMOUNT_MSAT(2200), AMOUNT_MSAT(0), + AMOUNT_MSAT(2200), + blockheight, 'A', 2, '*')); maybe_update_onchain_fees(ctx, db, &txid); @@ -943,6 +1080,8 @@ static bool test_account_balances(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "one", AMOUNT_MSAT(1000), AMOUNT_MSAT(0), + AMOUNT_MSAT(1000), + 1019, 'A', 1, '*')); /* -999btc */ @@ -950,6 +1089,8 @@ static bool test_account_balances(const tal_t *ctx, struct plugin *p) make_chain_event(ctx, "two", AMOUNT_MSAT(0), AMOUNT_MSAT(999), + AMOUNT_MSAT(999), + 1020, 'A', 2, '*')); /* -440btc */ @@ -967,7 +1108,9 @@ static bool test_account_balances(const tal_t *ctx, struct plugin *p) 'D')); /* +5000chf */ - ev1 = make_chain_event(ctx, "two", AMOUNT_MSAT(5000), AMOUNT_MSAT(0), + ev1 = make_chain_event(ctx, "two", + AMOUNT_MSAT(5000), AMOUNT_MSAT(0), + AMOUNT_MSAT(5000), 1999, 'A', 3, '*'); ev1->currency = "chf"; log_chain_event(db, acct, ev1); @@ -993,6 +1136,7 @@ static bool test_account_balances(const tal_t *ctx, struct plugin *p) /* -5001chf */ ev1 = make_chain_event(ctx, "two", AMOUNT_MSAT(0), AMOUNT_MSAT(5001), + AMOUNT_MSAT(5001), 2020, 'A', 4, '*'); ev1->currency = "chf"; log_chain_event(db, acct, ev1); diff --git a/tests/test_closing.py b/tests/test_closing.py index 3b0606667..e58b23e08 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -479,6 +479,107 @@ def test_closing_negotiation_step_700sat(node_factory, bitcoind, chainparams): closing_negotiation_step(node_factory, bitcoind, chainparams, opts) +# FIXME: move to bookkeeper tests +@pytest.mark.developer("dev-ignore-htlcs") +def test_closing_trimmed_htlcs(node_factory, bitcoind, executor): + l1, l2 = node_factory.line_graph(2) + + # give l2 an output!? + l1.pay(l2, 11000000) + + l1.rpc.dev_ignore_htlcs(id=l2.info['id'], ignore=True) + # This will get stuck due to l3 ignoring htlcs + executor.submit(l2.pay, l1, 100001) + l1.daemon.wait_for_log('their htlc 0 dev_ignore_htlcs') + + l1.rpc.dev_fail(l2.info['id']) + l1.wait_for_channel_onchain(l2.info['id']) + + bitcoind.generate_block(1) + l1.daemon.wait_for_log(' to ONCHAIN') + l2.daemon.wait_for_log(' to ONCHAIN') + + bitcoind.generate_block(5) + l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET') + bitcoind.generate_block(20) + l1.daemon.wait_for_log('All outputs resolved') + + def _find_tags(evs, tag): + return [e for e in evs if e['tag'] == tag] + + def _find_first_tag(evs, tag): + ev = _find_tags(evs, tag) + assert len(ev) > 0 + return ev[0] + + evs = l1.rpc.listaccountevents()['events'] + close = _find_first_tag(evs, 'channel_close') + delayed_to = _find_first_tag(evs, 'delayed_to_us') + + # find the chain fee entry for the channel close + fees = _find_tags(evs, 'onchain_fee') + close_fee = [e for e in fees if e['txid'] == close['txid']] + assert len(close_fee) == 1 + assert Millisatoshi(close_fee[0]['credit']) + Millisatoshi(delayed_to['credit']) == Millisatoshi(close['debit']) + + # l2's fees should equal the trimmed htlc out + evs = l2.rpc.listaccountevents()['events'] + close = _find_first_tag(evs, 'channel_close') + deposit = _find_first_tag(evs, 'deposit') + fees = _find_tags(evs, 'onchain_fee') + close_fee = [e for e in fees if e['txid'] == close['txid']] + assert len(close_fee) == 1 + # sent htlc was too small, we lose it, rounded up to nearest sat + assert close_fee[0]['credit'] == '101000msat' + assert Millisatoshi(close_fee[0]['credit']) + Millisatoshi(deposit['credit']) == Millisatoshi(close['debit']) + + +# FIXME: move to bookkeeper tests +def test_closing_subsat_htlcs(node_factory, bitcoind, chainparams): + """Test closing balances when HTLCs are: sub 1-satoshi""" + l1, l2 = node_factory.line_graph(2) + + l1.pay(l2, 111) + l1.pay(l2, 222) + l1.pay(l2, 4000000) + + l2.stop() + l1.rpc.close(l2.info['id'], 1) + bitcoind.generate_block(5) + l2.start() + sync_blockheight(bitcoind, [l1]) + l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET') + bitcoind.generate_block(80) + + def _find_tags(evs, tag): + return [e for e in evs if e['tag'] == tag] + + def _find_first_tag(evs, tag): + ev = _find_tags(evs, tag) + assert len(ev) > 0 + return ev[0] + + sync_blockheight(bitcoind, [l1, l2]) + evs = l1.rpc.listaccountevents()['events'] + # check that closing equals onchain deposits + fees + close = _find_first_tag(evs, 'channel_close') + delayed_to = _find_first_tag(evs, 'delayed_to_us') + fees = _find_tags(evs, 'onchain_fee') + close_fee = [e for e in fees if e['txid'] == close['txid']] + assert len(close_fee) == 1 + assert Millisatoshi(close_fee[0]['credit']) + Millisatoshi(delayed_to['credit']) == Millisatoshi(close['debit']) + + evs = l2.rpc.listaccountevents()['events'] + close = _find_first_tag(evs, 'channel_close') + deposit = _find_first_tag(evs, 'deposit') + fees = _find_tags(evs, 'onchain_fee') + close_fee = [e for e in fees if e['txid'] == close['txid']] + assert len(close_fee) == 1 + # too small to fit, we lose them as miner fees + assert close_fee[0]['credit'] == '333msat' + assert Millisatoshi(close_fee[0]['credit']) + Millisatoshi(deposit['credit']) == Millisatoshi(close['debit']) + + @pytest.mark.developer("needs dev-disable-commit-after") def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams): """Test penalty transaction with an incoming HTLC"""