From 72108f0cb9333bcf90ec33d7e124765d068efc95 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 18 Feb 2018 23:23:46 +1030 Subject: [PATCH] wallet: don't use rowid for the channel's DBID. We derive the seed from this, so it needs to be unique, but using rowid forced us to put the channel into the db early, before it was ready. Instead, use a counter to ensure uniqueness, initialized when we load existing peers. This doesn't need to touch the database at all. As we now have only two places where the channel is committed (the funder and fundee paths), so we create a new explicit 'wallet_channel_insert()' function: 'wallet_channel_save()' now just updates. Note that this also fixes some weirdness in wallet_channels_load_active: we strangely avoided loading channels in CLOSINGD_COMPLETE (which fortunately was a transient state, so unlikely anyone hit this). Note that since the lines above already delete all the OPENINGD channels, we now simply load them all. Signed-off-by: Rusty Russell --- lightningd/channel.c | 11 +++--- lightningd/channel.h | 4 --- lightningd/peer_control.c | 61 ++++++++++++++----------------- lightningd/peer_state.h | 1 + wallet/test/run-wallet.c | 5 ++- wallet/wallet.c | 76 +++++++++++++++++++++++---------------- wallet/wallet.h | 16 +++++++++ 7 files changed, 98 insertions(+), 76 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 238a90ff8..b93ee54b9 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -84,9 +84,9 @@ void delete_channel(struct channel *channel, const char *why) * reconnection. We use the DB channel ID to guarantee unique secrets * per channel. */ -void derive_channel_seed(struct lightningd *ld, struct privkey *seed, - const struct pubkey *peer_id, - const u64 dbid) +static void derive_channel_seed(struct lightningd *ld, struct privkey *seed, + const struct pubkey *peer_id, + const u64 dbid) { u8 input[PUBKEY_DER_LEN + sizeof(dbid)]; char *info = "per-peer seed"; @@ -106,6 +106,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, u32 first_blocknum) struct channel *channel = talz(peer->ld, struct channel); char *idname; + assert(dbid != 0); channel->dbid = dbid; channel->peer = peer; channel->first_blocknum = first_blocknum; @@ -120,9 +121,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, u32 first_blocknum) tal_free(idname); list_add_tail(&peer->channels, &channel->list); tal_add_destructor(channel, destroy_channel); - if (channel->dbid != 0) - derive_channel_seed(peer->ld, &channel->seed, &peer->id, - channel->dbid); + derive_channel_seed(peer->ld, &channel->seed, &peer->id, channel->dbid); return channel; } diff --git a/lightningd/channel.h b/lightningd/channel.h index 36778db39..6a435f543 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -95,10 +95,6 @@ const char *channel_state_name(const struct channel *channel); void channel_set_owner(struct channel *channel, struct subd *owner); -void derive_channel_seed(struct lightningd *ld, struct privkey *seed, - const struct pubkey *peer_id, - const u64 dbid); - /* Channel has failed, but can try again. */ PRINTF_FMT(2,3) void channel_fail_transient(struct channel *channel, const char *fmt,...); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index a4ee0accc..8ec12aeed 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1436,6 +1436,8 @@ static void opening_got_hsm_funding_sig(struct funding_channel *fc, fatal("HSM gave bad sign_funding_reply %s", tal_hex(fc, resp)); + wallet_channel_insert(ld->wallet, channel); + /* Send it out and watch for confirms. */ broadcast_tx(ld->topology, channel, tx, funding_broadcast_failed); watch_tx(channel, ld->topology, channel, tx, funding_lockin_cb, NULL); @@ -2219,11 +2221,14 @@ static void opening_fundee_finished(struct subd *opening, /* old_remote_per_commit not valid yet, copy valid one. */ channel_info->old_remote_per_commit = channel_info->remote_per_commit; + channel_commit_initial(channel); + + /* Now we finally put it in the database. */ + wallet_channel_insert(ld->wallet, channel); + /* Now, keep the initial commit as our last-tx-to-broadcast. */ channel_set_last_tx(channel, remote_commit, &remote_commit_sig); - channel_commit_initial(channel); - log_debug(channel->log, "Watching funding tx %s", type_to_string(reply, struct bitcoin_txid, channel->funding_txid)); @@ -2281,6 +2286,24 @@ static unsigned int opening_negotiation_failed(struct subd *openingd, return 0; } +static struct channel *new_uncommitted_channel(struct lightningd *ld, + const struct pubkey *peer_id, + const struct wireaddr *addr) +{ + struct peer *peer; + struct channel *channel; + + /* We make a new peer if necessary. */ + peer = peer_by_id(ld, peer_id); + if (!peer) + peer = new_peer(ld, 0, peer_id, addr); + + channel = new_channel(peer, wallet_get_channel_dbid(ld->wallet), + get_block_height(ld->topology)); + assert(channel->peer == peer); + return channel; +} + /* Peer has spontaneously exited from gossip due to open msg */ static void peer_accept_channel(struct lightningd *ld, const struct pubkey *peer_id, @@ -2294,18 +2317,11 @@ static void peer_accept_channel(struct lightningd *ld, u32 max_to_self_delay, max_minimum_depth; u64 min_effective_htlc_capacity_msat; u8 *msg; - struct peer *peer; struct channel *channel; assert(fromwire_peektype(open_msg) == WIRE_OPEN_CHANNEL); - /* We make a new peer if necessary. */ - peer = peer_by_id(ld, peer_id); - if (!peer) - peer = new_peer(ld, 0, peer_id, addr); - - channel = new_channel(peer, 0, get_block_height(ld->topology)); - assert(channel->peer == peer); + channel = new_uncommitted_channel(ld, peer_id, addr); channel_set_state(channel, UNINITIALIZED, OPENINGD); channel_set_owner(channel, @@ -2336,10 +2352,6 @@ static void peer_accept_channel(struct lightningd *ld, &max_to_self_delay, &max_minimum_depth, &min_effective_htlc_capacity_msat); - /* Store the channel in the database in order to get a channel - * ID that is unique and which we can base the peer_seed on */ - wallet_channel_save(ld->wallet, channel); - msg = towire_opening_init(channel, get_chainparams(ld)->index, &channel->our_config, max_to_self_delay, @@ -2374,16 +2386,8 @@ static void peer_offer_channel(struct lightningd *ld, u8 *msg; u32 max_to_self_delay, max_minimum_depth; u64 min_effective_htlc_capacity_msat; - struct peer *peer; - - /* We make a new peer if necessary. */ - peer = peer_by_id(ld, &fc->peerid); - if (!peer) - peer = new_peer(ld, 0, &fc->peerid, addr); - - fc->channel = new_channel(peer, 0, get_block_height(ld->topology)); - assert(fc->channel->peer == peer); + fc->channel = new_uncommitted_channel(ld, &fc->peerid, addr); fc->channel->funding_satoshi = fc->funding_satoshi; fc->channel->push_msat = fc->push_msat; @@ -2403,17 +2407,6 @@ static void peer_offer_channel(struct lightningd *ld, return; } - /* FIXME: This is wrong in several ways. - * - * 1. We should set the temporary channel id *now*, so that's the - * key. - * 2. We don't need the peer or channel in db until peer_persists(). - */ - - /* Store the channel in the database in order to get a channel - * ID that is unique and which we can base the peer_seed on */ - wallet_channel_save(ld->wallet, fc->channel); - /* We will fund channel */ fc->channel->funder = LOCAL; channel_config(ld, &fc->channel->our_config, diff --git a/lightningd/peer_state.h b/lightningd/peer_state.h index c0e73de6f..9a28b86ad 100644 --- a/lightningd/peer_state.h +++ b/lightningd/peer_state.h @@ -33,5 +33,6 @@ enum peer_state { ONCHAIND_OUR_UNILATERAL, ONCHAIND_MUTUAL }; +#define CHANNEL_STATE_MAX ONCHAIND_MUTUAL #endif /* LIGHTNING_LIGHTNINGD_PEER_STATE_H */ diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 618bd3755..bdff5bd6b 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -611,7 +611,7 @@ static struct wallet *create_test_wallet(struct lightningd *ld, const tal_t *ctx CHECK_MSG(w->db, "Failed opening the db"); db_migrate(w->db, w->log); CHECK_MSG(!wallet_err, "DB migration failed"); - + w->max_channel_dbid = 0; return w; } @@ -864,6 +864,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) c1.first_blocknum = 1; p = new_peer(ld, 0, &pk, NULL); c1.peer = p; + c1.dbid = wallet_get_channel_dbid(w); c1.our_msatoshi = NULL; c1.last_tx = NULL; c1.state = CHANNELD_NORMAL; @@ -879,6 +880,8 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) db_begin_transaction(w->db); CHECK(!wallet_err); + wallet_channel_insert(w, &c1); + /* Variant 1: insert with null for scid, funding_tx_id, channel_info, last_tx */ wallet_channel_save(w, &c1); CHECK_MSG(!wallet_err, diff --git a/wallet/wallet.c b/wallet/wallet.c index 1713825fa..8154ffeaf 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -525,6 +525,7 @@ static struct channel *wallet_stmt2channel(const tal_t *ctx, struct wallet *w, s remote_config_id = sqlite3_column_int64(stmt, 4); chan->state = sqlite3_column_int(stmt, 5); + assert(chan->state > OPENINGD && chan->state <= CHANNEL_STATE_MAX); chan->funder = sqlite3_column_int(stmt, 6); chan->channel_flags = sqlite3_column_int(stmt, 7); chan->minimum_depth = sqlite3_column_int(stmt, 8); @@ -647,11 +648,12 @@ bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w) sqlite3_bind_int64(stmt, 1, OPENINGD); db_exec_prepared(w->db, stmt); - /* Channels are active if they have reached at least the - * opening state and they are not marked as complete */ + /* We load all channels */ stmt = db_query( - __func__, w->db, "SELECT %s FROM channels WHERE state > %d AND state != %d;", - channel_fields, OPENINGD, CLOSINGD_COMPLETE); + __func__, w->db, "SELECT %s FROM channels;", + channel_fields); + + w->max_channel_dbid = 0; int count = 0; while (ok && stmt && sqlite3_step(stmt) == SQLITE_ROW) { @@ -660,6 +662,8 @@ bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w) ok = false; break; } + if (c->dbid > w->max_channel_dbid) + w->max_channel_dbid = c->dbid; count++; } log_debug(w->log, "Loaded %d channels from DB", count); @@ -781,38 +785,17 @@ bool wallet_channel_config_load(struct wallet *w, const u64 id, return ok; } +u64 wallet_get_channel_dbid(struct wallet *wallet) +{ + return ++wallet->max_channel_dbid; +} + void wallet_channel_save(struct wallet *w, struct channel *chan) { tal_t *tmpctx = tal_tmpctx(w); sqlite3_stmt *stmt; assert(chan->first_blocknum); - if (chan->peer->dbid == 0) { - /* Need to store the peer first */ - stmt = db_prepare(w->db, "INSERT INTO peers (node_id, address) VALUES (?, ?);"); - sqlite3_bind_pubkey(stmt, 1, &chan->peer->id); - if (chan->peer->addr.type == ADDR_TYPE_PADDING) - sqlite3_bind_null(stmt, 2); - else - sqlite3_bind_text(stmt, 2, - type_to_string(tmpctx, struct wireaddr, &chan->peer->addr), - -1, SQLITE_TRANSIENT); - db_exec_prepared(w->db, stmt); - chan->peer->dbid = sqlite3_last_insert_rowid(w->db->sql); - } - - /* Insert a stub, that we can update, unifies INSERT and UPDATE paths */ - if (chan->dbid == 0) { - stmt = db_prepare(w->db, "INSERT INTO channels (" - "peer_id, first_blocknum) VALUES (?, ?);"); - sqlite3_bind_int64(stmt, 1, chan->peer->dbid); - sqlite3_bind_int(stmt, 2, chan->first_blocknum); - db_exec_prepared(w->db, stmt); - chan->dbid = sqlite3_last_insert_rowid(w->db->sql); - derive_channel_seed(w->ld, &chan->seed, &chan->peer->id, - chan->dbid); - } - /* Need to initialize the shachain first so we get an id */ if (chan->their_shachain.id == 0) { wallet_shachain_init(w, &chan->their_shachain); @@ -820,7 +803,6 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) wallet_channel_config_save(w, &chan->our_config); - /* Now do the real update */ stmt = db_prepare(w->db, "UPDATE channels SET" " shachain_remote_id=?," " short_channel_id=?," @@ -925,6 +907,38 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) tal_free(tmpctx); } +void wallet_channel_insert(struct wallet *w, struct channel *chan) +{ + tal_t *tmpctx = tal_tmpctx(w); + sqlite3_stmt *stmt; + + if (chan->peer->dbid == 0) { + /* Need to create the peer first */ + stmt = db_prepare(w->db, "INSERT INTO peers (node_id, address) VALUES (?, ?);"); + sqlite3_bind_pubkey(stmt, 1, &chan->peer->id); + if (chan->peer->addr.type == ADDR_TYPE_PADDING) + sqlite3_bind_null(stmt, 2); + else + sqlite3_bind_text(stmt, 2, + type_to_string(tmpctx, struct wireaddr, &chan->peer->addr), + -1, SQLITE_TRANSIENT); + db_exec_prepared(w->db, stmt); + chan->peer->dbid = sqlite3_last_insert_rowid(w->db->sql); + } + + /* Insert a stub, that we update, unifies INSERT and UPDATE paths */ + stmt = db_prepare(w->db, "INSERT INTO channels (" + "peer_id, first_blocknum, id) VALUES (?, ?, ?);"); + sqlite3_bind_int64(stmt, 1, chan->peer->dbid); + sqlite3_bind_int(stmt, 2, chan->first_blocknum); + sqlite3_bind_int(stmt, 3, chan->dbid); + db_exec_prepared(w->db, stmt); + + /* Now save path as normal */ + wallet_channel_save(w, chan); + tal_free(tmpctx); +} + void wallet_channel_delete(struct wallet *w, u64 wallet_id) { sqlite3_stmt *stmt; diff --git a/wallet/wallet.h b/wallet/wallet.h index de32f47ef..60e0866bf 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -30,6 +30,7 @@ struct wallet { struct ext_key *bip32_base; struct invoices *invoices; struct list_head unstored_payments; + u64 max_channel_dbid; }; /* Possible states for tracked outputs in the database. Not sure yet @@ -206,6 +207,13 @@ bool wallet_shachain_load(struct wallet *wallet, u64 id, struct wallet_shachain *chain); +/** + * wallet_get_uncommitted_channel_dbid -- get a unique channel dbid + * + * @wallet: the wallet + */ +u64 wallet_get_channel_dbid(struct wallet *wallet); + /** * wallet_channel_save -- Upsert the channel into the database * @@ -215,6 +223,14 @@ bool wallet_shachain_load(struct wallet *wallet, u64 id, */ void wallet_channel_save(struct wallet *w, struct channel *chan); +/** + * wallet_channel_insert -- Insert the initial channel into the database + * + * @wallet: the wallet to save into + * @chan: the instance to store + */ +void wallet_channel_insert(struct wallet *w, struct channel *chan); + /** * wallet_channel_delete -- After resolving a channel, forget about it */