Skip to content

Commit 1b4672b

Browse files
committed
wallet: fix crash on listtransactions.
We removed the (experimental-only!) annotation output in 611795b but we still loaded them from the db. Turns out that we were putting bogus annotations into the db, and accessing out of range when loading them. Consider the following db entry in transaction_annotations: ``` CREATE TABLE transaction_annotations ( txid BLOB, idx INTEGER, location INTEGER, type INTEGER, channel INTEGER REFERENCES channels(id), UNIQUE(txid, idx)); ... INSERT INTO transaction_annotations VALUES(X'19706f9af2875508a06c7db1754ef7ecb3da745ead005992e626441e4e83465f',18,1,129,53699); ``` Here is the corresponding entry in txs: ``` INSERT INTO transactions VALUES(X'19706f9af2875508a06c7db1754ef7ecb3da745ead005992e626441e4e83465f',710327,966,X'02000000000101f2add69112a1d557317826120e1f4ea3bc1cbe4674d720325695b26ecfe8355d120000000000000000013634000000000000160014dca21f104359bbb81e88ed7da985549f2cd0cbc30347304402201cdc854b76c4c7523e3ca09f38a81539d3b2f7fbd9a0de6ae10b7ceaa65ed9d402205a1770058cd1ef081c77c2fe957c07a334cb3a11bc0cc502834a29c59424fe010120589da1f809d955c7af150bf53123c27ffc0a741489b5291f6be811189863ec838576a9142f188d0d973c4ad1865a619d3748340b30746e088763ac672103b7bbcd592197ba6501e7176aabd3f908d94b126ae82ab1e7a4c58f5a789782e57c820120876475527c21029bcf62114eb36758fcb1aead7e67b6f707d32f34e67816894d5211ac9f2d6ce752ae67a9144483a115219ba65c63a3844be8445f739703bea988ac686800000000',NULL,NULL); ``` The annotation refers to output 18 of the tx, but it only has one output! However, decoding the tx shows that it spent output 18 of a previous tx, so that's probably where the `18` came from. Remove this logic: we can remove the remaining (clearly broken!) annotation adding code in another cleanup commit. Signed-off-by: Rusty Russell <[email protected]>
1 parent d1cf88c commit 1b4672b

File tree

2 files changed

+16
-77
lines changed

2 files changed

+16
-77
lines changed

wallet/wallet.c

Lines changed: 16 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -4809,11 +4809,7 @@ bool wallet_forward_delete(struct wallet *w,
48094809
struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t *ctx)
48104810
{
48114811
struct db_stmt *stmt;
4812-
struct wallet_transaction *cur = NULL, *txs = tal_arr(ctx, struct wallet_transaction, 0);
4813-
struct bitcoin_txid last;
4814-
4815-
/* Make sure we can check for changing txids */
4816-
memset(&last, 0, sizeof(last));
4812+
struct wallet_transaction *txs = tal_arr(ctx, struct wallet_transaction, 0);
48174813

48184814
stmt = db_prepare_v2(
48194815
w->db,
@@ -4822,82 +4818,31 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
48224818
", t.rawtx"
48234819
", t.blockheight"
48244820
", t.txindex"
4825-
", a.location"
4826-
", a.idx as ann_idx"
4827-
", a.type as annotation_type"
4828-
", c.scid"
48294821
" FROM"
48304822
" transactions t LEFT JOIN"
4831-
" transaction_annotations a ON (a.txid = t.id) LEFT JOIN"
4832-
" channels c ON (a.channel = c.id) "
4823+
" channels c ON (t.channel_id = c.id) "
48334824
"ORDER BY t.blockheight, t.txindex ASC"));
48344825
db_query_prepared(stmt);
48354826

