diff --git a/lightningd/channel.c b/lightningd/channel.c index b835964b1..238a90ff8 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -51,22 +51,22 @@ static void destroy_channel(struct channel *channel) channel_set_owner(channel, NULL); list_del_from(&channel->peer->channels, &channel->list); - - /* Last one out frees the peer */ - if (list_empty(&channel->peer->channels)) - tal_free(channel->peer); } /* This lets us give a more detailed error than just a destructor. */ void delete_channel(struct channel *channel, const char *why) { + struct peer *peer = channel->peer; if (channel->opening_cmd) { command_fail(channel->opening_cmd, "%s", why); channel->opening_cmd = NULL; } - wallet_channel_delete(channel->peer->ld->wallet, channel->dbid, - channel->peer->dbid); + wallet_channel_delete(channel->peer->ld->wallet, channel->dbid); tal_free(channel); + + /* Last one out frees the peer */ + if (list_empty(&peer->channels)) + delete_peer(peer); } /* FIXME: We have no business knowing this! */ diff --git a/lightningd/channel.h b/lightningd/channel.h index 5a68424c7..36778db39 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -86,7 +86,9 @@ struct channel { }; struct channel *new_channel(struct peer *peer, u64 dbid, u32 first_blocknum); -/* This lets us give a more detailed error than just a destructor. */ + +/* This lets us give a more detailed error than just a destructor, and + * deletes from db. */ void delete_channel(struct channel *channel, const char *why); const char *channel_state_name(const struct channel *channel); diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index be37a95c2..2b3ee7a9e 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -200,16 +200,16 @@ static void shutdown_subdaemons(struct lightningd *ld) free_htlcs(ld, NULL); while ((p = list_top(&ld->peers, struct peer, list)) != NULL) { - struct channel *c, *next; - - /* Careful: deleting last channel frees p! */ - do { - c = list_top(&p->channels, struct channel, list); - next = list_next(&p->channels, c, list); + struct channel *c; + while ((c = list_top(&p->channels, struct channel, list)) + != NULL) { + /* Removes itself from list as we free it */ tal_free(c); - c = next; - } while (c); + } + + /* Removes itself from list as we free it */ + tal_free(p); } db_commit_transaction(ld->wallet->db); } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index a2178b55b..a4ee0accc 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -120,6 +120,13 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, return peer; } +void delete_peer(struct peer *peer) +{ + assert(list_empty(&peer->channels)); + wallet_peer_delete(peer->ld->wallet, peer->dbid); + tal_free(peer); +} + struct peer *find_peer_by_dbid(struct lightningd *ld, u64 dbid) { struct peer *p; diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index c29815bfc..fd9045e38 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -50,6 +50,9 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct pubkey *id, const struct wireaddr *addr); +/* Also removes from db. */ +void delete_peer(struct peer *peer); + struct peer *peer_by_id(struct lightningd *ld, const struct pubkey *id); struct peer *peer_from_json(struct lightningd *ld, const char *buffer, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index aaed66c19..618bd3755 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -887,6 +887,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Load from DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v1)"); + tal_free(c2); /* We just inserted them into an empty DB so this must be 1 */ CHECK(c1.dbid == 1); @@ -902,6 +903,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v2)"); + tal_free(c2); /* Updates should not result in new ids */ CHECK(c1.dbid == 1); @@ -918,6 +920,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v3)"); + tal_free(c2); /* Variant 4: update with funding_tx_id */ c1.funding_txid = hash; @@ -927,6 +930,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v4)"); + tal_free(c2); /* Variant 5: update with channel_info */ c1.channel_info = &ci; @@ -936,6 +940,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v5)"); + tal_free(c2); /* Variant 6: update with last_commit_sent */ c1.last_sent_commit = &last_commit; @@ -945,6 +950,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v6)"); + tal_free(c2); /* Variant 7: update with last_tx (taken from BOLT #3) */ c1.last_tx = bitcoin_tx_from_hex(w, "02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8003a00f0000000000002200208c48d15160397c9731df9bc3b236656efb6665fbfe92b4a6878e88a499f741c4c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de843110ae8f6a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e040047304402206a2679efa3c7aaffd2a447fd0df7aba8792858b589750f6a1203f9259173198a022008d52a0e77a99ab533c36206cb15ad7aeb2aa72b93d4b571e728cb5ec2f6fe260147304402206d6cb93969d39177a09d5d45b583f34966195b77c7e585cf47ac5cce0c90cefb022031d71ae4e33a4e80df7f981d696fbdee517337806a3c7138b7491e2cbb077a0e01475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220", strlen("02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8003a00f0000000000002200208c48d15160397c9731df9bc3b236656efb6665fbfe92b4a6878e88a499f741c4c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de843110ae8f6a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e040047304402206a2679efa3c7aaffd2a447fd0df7aba8792858b589750f6a1203f9259173198a022008d52a0e77a99ab533c36206cb15ad7aeb2aa72b93d4b571e728cb5ec2f6fe260147304402206d6cb93969d39177a09d5d45b583f34966195b77c7e585cf47ac5cce0c90cefb022031d71ae4e33a4e80df7f981d696fbdee517337806a3c7138b7491e2cbb077a0e01475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220")); @@ -955,6 +961,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v7)"); + tal_free(c2); /* Variant 8: update and add remote_shutdown_scriptpubkey */ c1.remote_shutdown_scriptpubkey = scriptpubkey; @@ -965,10 +972,13 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v8)"); + tal_free(c2); db_commit_transaction(w->db); CHECK(!wallet_err); tal_free(w); + /* Normally freed by destroy_channel, but we don't call that */ + tal_free(p); return true; } diff --git a/wallet/wallet.c b/wallet/wallet.c index 7e18de02f..361d87845 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -884,21 +884,28 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) tal_free(tmpctx); } -void wallet_channel_delete(struct wallet *w, u64 wallet_id, u64 peer_dbid) +void wallet_channel_delete(struct wallet *w, u64 wallet_id) { sqlite3_stmt *stmt; - /* FIXME: The line to clean up if we're last channel for peer would - * be better as a trigger. */ stmt = db_prepare(w->db, "DELETE FROM channels WHERE id=?"); sqlite3_bind_int64(stmt, 1, wallet_id); db_exec_prepared(w->db, stmt); +} - stmt = db_prepare(w->db, - "DELETE FROM peers WHERE id = ? AND NOT EXISTS" - " (SELECT 1 FROM channels WHERE peer_id = ?)"); +void wallet_peer_delete(struct wallet *w, u64 peer_dbid) +{ + sqlite3_stmt *stmt; + + /* Must not have any channels still using this peer */ + stmt = db_query(__func__, w->db, + "SELECT * FROM channels WHERE peer_id = %"PRIu64, + peer_dbid); + assert(sqlite3_step(stmt) == SQLITE_DONE); + sqlite3_finalize(stmt); + + stmt = db_prepare(w->db, "DELETE FROM peers WHERE id=?"); sqlite3_bind_int64(stmt, 1, peer_dbid); - sqlite3_bind_int64(stmt, 2, peer_dbid); db_exec_prepared(w->db, stmt); } diff --git a/wallet/wallet.h b/wallet/wallet.h index 0fadfc58a..6120852b4 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -216,10 +216,13 @@ void wallet_channel_save(struct wallet *w, struct channel *chan); /** * wallet_channel_delete -- After resolving a channel, forget about it - * - * Deletes peer too if no more channels. */ -void wallet_channel_delete(struct wallet *w, u64 wallet_id, u64 peer_dbid); +void wallet_channel_delete(struct wallet *w, u64 wallet_id); + +/** + * wallet_peer_delete -- After no more channels in peer, forget about it + */ +void wallet_peer_delete(struct wallet *w, u64 peer_dbid); /** * wallet_channel_config_save -- Upsert a channel_config into the database