From e1dbc0b12b1684a0cb9bedb47bf2bdee267e6a31 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 5 Jun 2019 16:30:05 +0930 Subject: [PATCH] wallet: clean reserved inputs on startup. We reserve inputs when we're going to send a transaction, but we don't unreserve them if we crash. This is most graphically demonstrated by the txprepare case, which makes it easier to trigger. Instead, we should query bitcoind to see whether the tx made it out or not, as we would do manually with dev-rescan-outputs. Signed-off-by: Rusty Russell --- lightningd/lightningd.c | 4 +++ lightningd/test/run-find_my_abspath.c | 3 +++ tests/test_wallet.py | 5 +++- wallet/wallet.c | 38 +++++++++++++++++++++++++++ wallet/wallet.h | 11 ++++++++ 5 files changed, 60 insertions(+), 1 deletion(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 9f19a4334..357212223 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -736,6 +736,10 @@ int main(int argc, char *argv[]) else if (max_blockheight != UINT32_MAX) max_blockheight -= ld->config.rescan; + /*~ Tell the wallet to start figuring out what to do for any reserved + * unspent outputs we may have crashed with. */ + wallet_clean_utxos(ld->wallet, ld->topology->bitcoind); + /*~ That's all of the wallet db operations for now. */ db_commit_transaction(ld->wallet->db); diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 593d6bc3f..cdc58a8e1 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -173,6 +173,9 @@ const char *version(void) /* Generated stub for wallet_blocks_heights */ void wallet_blocks_heights(struct wallet *w UNNEEDED, u32 def UNNEEDED, u32 *min UNNEEDED, u32 *max UNNEEDED) { fprintf(stderr, "wallet_blocks_heights called!\n"); abort(); } +/* Generated stub for wallet_clean_utxos */ +void wallet_clean_utxos(struct wallet *w UNNEEDED, struct bitcoind *bitcoind UNNEEDED) +{ fprintf(stderr, "wallet_clean_utxos called!\n"); abort(); } /* Generated stub for wallet_network_check */ bool wallet_network_check(struct wallet *w UNNEEDED, const struct chainparams *chainparams UNNEEDED) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a43677640..b2178754b 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -323,7 +323,6 @@ def test_txsend(node_factory, bitcoind): assert decode['vout'][changenum]['scriptPubKey']['addresses'][0] in [f['address'] for f in l1.rpc.listfunds()['outputs']] -@pytest.mark.xfail(strict=True) def test_txprepare_restart(node_factory, bitcoind): amount = 1000000 l1 = node_factory.get_node(may_fail=True) @@ -367,6 +366,10 @@ def test_txprepare_restart(node_factory, bitcoind): # It goes backwards in blockchain just in case there was a reorg. Wait. wait_for(lambda: [o['status'] for o in l1.rpc.listfunds()['outputs']] == ['confirmed'] * 10) + # It should have logged this for each output. + for i in decode['vin']: + assert l1.daemon.is_in_log('wallet: reserved output {}/{} reset to available'.format(i['txid'], i['vout'])) + prep = l1.rpc.txprepare('bcrt1qeyyk6sl5pr49ycpqyckvmttus5ttj25pd0zpvg', 'all') decode = bitcoind.rpc.decoderawtransaction(prep['unsigned_tx']) diff --git a/wallet/wallet.c b/wallet/wallet.c index d86b8cad7..ac3143ee9 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -5,8 +5,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -2739,3 +2741,39 @@ void free_unreleased_txs(struct wallet *w) tal_free(utx); } +static void process_utxo_result(struct bitcoind *bitcoind, + const struct bitcoin_tx_output *txout, + void *_utxos) +{ + struct utxo **utxos = _utxos; + enum output_status newstate = + txout == NULL ? output_state_spent : output_state_available; + + log_unusual(bitcoind->ld->wallet->log, + "wallet: reserved output %s/%u reset to %s", + type_to_string(tmpctx, struct bitcoin_txid, &utxos[0]->txid), + utxos[0]->outnum, + newstate == output_state_spent ? "spent" : "available"); + wallet_update_output_status(bitcoind->ld->wallet, + &utxos[0]->txid, utxos[0]->outnum, + utxos[0]->status, newstate); + + /* If we have more, resolve them too. */ + tal_arr_remove(&utxos, 0); + if (tal_count(utxos) != 0) { + bitcoind_gettxout(bitcoind, &utxos[0]->txid, utxos[0]->outnum, + process_utxo_result, utxos); + } else + tal_free(utxos); +} + +void wallet_clean_utxos(struct wallet *w, struct bitcoind *bitcoind) +{ + struct utxo **utxos = wallet_get_utxos(NULL, w, output_state_reserved); + + if (tal_count(utxos) != 0) { + bitcoind_gettxout(bitcoind, &utxos[0]->txid, utxos[0]->outnum, + process_utxo_result, notleak(utxos)); + } else + tal_free(utxos); +} diff --git a/wallet/wallet.h b/wallet/wallet.h index 06eedf46f..f78398710 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1099,6 +1099,17 @@ bool wallet_remote_ann_sigs_load(const tal_t *ctx, struct wallet *w, u64 id, secp256k1_ecdsa_signature **remote_ann_node_sig, secp256k1_ecdsa_signature **remote_ann_bitcoin_sig); +/** + * wallet_clean_utxos: clean up any reserved UTXOs on restart. + * @w: wallet + * + * If we crash, it's unclear if we have actually used the inputs. eg. if + * we crash around transaction broadcast. + * + * We ask bitcoind to clarify in this case. + */ +void wallet_clean_utxos(struct wallet *w, struct bitcoind *bitcoind); + /* Operations for unreleased transactions */ struct unreleased_tx *find_unreleased_tx(struct wallet *w, const struct bitcoin_txid *txid);