From 95f79d5815ac340e356f79fa02413a824c784051 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 08:39:53 +1030 Subject: [PATCH 1/6] wallet: store last block number we searched for UTXOs. We already go back 100 from this in case of reorgs, so the block number itself is sufficient. Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index cac3cfe7f237..1e9e974d6e4d 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -348,6 +348,10 @@ static void updates_complete(struct chain_topology *topo) /* Maybe need to rebroadcast. */ rebroadcast_txs(topo, NULL); + /* We've processed these UTXOs */ + db_set_intvar(topo->bitcoind->ld->wallet->db, + "last_processed_block", topo->tip->height); + topo->prev_tip = topo->tip; } From 1fac4efb75e6f930c6ce6131f7e1905213809986 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 08:39:55 +1030 Subject: [PATCH 2/6] wallet: use last_processed_block to determine scan start. With fallback depending on chainparams: this means the first upgrade will be slow, but after that it'll be fast. Fixes: #990 Signed-off-by: Rusty Russell --- bitcoin/chainparams.c | 5 +++++ bitcoin/chainparams.h | 1 + lightningd/chaintopology.c | 18 +++++++++--------- lightningd/lightningd.c | 9 ++++++--- lightningd/test/run-find_my_path.c | 6 +++--- wallet/wallet.c | 18 ++++++++++++------ wallet/wallet.h | 7 +++---- 7 files changed, 39 insertions(+), 25 deletions(-) diff --git a/bitcoin/chainparams.c b/bitcoin/chainparams.c index 5ce5168210a0..800e05594664 100644 --- a/bitcoin/chainparams.c +++ b/bitcoin/chainparams.c @@ -12,6 +12,8 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = NULL, .dust_limit = 546, + /* "Lightning Charge Powers Developers & Blockstream Store" */ + .when_lightning_became_cool = 504500, .testnet = false}, {.index = 1, .network_name = "regtest", @@ -21,6 +23,7 @@ const struct chainparams networks[] = { .cli = "bitcoin-cli", .cli_args = "-regtest", .dust_limit = 546, + .when_lightning_became_cool = 1, .testnet = true}, {.index = 2, .network_name = "testnet", @@ -39,6 +42,7 @@ const struct chainparams networks[] = { .cli = "litecoin-cli", .cli_args = NULL, .dust_limit = 100000, + .when_lightning_became_cool = 1, .testnet = false}, {.index = 4, .network_name = "litecoin-testnet", @@ -48,6 +52,7 @@ const struct chainparams networks[] = { .cli = "litecoin-cli", .cli_args = "-testnet", .dust_limit = 100000, + .when_lightning_became_cool = 1, .testnet = true} }; diff --git a/bitcoin/chainparams.h b/bitcoin/chainparams.h index 3a5d65489b5c..92a0d2d6dc32 100644 --- a/bitcoin/chainparams.h +++ b/bitcoin/chainparams.h @@ -15,6 +15,7 @@ struct chainparams { const char *cli; const char *cli_args; const u64 dust_limit; + const u32 when_lightning_became_cool; /* Whether this is a test network or not */ const bool testnet; diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 1e9e974d6e4d..c58b7130c93c 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -470,16 +470,16 @@ static void get_init_block(struct bitcoind *bitcoind, static void get_init_blockhash(struct bitcoind *bitcoind, u32 blockcount, struct chain_topology *topo) { - /* This happens if first_blocknum is UINTMAX-1 */ - if (blockcount < topo->first_blocknum) - topo->first_blocknum = blockcount; - /* FIXME: Because we don't handle our root disappearing, we go * 100 blocks back */ - if (topo->first_blocknum < 100) - topo->first_blocknum = 0; + if (blockcount < 100) + blockcount = 0; else - topo->first_blocknum -= 100; + blockcount -= 100; + + /* This happens if first_blocknum is UINTMAX-1 */ + if (blockcount < topo->first_blocknum) + topo->first_blocknum = blockcount; /* Get up to speed with topology. */ bitcoind_getblockhash(bitcoind, topo->first_blocknum, @@ -714,14 +714,14 @@ struct chain_topology *new_topology(struct lightningd *ld, struct log *log) void setup_topology(struct chain_topology *topo, struct timers *timers, - struct timerel poll_time, u32 first_channel_block) + struct timerel poll_time, u32 first_blocknum) { memset(&topo->feerate, 0, sizeof(topo->feerate)); topo->timers = timers; topo->poll_time = poll_time; /* Start one before the block we are interested in (as we won't * get notifications on txs in that block). */ - topo->first_blocknum = first_channel_block - 1; + topo->first_blocknum = first_blocknum - 1; /* Make sure bitcoind is started, and ready */ wait_for_bitcoind(topo->bitcoind); diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 2b3ee7a9e452..a08824dff3e5 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -239,7 +239,7 @@ int main(int argc, char *argv[]) struct log_book *log_book; struct lightningd *ld; bool newdir; - u32 peer_first_blocknum; + u32 first_blocknum; err_set_progname(argv[0]); @@ -322,7 +322,10 @@ int main(int argc, char *argv[]) if (!wallet_htlcs_reconnect(ld->wallet, &ld->htlcs_in, &ld->htlcs_out)) fatal("could not reconnect htlcs loaded from wallet, wallet may be inconsistent."); - peer_first_blocknum = wallet_channels_first_blocknum(ld->wallet); + /* Worst case, scan back to the first lightning deployment */ + first_blocknum = wallet_first_blocknum(ld->wallet, + get_chainparams(ld) + ->when_lightning_became_cool); db_commit_transaction(ld->wallet->db); @@ -330,7 +333,7 @@ int main(int argc, char *argv[]) setup_topology(ld->topology, &ld->timers, ld->config.poll_time, - peer_first_blocknum); + first_blocknum); /* Create RPC socket (if any) */ setup_jsonrpc(ld, ld->rpc_filename); diff --git a/lightningd/test/run-find_my_path.c b/lightningd/test/run-find_my_path.c index a00fb20f1acb..9b16093c31fc 100644 --- a/lightningd/test/run-find_my_path.c +++ b/lightningd/test/run-find_my_path.c @@ -87,12 +87,12 @@ struct txfilter *txfilter_new(const tal_t *ctx UNNEEDED) /* Generated stub for version */ const char *version(void) { fprintf(stderr, "version called!\n"); abort(); } -/* Generated stub for wallet_channels_first_blocknum */ -u32 wallet_channels_first_blocknum(struct wallet *w UNNEEDED) -{ fprintf(stderr, "wallet_channels_first_blocknum called!\n"); abort(); } /* Generated stub for wallet_channels_load_active */ bool wallet_channels_load_active(const tal_t *ctx UNNEEDED, struct wallet *w UNNEEDED) { fprintf(stderr, "wallet_channels_load_active called!\n"); abort(); } +/* Generated stub for wallet_first_blocknum */ +u32 wallet_first_blocknum(struct wallet *w UNNEEDED, u32 first_possible UNNEEDED) +{ fprintf(stderr, "wallet_first_blocknum called!\n"); abort(); } /* Generated stub for wallet_htlcs_load_for_channel */ bool wallet_htlcs_load_for_channel(struct wallet *wallet UNNEEDED, struct channel *chan UNNEEDED, diff --git a/wallet/wallet.c b/wallet/wallet.c index cb23976342c4..3ee3352c71fc 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -667,22 +667,28 @@ bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w) return ok; } -u32 wallet_channels_first_blocknum(struct wallet *w) +u32 wallet_first_blocknum(struct wallet *w, u32 first_possible) { int err; - u32 first_blocknum; + u32 first_channel, first_utxo; sqlite3_stmt *stmt = db_query(__func__, w->db, "SELECT MIN(first_blocknum) FROM channels;"); err = sqlite3_step(stmt); if (err == SQLITE_ROW && sqlite3_column_type(stmt, 0) != SQLITE_NULL) - first_blocknum = sqlite3_column_int(stmt, 0); + first_channel = sqlite3_column_int(stmt, 0); else - first_blocknum = UINT32_MAX; - + first_channel = UINT32_MAX; sqlite3_finalize(stmt); - return first_blocknum; + + /* If it's an old database, go back to before c-lightning was cool */ + first_utxo = db_get_intvar(w->db, "last_processed_block", + first_possible); + if (first_utxo < first_channel) + return first_utxo; + else + return first_channel; } void wallet_channel_config_save(struct wallet *w, struct channel_config *cc) diff --git a/wallet/wallet.h b/wallet/wallet.h index 6120852b42be..f911065fe5c3 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -260,13 +260,12 @@ bool wallet_peer_by_nodeid(struct wallet *w, const struct pubkey *nodeid, bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w); /** - * wallet_channels_first_blocknum - get first block we're interested in. + * wallet_first_blocknum - get first block we're interested in. * * @w: wallet to load from. - * - * Returns UINT32_MAX if nothing interesting. + * @first_possible: when c-lightning may have been active from */ -u32 wallet_channels_first_blocknum(struct wallet *w); +u32 wallet_first_blocknum(struct wallet *w, u32 first_possible); /** * wallet_extract_owned_outputs - given a tx, extract all of our outputs From 575b7faa52158e8379ff9aba2ecdb571b115d26a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 08:40:55 +1030 Subject: [PATCH 3/6] test: test for funds sent while we were offline. As described by @lvaccaro in #990. Signed-off-by: Rusty Russell --- tests/test_lightningd.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index be6b510c4aff..edaacb02fcfe 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -2823,6 +2823,25 @@ def test_lockin_between_restart(self): l1.daemon.wait_for_log(' to CHANNELD_NORMAL') l2.daemon.wait_for_log(' to CHANNELD_NORMAL') + def test_funding_while_offline(self): + l1 = self.node_factory.get_node() + addr = l1.rpc.newaddr()['address'] + + # l1 goes down. + l1.stop() + + # We send funds + bitcoind.rpc.sendtoaddress(addr, (10**6 + 1000000) / 10**8) + + # Now 120 blocks go by... + bitcoind.generate_block(120) + + # Restart + l1.daemon.start() + sync_blockheight([l1]) + + assert len(l1.rpc.listfunds()['outputs']) == 1 + def test_addfunds_from_block(self): """Send funds to the daemon without telling it explicitly """ From cefdbf2caf50deb8b0e8a623610511f36c9be81d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 08:44:44 +1030 Subject: [PATCH 4/6] wallet: don't scan from worst-case start on first use. We only need to do that if it's possible there's something to find: either we have an unspent output from a unilateral close, or we've ever handed out an address. Signed-off-by: Rusty Russell --- wallet/wallet.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/wallet/wallet.c b/wallet/wallet.c index 3ee3352c71fc..4fa5900b7d8b 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -667,6 +667,27 @@ bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w) return ok; } +/* Upgrade of db (or initial create): do we have anything to scan for? */ +static bool wallet_ever_used(struct wallet *w) +{ + sqlite3_stmt *stmt; + bool channel_utxos; + + /* If we ever handed out an address. */ + if (db_get_intvar(w->db, "bip32_max_index", 0) != 0) + return true; + + /* Or we could have had a channel terminate unilaterally, + * providing a UTXO. */ + stmt = db_query(__func__, w->db, + "SELECT COUNT(*) FROM outputs WHERE commitment_point IS NOT NULL;"); + sqlite3_step(stmt); + channel_utxos = (sqlite3_column_int(stmt, 0) != 0); + sqlite3_finalize(stmt); + + return channel_utxos; +} + u32 wallet_first_blocknum(struct wallet *w, u32 first_possible) { int err; @@ -675,6 +696,7 @@ u32 wallet_first_blocknum(struct wallet *w, u32 first_possible) db_query(__func__, w->db, "SELECT MIN(first_blocknum) FROM channels;"); + /* If we ever opened a channel, this will give us the first block. */ err = sqlite3_step(stmt); if (err == SQLITE_ROW && sqlite3_column_type(stmt, 0) != SQLITE_NULL) first_channel = sqlite3_column_int(stmt, 0); @@ -682,9 +704,16 @@ u32 wallet_first_blocknum(struct wallet *w, u32 first_possible) first_channel = UINT32_MAX; sqlite3_finalize(stmt); - /* If it's an old database, go back to before c-lightning was cool */ - first_utxo = db_get_intvar(w->db, "last_processed_block", - first_possible); + first_utxo = db_get_intvar(w->db, "last_processed_block", 0); + if (first_utxo == 0) { + /* Don't know? New db, or upgraded. */ + if (wallet_ever_used(w)) + /* Be conservative */ + first_utxo = first_possible; + else + first_utxo = UINT32_MAX; + } + if (first_utxo < first_channel) return first_utxo; else From 54cffacd446ea9d563c4da0507c45265e2dbbac2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 09:00:58 +1030 Subject: [PATCH 5/6] wallet: provide better comments on wallet_first_blocknum. ZmnSCPxj queried the unilateral close case, so make that clearer. Christian raise concerns about existing channels, so make it clear what we're doing there too. Signed-off-by: Rusty Russell --- wallet/wallet.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/wallet/wallet.c b/wallet/wallet.c index 4fa5900b7d8b..6795ee7c1c45 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -677,8 +677,7 @@ static bool wallet_ever_used(struct wallet *w) if (db_get_intvar(w->db, "bip32_max_index", 0) != 0) return true; - /* Or we could have had a channel terminate unilaterally, - * providing a UTXO. */ + /* Or if they do a unilateral close, the output to us provides a UTXO. */ stmt = db_query(__func__, w->db, "SELECT COUNT(*) FROM outputs WHERE commitment_point IS NOT NULL;"); sqlite3_step(stmt); @@ -688,6 +687,13 @@ static bool wallet_ever_used(struct wallet *w) return channel_utxos; } +/* We want the earlier of either: + * 1. The first channel we're still watching (it might have closed), + * 2. The last block we scanned for UTXO (might have new incoming payments) + * + * chaintopology actually subtracts another 100 blocks to make sure we + * catch chain forks. + */ u32 wallet_first_blocknum(struct wallet *w, u32 first_possible) { int err; From 2ac26dc403d9edb5ee77a2b10f5e8b9b3e1a59b3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Feb 2018 09:37:04 +1030 Subject: [PATCH 6/6] chaintopology: fix 100 block subtraction. We do a complicated dance because we don't know the current block height before setting up the topology. If we're starting at a particular block, we want to go back 100 blocks before that to cover any reorgs. If we're not (fresh startup), we still want to go back 100 blocks because we don't bother handling a reorg which removes all the blocks we know. Signed-off-by: Rusty Russell --- lightningd/chaintopology.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index c58b7130c93c..16d040dda8fe 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -470,17 +470,18 @@ static void get_init_block(struct bitcoind *bitcoind, static void get_init_blockhash(struct bitcoind *bitcoind, u32 blockcount, struct chain_topology *topo) { - /* FIXME: Because we don't handle our root disappearing, we go - * 100 blocks back */ - if (blockcount < 100) - blockcount = 0; - else - blockcount -= 100; - - /* This happens if first_blocknum is UINTMAX-1 */ + /* This can happen if bitcoind still syncing, or first_blocknum is MAX */ if (blockcount < topo->first_blocknum) topo->first_blocknum = blockcount; + /* For fork protection (esp. because we don't handle our own first + * block getting reorged out), we always go 100 blocks further back + * than we need. */ + if (topo->first_blocknum < 100) + topo->first_blocknum = 0; + else + topo->first_blocknum -= 100; + /* Get up to speed with topology. */ bitcoind_getblockhash(bitcoind, topo->first_blocknum, get_init_block, topo); @@ -719,9 +720,8 @@ void setup_topology(struct chain_topology *topo, memset(&topo->feerate, 0, sizeof(topo->feerate)); topo->timers = timers; topo->poll_time = poll_time; - /* Start one before the block we are interested in (as we won't - * get notifications on txs in that block). */ - topo->first_blocknum = first_blocknum - 1; + + topo->first_blocknum = first_blocknum; /* Make sure bitcoind is started, and ready */ wait_for_bitcoind(topo->bitcoind);