From bf1fb7ea79fc5999a87a25092d709710321165c9 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Thu, 14 Nov 2019 13:33:11 +0100 Subject: [PATCH 1/3] wallet: fix accessing blockheight of unconfirmed transaction --- wallet/wallet.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/wallet/wallet.c b/wallet/wallet.c index 054985fb53f0..0484e660264b 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -3445,7 +3445,7 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t " transaction_annotations a ON (a.txid = t.id) LEFT JOIN" " channels c ON (a.channel = c.id) LEFT JOIN" " channels c2 ON (t.channel_id = c2.id) " - "ORDER BY blockheight, txindex ASC")); + "ORDER BY t.blockheight, t.txindex ASC")); db_query_prepared(stmt); for (count = 0; db_step(stmt); count++) { @@ -3463,8 +3463,16 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t cur->tx = db_column_tx(txs, stmt, 1); cur->rawtx = tal_dup_arr(txs, u8, db_column_blob(stmt, 1), db_column_bytes(stmt, 1), 0); - cur->blockheight = db_column_int(stmt, 2); - cur->txindex = db_column_int(stmt, 3); + /* TX may be unconfirmed. */ + if (!db_column_is_null(stmt, 2) || !db_column_is_null(stmt, 3)) { + /* assert incomplete information */ + assert(!db_column_is_null(stmt, 2) && !db_column_is_null(stmt, 3)); + cur->blockheight = db_column_int(stmt, 2); + cur->txindex = db_column_int(stmt, 3); + } else { + cur->blockheight = 0; + cur->txindex = 0; + } if (!db_column_is_null(stmt, 4)) cur->annotation.type = db_column_u64(stmt, 4); else From 0c9665010341ebb3548c294828e1662bfad05139 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Thu, 14 Nov 2019 14:14:29 +0100 Subject: [PATCH 2/3] wallet: fix skipping tx dups memory corruption Changelog-Fixed: #3231 listtransactions crash --- wallet/wallet.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/wallet/wallet.c b/wallet/wallet.c index 0484e660264b..860167f44c21 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -3453,12 +3453,11 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t db_column_txid(stmt, 0, &curtxid); /* If this is a new entry, allocate it in the array and set - * the common fields (all fields from the transactions - * table. */ + * the common fields (all fields from the transactions table. */ if (!bitcoin_txid_eq(&last, &curtxid)) { last = curtxid; - tal_resize(&txs, count + 1); - cur = &txs[count]; + tal_resize(&txs, tal_count(txs) + 1); + cur = &txs[tal_count(txs) - 1]; db_column_txid(stmt, 0, &cur->id); cur->tx = db_column_tx(txs, stmt, 1); cur->rawtx = tal_dup_arr(txs, u8, db_column_blob(stmt, 1), From b4c3be9abcb759bf9ad2bd3037a257b5c6ff0005 Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Tue, 12 Nov 2019 17:15:13 +0100 Subject: [PATCH 3/3] pytest: add test for raising listtransactions after funding --- tests/test_wallet.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 03a6556097a1..404adeabeb23 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -663,3 +663,18 @@ def test_hsmtool_secret_decryption(node_factory): l1.daemon.opts.pop("encrypted-hsm") l1.daemon.start(stdin=slave_fd, wait_for_initialized=True) assert node_id == l1.rpc.getinfo()["id"] + + +# this test does a 'listtransactions' on a yet unconfirmed channel +def test_fundchannel_listtransaction(node_factory, bitcoind): + l1, l2 = node_factory.get_nodes(2) + l1.fundwallet(10**6) + + l1.connect(l2) + txid = l1.rpc.fundchannel(l2.info['id'], 10**5)['txid'] + + # next call warned about SQL Accessing a null column + # and crashed the daemon for accessing random memory or null + txs = l1.rpc.listtransactions()['transactions'] + assert txs[0]['hash'] == txid + assert txs[0]['blockheight'] == 0