From e84baf78ac1dc23fadce580f9b9b26a7a6d19bb9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 31 Jan 2024 13:46:17 +1030 Subject: [PATCH] lightningd: tweak db remote channel_announcement sigs API. Don't return false on db errors (we always fail on those), but return false if they don't exist. Also, add routine to clear them. Signed-off-by: Rusty Russell --- lightningd/channel_control.c | 17 +++++------ wallet/test/run-wallet.c | 6 +++- wallet/wallet.c | 55 ++++++++++++++++++++---------------- wallet/wallet.h | 20 +++++++++---- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index a931fd02b..b2b04e3a2 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -1557,14 +1557,15 @@ bool peer_start_channeld(struct channel *channel, if (channel->ignore_fee_limits || ld->config.ignore_fee_limits) log_unusual(channel->log, "Ignoring fee limits!"); - if (!wallet_remote_ann_sigs_load(tmpctx, channel->peer->ld->wallet, - channel->dbid, - &remote_ann_node_sig, - &remote_ann_bitcoin_sig)) { - channel_internal_error(channel, - "Could not load remote announcement" - " signatures"); - return false; + remote_ann_node_sig = tal(tmpctx, secp256k1_ecdsa_signature); + remote_ann_bitcoin_sig = tal(tmpctx, secp256k1_ecdsa_signature); + + if (!wallet_remote_ann_sigs_load(channel->peer->ld->wallet, + channel, + remote_ann_node_sig, + remote_ann_bitcoin_sig)) { + remote_ann_node_sig = tal_free(remote_ann_node_sig); + remote_ann_bitcoin_sig = tal_free(remote_ann_bitcoin_sig); } pbases = wallet_penalty_base_load_for_channel( diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 8860e184c..ebc7e4ce3 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1818,12 +1818,16 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) wallet_announcement_save(w, c1.dbid, node_sig1, bitcoin_sig1); CHECK_MSG(!wallet_err, tal_fmt(w, "Insert ann sigs into DB: %s", wallet_err)); - CHECK_MSG(load = wallet_remote_ann_sigs_load(w, w, c1.dbid, &node_sig2, &bitcoin_sig2), tal_fmt(w, "Load ann sigs from DB")); + node_sig2 = tal(tmpctx, secp256k1_ecdsa_signature); + bitcoin_sig2 = tal(tmpctx, secp256k1_ecdsa_signature); + CHECK_MSG(load = wallet_remote_ann_sigs_load(w, &c1, node_sig2, bitcoin_sig2), tal_fmt(w, "Load ann sigs from DB")); CHECK_MSG(!wallet_err, tal_fmt(w, "Load ann sigs from DB: %s", wallet_err)); CHECK(load == true); CHECK_MSG(!memcmp(node_sig1, node_sig2, sizeof(*node_sig1)), "Compare ann sigs loaded with saved (v5)"); CHECK_MSG(!memcmp(bitcoin_sig1, bitcoin_sig2, sizeof(*node_sig1)), "Compare ann sigs loaded with saved (v5)"); + wallet_remote_ann_sigs_clear(w, &c1); + CHECK(!wallet_remote_ann_sigs_load(w, &c1, node_sig2, bitcoin_sig2)); db_commit_transaction(w->db); CHECK(!wallet_err); diff --git a/wallet/wallet.c b/wallet/wallet.c index f7af8fea4..afeff42f2 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1075,16 +1075,17 @@ wallet_htlc_sigs_load(const tal_t *ctx, struct wallet *w, u64 channelid, return htlc_sigs; } -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) +bool wallet_remote_ann_sigs_load(struct wallet *w, + const struct channel *chan, + secp256k1_ecdsa_signature *remote_ann_node_sig, + secp256k1_ecdsa_signature *remote_ann_bitcoin_sig) { struct db_stmt *stmt; bool res; stmt = db_prepare_v2( w->db, SQL("SELECT remote_ann_node_sig, remote_ann_bitcoin_sig" " FROM channels WHERE id = ?")); - db_bind_u64(stmt, id); + db_bind_u64(stmt, chan->dbid); db_query_prepared(stmt); res = db_step(stmt); @@ -1096,29 +1097,31 @@ bool wallet_remote_ann_sigs_load(const tal_t *ctx, struct wallet *w, u64 id, if (db_col_is_null(stmt, "remote_ann_node_sig") || db_col_is_null(stmt, "remote_ann_bitcoin_sig")) { db_col_ignore(stmt, "remote_ann_bitcoin_sig"); - *remote_ann_node_sig = *remote_ann_bitcoin_sig = NULL; tal_free(stmt); - return true; + return false; } - /* the case left over is both sigs exist */ - *remote_ann_node_sig = tal(ctx, secp256k1_ecdsa_signature); - *remote_ann_bitcoin_sig = tal(ctx, secp256k1_ecdsa_signature); + if (!db_col_signature(stmt, "remote_ann_node_sig", remote_ann_node_sig)) + db_fatal(w->db, "Failed to decode remote_ann_node_sig for id %"PRIu64, chan->dbid); - if (!db_col_signature(stmt, "remote_ann_node_sig", *remote_ann_node_sig)) - goto fail; - - if (!db_col_signature(stmt, "remote_ann_bitcoin_sig", *remote_ann_bitcoin_sig)) - goto fail; + if (!db_col_signature(stmt, "remote_ann_bitcoin_sig", remote_ann_bitcoin_sig)) + db_fatal(w->db, "Failed to decode remote_ann_bitcoin_sig for id %"PRIu64, chan->dbid); tal_free(stmt); return true; +} -fail: - *remote_ann_node_sig = tal_free(*remote_ann_node_sig); - *remote_ann_bitcoin_sig = tal_free(*remote_ann_bitcoin_sig); - tal_free(stmt); - return false; +void wallet_remote_ann_sigs_clear(struct wallet *w, const struct channel *chan) +{ + struct db_stmt *stmt; + stmt = db_prepare_v2(w->db, + SQL("UPDATE channels" + " SET remote_ann_node_sig=?, remote_ann_bitcoin_sig=?" + " WHERE id = ?")); + db_bind_null(stmt); + db_bind_null(stmt); + db_bind_u64(stmt, chan->dbid); + db_exec_prepared_v2(take(stmt)); } static struct fee_states *wallet_channel_fee_states_load(struct wallet *w, @@ -2215,6 +2218,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) { struct db_stmt *stmt; u8 *last_sent_commit; + const struct peer_update *peer_update; assert(chan->first_blocknum); wallet_channel_config_save(w, &chan->our_config); @@ -2353,12 +2357,13 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) db_bind_null(stmt); db_bind_int(stmt, chan->ignore_fee_limits); - if (chan->peer_update) { - db_bind_int(stmt, chan->peer_update->fee_base); - db_bind_int(stmt, chan->peer_update->fee_ppm); - db_bind_int(stmt, chan->peer_update->cltv_delta); - db_bind_amount_msat(stmt, &chan->peer_update->htlc_minimum_msat); - db_bind_amount_msat(stmt, &chan->peer_update->htlc_maximum_msat); + peer_update = chan->peer_update; + if (peer_update) { + db_bind_int(stmt, peer_update->fee_base); + db_bind_int(stmt, peer_update->fee_ppm); + db_bind_int(stmt, peer_update->cltv_delta); + db_bind_amount_msat(stmt, &peer_update->htlc_minimum_msat); + db_bind_amount_msat(stmt, &peer_update->htlc_maximum_msat); } else { db_bind_null(stmt); db_bind_null(stmt); diff --git a/wallet/wallet.h b/wallet/wallet.h index 2e8138d5f..904cd2301 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1262,15 +1262,25 @@ bool wallet_forward_delete(struct wallet *w, /** * Load remote_ann_node_sig and remote_ann_bitcoin_sig * - * @ctx: allocation context for the return value * @w: wallet containing the channel - * @id: channel database id + * @chan: channel (must be in db) * @remote_ann_node_sig: location to load remote_ann_node_sig to * @remote_ann_bitcoin_sig: location to load remote_ann_bitcoin_sig to + * + * Returns false if the signatures were null. */ -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); +bool wallet_remote_ann_sigs_load(struct wallet *w, + const struct channel *chan, + secp256k1_ecdsa_signature *remote_ann_node_sig, + secp256k1_ecdsa_signature *remote_ann_bitcoin_sig); + +/** + * Null out remote_ann_node_sig and remote_ann_bitcoin_sig + * + * @w: wallet containing the channel + * @id: channel database id + */ +void wallet_remote_ann_sigs_clear(struct wallet *w, const struct channel *chan); /** * Get a list of transactions that we track in the wallet.