48364827
while (db_step(stmt)) {
4837-
struct bitcoin_txid curtxid;
4838-
db_col_txid(stmt, "t.id", &curtxid);
4839-
4840-
/* If this is a new entry, allocate it in the array and set
4841-
* the common fields (all fields from the transactions table. */
4842-
if (!bitcoin_txid_eq(&last, &curtxid)) {
4843-
last = curtxid;
4844-
tal_resize(&txs, tal_count(txs) + 1);
4845-
cur = &txs[tal_count(txs) - 1];
4846-
db_col_txid(stmt, "t.id", &cur->id);
4847-
cur->tx = db_col_tx(txs, stmt, "t.rawtx");
4848-
cur->rawtx = db_col_arr(txs, stmt, "t.rawtx", u8);
4849-
/* TX may be unconfirmed. */
4850-
if (!db_col_is_null(stmt, "t.blockheight")) {
4851-
cur->blockheight
4852-
= db_col_int(stmt, "t.blockheight");
4853-
if (!db_col_is_null(stmt, "t.txindex")) {
4854-
cur->txindex
4855-
= db_col_int(stmt, "t.txindex");
4856-
} else {
4857-
cur->txindex = 0;
4858-
}
4828+
struct wallet_transaction *cur;
4829+
4830+
tal_resize(&txs, tal_count(txs) + 1);
4831+
cur = &txs[tal_count(txs) - 1];
4832+
db_col_txid(stmt, "t.id", &cur->id);
4833+
cur->tx = db_col_tx(txs, stmt, "t.rawtx");
4834+
cur->rawtx = db_col_arr(txs, stmt, "t.rawtx", u8);
4835+
if (!db_col_is_null(stmt, "t.blockheight")) {
4836+
cur->blockheight = db_col_int(stmt, "t.blockheight");
4837+
if (!db_col_is_null(stmt, "t.txindex")) {
4838+
cur->txindex = db_col_int(stmt, "t.txindex");
48594839
} else {
4860-
db_col_ignore(stmt, "t.txindex");
4861-
cur->blockheight = 0;
48624840
cur->txindex = 0;
48634841
}
4864-
cur->output_annotations = tal_arrz(txs, struct tx_annotation, cur->tx->wtx->num_outputs);
4865-
cur->input_annotations = tal_arrz(txs, struct tx_annotation, cur->tx->wtx->num_inputs);
4866-
}
4867-
4868-
/* This should always be set by the above if-statement,
4869-
* otherwise we have a txid of all 0x00 bytes... */
4870-
assert(cur != NULL);
4871-
4872-
/* Check if we have any annotations. If there are none the
4873-
* fields are all set to null */
4874-
if (!db_col_is_null(stmt, "a.location")) {
4875-
enum wallet_tx_annotation_type loc
4876-
= db_col_int(stmt, "a.location");
4877-
int idx = db_col_int(stmt, "ann_idx");
4878-
struct tx_annotation *ann;
4879-
4880-
/* Select annotation from array to fill in. */
4881-
switch (loc) {
4882-
case OUTPUT_ANNOTATION:
4883-
ann = &cur->output_annotations[idx];
4884-
goto got_ann;
4885-
case INPUT_ANNOTATION:
4886-
ann = &cur->input_annotations[idx];
4887-
goto got_ann;
4888-
}
4889-
fatal("Transaction annotations are only available for inputs and outputs. Value %d", loc);
4890-
4891-
got_ann:
4892-
ann->type = db_col_int(stmt, "annotation_type");
4893-
if (!db_col_is_null(stmt, "c.scid"))
4894-
db_col_short_channel_id(stmt, "c.scid", &ann->channel);
4895-
else
4896-
ann->channel.u64 = 0;
48974842
} else {
4898-
db_col_ignore(stmt, "ann_idx");
4899-
db_col_ignore(stmt, "annotation_type");
4900-
db_col_ignore(stmt, "c.scid");
4843+
db_col_ignore(stmt, "t.txindex");
4844+
cur->blockheight = 0;
4845+
cur->txindex = 0;
49014846
}
49024847
}
49034848
tal_free(stmt);

wallet/wallet.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,12 +406,6 @@ struct wallet_transaction {
406406

407407
/* Fully parsed transaction */
408408
const struct bitcoin_tx *tx;
409-
410-
/* tal_arr containing the annotation types, if any, for the respective
411-
* inputs and outputs. 0 if there are no annotations for the
412-
* element. */
413-
struct tx_annotation *input_annotations;
414-
struct tx_annotation *output_annotations;
415409
};
416410

417411
/**

0 commit comments

Comments
 (0)