multi: enable closed-channel tombstones on KV-SQL backends
What changed, and why it matters
This change adjusts how LND closes payment channels when using SQLite or Postgres database backends. Instead of immediately deleting all channel-related data in one big operation, it marks the channel as closed with a 'tombstone' and leaves the old data in place temporarily. This avoids long database lockups during channel closure. It is a performance and reliability improvement, not a fix for an exploitable vulnerability.
Treat as a normal code review / reliability change. Verify that tombstoned closed-channel data is eventually reclaimed by the planned native-SQL migration and that backup, privacy, and operational runbooks account for the new retention behavior on sqlite/postgres backends. No emergency security action is indicated by this commit alone.
Security signals we found
Behavioral change in channel close state cleanup
Intentional retention of closed-channel data on sqlite/postgres backends
Potential for data retention expectations to differ by backend
No input validation, cryptographic, or authorization changes observed
Evidence from the diff
The commit wires channeldb.OptionTombstoneClosedChannels for KV-over-SQL (sqlite, postgres) backends in config_builder.go. bbolt and etcd retain the existing synchronous nested-bucket delete path. The tombstone path intentionally preserves per-channel forwarding-package and revocation-log state on disk until a future native-SQL channel-state migration reclaims it. The integration test harness gains UsesClosedChanTombstones() so backend-symmetric tests can skip immediate post-close deletion assertions on sqlite/postgres while still exercising the close flow.
Changed components
lnd/config_builder.golnd/channeldb (via OptionTombstoneClosedChannels)lnd/itest/lnd_wipe_fwdpkgs_test.golnd/lntest/harness.goInspect captured patch +44 / −7
diff --git a/config_builder.go b/config_builder.go
index 394d379..8da8b39 100644
--- a/config_builder.go
+++ b/config_builder.go
@@ -1072,6 +1072,15 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
)
}
+ // KV-over-SQL backends (sqlite, postgres) opt in to closing channels
+ // via tombstone markers because nested-bucket deletes inside a write
+ // transaction translate into a long-running ON DELETE CASCADE on the
+ // kvdb-on-SQL schema, holding the database write-lock for many seconds
+ // on long-lived channels. bbolt and etcd keep the synchronous one-shot
+ // close path, where nested-bucket deletion is already cheap.
+ tombstoneClosedChans := cfg.DB.Backend == lncfg.SqliteBackend ||
+ cfg.DB.Backend == lncfg.PostgresBackend
+
dbOptions := []channeldb.OptionModifier{
channeldb.OptionDryRunMigration(cfg.DryRunMigration),
channeldb.OptionStoreFinalHtlcResolutions(
@@ -1081,6 +1090,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
channeldb.OptionNoRevLogAmtData(cfg.DB.NoRevLogAmtData),
channeldb.OptionGcDecayedLog(cfg.DB.NoGcDecayedLog),
channeldb.OptionWithDecayedLogDB(dbs.DecayedLogDB),
+ channeldb.OptionTombstoneClosedChannels(tombstoneClosedChans),
}
// Otherwise, we'll open two instances, one for the state we only need
diff --git a/itest/lnd_wipe_fwdpkgs_test.go b/itest/lnd_wipe_fwdpkgs_test.go
index 337331f..d0c6695 100644
--- a/itest/lnd_wipe_fwdpkgs_test.go
+++ b/itest/lnd_wipe_fwdpkgs_test.go
@@ -85,13 +85,28 @@ func testWipeForwardingPackages(ht *lntest.HarnessTest) {
// close channel should now become pending force closed channel.
pendingAB = ht.AssertChannelPendingForceClose(bob, chanPointAB).Channel
- // Check the forwarding pacakges are deleted.
- require.Zero(ht, pendingAB.NumForwardingPackages)
-
- // For Alice, the forwarding packages should have been wiped too.
- pending := ht.AssertChannelPendingForceClose(alice, chanPointAB)
- pendingAB = pending.Channel
- require.Zero(ht, pendingAB.NumForwardingPackages)
+ // On backends that close channels via tombstone markers (sqlite,
+ // postgres), the per-channel forwarding-package bucket is left on
+ // disk by design — the synchronous close path's nested-bucket
+ // delete is exactly what tombstoning avoids. The bytes are reclaimed
+ // by the upcoming native-SQL channel-state migration. The unit-test
+ // suite in channeldb covers the tombstone semantics directly, so
+ // here we just skip the post-close fwd-pkg assertions on those
+ // backends while still exercising the rest of the close flow for
+ // backend symmetry.
+ if !ht.UsesClosedChanTombstones() {
+ require.Zero(ht, pendingAB.NumForwardingPackages)
+
+ // For Alice, the forwarding packages should have been wiped
+ // too.
+ pending := ht.AssertChannelPendingForceClose(alice, chanPointAB)
+ pendingAB = pending.Channel
+ require.Zero(ht, pendingAB.NumForwardingPackages)
+ } else {
+ // Still drive Alice's pending-force-close lookup so the rest
+ // of the test stays backend-symmetric.
+ ht.AssertChannelPendingForceClose(alice, chanPointAB)
+ }
// Alice should one pending sweep.
ht.AssertNumPendingSweeps(alice, 1)
diff --git a/lntest/harness.go b/lntest/harness.go
index 08c62a9..62cbac3 100644
--- a/lntest/harness.go
+++ b/lntest/harness.go
@@ -1444,6 +1444,18 @@ func (h *HarnessTest) IsPostgresBackend() bool {
return h.manager.dbBackend == node.BackendPostgres
}
+// UsesClosedChanTombstones reports whether the test harness's database
+// backend closes channels via tombstone markers rather than cascading the
+// nested-bucket delete. This is true on the KV-over-SQL backends (sqlite,
+// postgres) and false on bbolt. Tests that observe forwarding-package or
+// revocation-log deletion immediately after a channel close should consult
+// this predicate; on tombstone backends the bulk state remains on disk
+// until the upcoming native-SQL channel-state migration reclaims it.
+func (h *HarnessTest) UsesClosedChanTombstones() bool {
+ return h.manager.dbBackend == node.BackendSqlite ||
+ h.manager.dbBackend == node.BackendPostgres
+}
+
// fundCoins attempts to send amt satoshis from the internal mining node to the
// targeted lightning node. The confirmed boolean indicates whether the
// transaction that pays to the target should confirm. For neutrino backend,
Why this scored 26/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.