lightningd: change watch_scriptpubkey and watch_blockdepth not to insert exact duplicates.
What the developer wrote
Authored by Rusty Russell
lightningd: change watch_scriptpubkey and watch_blockdepth not to insert exact duplicates.
This double-watching is in fact invoked for splicing (on restart, see
tests/test_splicing.py::test_commit_crash_splice) and the "not
creating duplicates" API mirrors the future bwatch API which is persistent.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This change prevents Core Lightning from registering the same blockchain watch twice. Duplicate watches could cause the same callback to fire multiple times, which during splicing (a way to resize a Lightning channel) after a restart could confuse internal bookkeeping. The patch makes the watch functions return false when an identical watch already exists and removes an unused memory-handling marker from the unwatch function.
Treat as a hardening/correctness fix rather than an urgent security patch. Review callers of watch_scriptpubkey and watch_blockdepth to ensure they handle the new false return value correctly and do not rely on duplicate registrations. Include the referenced splicing restart test in regression suite.
Security signals we found
Duplicate callback registration could lead to double-triggering of event handlers
Splicing restart path explicitly mentioned as invoking double-watching
Memory ownership annotation TAKES removed from unwatch API, indicating prior misuse risk
No explicit CVE, advisory, or security disclosure in commit or references
Evidence from the diff
watch_scriptpubkey_() and watch_blockdepth_() now search their respective hash tables for an entry matching scriptpubkey/outpoint/amount/cb/arg (or blockheight/cb/reorgcb/arg) before inserting. If a duplicate exists, they free the incoming scriptpubkey if it was TAKES and return false. unwatch_scriptpubkey_() no longer marks scriptpubkey as TAKES because the caller no longer passes ownership; the helper find_watchscriptpubkey() is shared between add and remove. The commit message says this is needed for splicing restart behavior (test_commit_crash_splice) and to align with a future persistent bwatch API.
Changed components
lightningd/watch.clightningd/watch.hchain_topology watch subsystemsplicing restart handlingInspect captured patch +83 / −22
diff --git a/lightningd/watch.c b/lightningd/watch.c
index eada9fa0..fdc57f07 100644
--- a/lightningd/watch.c
+++ b/lightningd/watch.c
@@ -346,7 +346,34 @@ static void destroy_scriptpubkeywatch(struct scriptpubkeywatch *w, struct chain_
scriptpubkeywatch_hash_del(topo->scriptpubkeywatches, w);
}
-void watch_scriptpubkey_(const tal_t *ctx,
+static struct scriptpubkeywatch *find_watchscriptpubkey(const struct scriptpubkeywatch_hash *scriptpubkeywatches,
+ const u8 *scriptpubkey,
+ const struct bitcoin_outpoint *expected_outpoint,
+ struct amount_sat expected_amount,
+ void (*cb)(struct lightningd *ld,
+ const struct bitcoin_tx *tx,
+ u32 outnum,
+ const struct txlocator *loc,
+ void *),
+ void *arg)
+{
+ struct scriptpubkeywatch_hash_iter it;
+ const struct script_with_len swl = { scriptpubkey, tal_bytelen(scriptpubkey) };
+
+ for (struct scriptpubkeywatch *w = scriptpubkeywatch_hash_getfirst(scriptpubkeywatches, &swl, &it);
+ w;
+ w = scriptpubkeywatch_hash_getnext(scriptpubkeywatches, &swl, &it)) {
+ if (bitcoin_outpoint_eq(&w->expected_outpoint, expected_outpoint)
+ && amount_sat_eq(w->expected_amount, expected_amount)
+ && w->cb == cb
+ && w->arg == arg) {
+ return w;
+ }
+ }
+ return NULL;
+}
+
+bool watch_scriptpubkey_(const tal_t *ctx,
struct chain_topology *topo,
const u8 *scriptpubkey TAKES,
const struct bitcoin_outpoint *expected_outpoint,
@@ -358,7 +385,19 @@ void watch_scriptpubkey_(const tal_t *ctx,
void *),
void *arg)
{
- struct scriptpubkeywatch *w = tal(ctx, struct scriptpubkeywatch);
+ struct scriptpubkeywatch *w;
+
+ if (find_watchscriptpubkey(topo->scriptpubkeywatches,
+ scriptpubkey,
+ expected_outpoint,
+ expected_amount,
+ cb, arg)) {
+ if (taken(scriptpubkey))
+ tal_free(scriptpubkey);
+ return false;
+ }
+
+ w = tal(ctx, struct scriptpubkeywatch);
w->swl.script = tal_dup_talarr(w, u8, scriptpubkey);
w->swl.len = tal_bytelen(w->swl.script);
w->expected_outpoint = *expected_outpoint;
@@ -367,11 +406,12 @@ void watch_scriptpubkey_(const tal_t *ctx,
w->arg = arg;
scriptpubkeywatch_hash_add(topo->scriptpubkeywatches, w);
tal_add_destructor2(w, destroy_scriptpubkeywatch, topo);
+ return true;
}
bool unwatch_scriptpubkey_(const tal_t *ctx,
struct chain_topology *topo,
- const u8 *scriptpubkey TAKES,
+ const u8 *scriptpubkey,
const struct bitcoin_outpoint *expected_outpoint,
struct amount_sat expected_amount,
void (*cb)(struct lightningd *ld,
@@ -381,18 +421,12 @@ bool unwatch_scriptpubkey_(const tal_t *ctx,
void *),
void *arg)
{
- struct scriptpubkeywatch_hash_iter it;
- const struct script_with_len swl = { scriptpubkey, tal_bytelen(scriptpubkey) };
-
- for (struct scriptpubkeywatch *w = scriptpubkeywatch_hash_getfirst(topo->scriptpubkeywatches, &swl, &it);
- w;
- w = scriptpubkeywatch_hash_getnext(topo->scriptpubkeywatches, &swl, &it)) {
- if (!bitcoin_outpoint_eq(&w->expected_outpoint, expected_outpoint)
- || !amount_sat_eq(w->expected_amount, expected_amount)
- || w->cb != cb
- || w->arg != arg) {
- continue;
- }
+ struct scriptpubkeywatch *w = find_watchscriptpubkey(topo->scriptpubkeywatches,
+ scriptpubkey,
+ expected_outpoint,
+ expected_amount,
+ cb, arg);
+ if (w) {
tal_free(w);
return true;
}
@@ -500,20 +534,47 @@ static void destroy_blockdepthwatch(struct blockdepthwatch *w, struct chain_topo
blockdepthwatch_hash_del(topo->blockdepthwatches, w);
}
-void watch_blockdepth_(const tal_t *ctx,
+static struct blockdepthwatch *find_blockdepthwatch(const struct blockdepthwatch_hash *blockdepthwatches,
+ u32 blockheight,
+ enum watch_result (*depthcb)(struct lightningd *ld, u32 depth, void *),
+ enum watch_result (*reorgcb)(struct lightningd *ld, void *),
+ void *arg)
+{
+ struct blockdepthwatch_hash_iter it;
+
+ for (struct blockdepthwatch *w = blockdepthwatch_hash_first(blockdepthwatches, &it);
+ w;
+ w = blockdepthwatch_hash_next(blockdepthwatches, &it)) {
+ if (w->height == blockheight
+ && w->depthcb == depthcb
+ && w->reorgcb == reorgcb
+ && w->arg == arg) {
+ return w;
+ }
+ }
+ return NULL;
+}
+
+bool watch_blockdepth_(const tal_t *ctx,
struct chain_topology *topo,
u32 blockheight,
enum watch_result (*depthcb)(struct lightningd *ld, u32 depth, void *),
enum watch_result (*reorgcb)(struct lightningd *ld, void *),
void *arg)
{
- struct blockdepthwatch *w = tal(ctx, struct blockdepthwatch);
+ struct blockdepthwatch *w;
+
+ if (find_blockdepthwatch(topo->blockdepthwatches, blockheight, depthcb, reorgcb, arg))
+ return false;
+
+ w = tal(ctx, struct blockdepthwatch);
w->height = blockheight;
w->depthcb = depthcb;
w->reorgcb = reorgcb;
w->arg = arg;
blockdepthwatch_hash_add(topo->blockdepthwatches, w);
tal_add_destructor2(w, destroy_blockdepthwatch, topo);
+ return true;
}
void watch_check_block_added(const struct chain_topology *topo, u32 blockheight)
diff --git a/lightningd/watch.h b/lightningd/watch.h
index bd371ef6..ec209a6d 100644
--- a/lightningd/watch.h
+++ b/lightningd/watch.h
@@ -107,8 +107,8 @@ void txwatch_inform(const struct chain_topology *topo,
const struct bitcoin_txid *txid,
struct bitcoin_tx *tx TAKES);
-/* Watch for specific spends to this scriptpubkey */
-void watch_scriptpubkey_(const tal_t *ctx,
+/* Watch for specific spends to this scriptpubkey: returns false if was already watched. */
+bool watch_scriptpubkey_(const tal_t *ctx,
struct chain_topology *topo,
const u8 *scriptpubkey TAKES,
const struct bitcoin_outpoint *expected_outpoint,
@@ -133,7 +133,7 @@ void watch_scriptpubkey_(const tal_t *ctx,
bool unwatch_scriptpubkey_(const tal_t *ctx,
struct chain_topology *topo,
- const u8 *scriptpubkey TAKES,
+ const u8 *scriptpubkey,
const struct bitcoin_outpoint *expected_outpoint,
struct amount_sat expected_amount,
void (*cb)(struct lightningd *ld,
@@ -154,8 +154,8 @@ bool unwatch_scriptpubkey_(const tal_t *ctx,
const struct txlocator *), \
(arg))
-/* Watch for this block getting deeper (or reorged out) */
-void watch_blockdepth_(const tal_t *ctx,
+/* Watch for this block getting deeper (or reorged out). Returns false it if was a duplicate. */
+bool watch_blockdepth_(const tal_t *ctx,
struct chain_topology *topo,
u32 blockheight,
enum watch_result (*depthcb)(struct lightningd *ld, u32 depth, void *),
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.