|
| 1 | +# Minimal Safe Failover for LDK Node |
| 2 | + |
| 3 | +The minimal safe implementation makes `PostgresStore` always leased, with node integration automatic for the built-in PostgreSQL builder and process termination and restart handled by ldk-server and its supervisor. It requires no rust-lightning changes, no PostgreSQL advisory locks, no standby state machine, and no extension of the generic `KVStore` interface. |
| 4 | + |
| 5 | +## Scope and assumptions |
| 6 | + |
| 7 | +- Support only the built-in PostgreSQL store. Generic user-provided stores are out of scope. |
| 8 | +- Require one PostgreSQL KV table per node identity. Multiple nodes may share a database when each uses a distinct `kv_table_name`; each KV table has its own companion lease table. |
| 9 | +- Assume every safety-critical external effect has an immediately preceding fenced PostgreSQL mutation and completes within the lease interval renewed by that mutation. An unbounded pause between the committed mutation and the external effect is not protected by this design. |
| 10 | +- All processes using a node identity must run the failover-aware implementation. An older process that ignores the lease can still corrupt state. |
| 11 | +- The application or process supervisor is responsible for retrying construction after `BuildError::NodeLeaseUnavailable`. ldk-node does not keep a deserialized standby node in memory. |
| 12 | + |
| 13 | +## 1. Add an internal lease primitive |
| 14 | + |
| 15 | +Add `src/io/node_lease.rs` containing: |
| 16 | + |
| 17 | +- A random owner ID generated for each `PostgresStore` construction attempt. |
| 18 | +- A shared `lease_lost: AtomicBool`, initialized to `false`, with a single terminal transition to `true`. |
| 19 | +- A fixed initial lease duration and renewal interval, for example 30 seconds and 10 seconds. |
| 20 | +- A notification that becomes permanently signalled when leadership is lost. |
| 21 | +- The owner ID is held by `NodeLease`; renewal cancellation and best-effort clean release are owned by `PostgresStore`. They do not require additional lease states. |
| 22 | +- No automatic reacquisition. Once a `Node` loses its lease, the process must terminate and construct a new `Node` after restart. |
| 23 | + |
| 24 | +Keep this crate-internal initially. Configuration can be added later if operational experience shows the default timings are insufficient. |
| 25 | + |
| 26 | +## 2. Implement PostgreSQL lease rows |
| 27 | + |
| 28 | +Change `src/io/postgres_store/mod.rs` so every constructed store owns a lease for its KV table. |
| 29 | + |
| 30 | +When `PostgresStore` creates its PostgreSQL KV table, also create an ordinary companion lease table in the same schema, not an advisory lock. Derive its name from the effective `kv_table_name`, including the default. For example, `ldk_data` uses `ldk_data_node_lease`, and `alice.ldk_data` uses `alice.ldk_data_node_lease`. |
| 31 | + |
| 32 | +```sql |
| 33 | +CREATE TABLE IF NOT EXISTS <kv_table_name>_node_lease ( |
| 34 | + id SMALLINT PRIMARY KEY CHECK (id = 1), |
| 35 | + owner_id BYTEA NOT NULL, |
| 36 | + expires_at TIMESTAMPTZ NOT NULL |
| 37 | +); |
| 38 | +``` |
| 39 | + |
| 40 | +Each companion table contains exactly one row with `id = 1`. It leases only the node state in its corresponding KV table. There is no shared lease table and no `store_key` column. |
| 41 | + |
| 42 | +Validate that the derived PostgreSQL identifier fits PostgreSQL's identifier length limit rather than allowing truncation to make two companion table names collide. |
| 43 | + |
| 44 | +Lease acquisition should atomically insert the singleton row or replace it only when `expires_at <= clock_timestamp()`. PostgreSQL time is authoritative, so failover does not depend on clock synchronization between nodes. |
| 45 | + |
| 46 | +Run renewal on the store's existing private runtime. Renewal must update the row only when: |
| 47 | + |
| 48 | +```text |
| 49 | +id is 1 |
| 50 | +owner_id matches |
| 51 | +the existing lease has not already expired |
| 52 | +``` |
| 53 | + |
| 54 | +A timeout, ownership mismatch, expired lease, or ambiguous database result eventually marks the lease permanently lost. The same `Node` must never reacquire it. |
| 55 | + |
| 56 | +## 3. Fence every PostgreSQL mutation |
| 57 | + |
| 58 | +Modify `write_internal` and `remove_internal` so each mutation runs in a database transaction: |
| 59 | + |
| 60 | +1. Atomically lock and update the singleton row only if the owner ID matches and the lease remains unexpired. |
| 61 | +2. Refresh `expires_at`, giving the following critical operation a full lease interval. |
| 62 | +3. Perform the KV write or removal. |
| 63 | +4. Commit. |
| 64 | + |
| 65 | +Holding the row lock ensures a fallback cannot take over while an old leader's persistence transaction is still executing. |
| 66 | + |
| 67 | +Reads and list operations do not need transaction-level fencing because: |
| 68 | + |
| 69 | +- Node construction acquires the lease before reading live state. |
| 70 | +- Runtime reads have no external effect. |
| 71 | +- All state mutations and assumed critical operations are fenced separately. |
| 72 | + |
| 73 | +Missing or expired ownership marks the lease lost inside `PostgresStore` before returning a persistence error. Database errors remain ordinary persistence errors while the conservative local renewal deadline remains valid; once that deadline has elapsed, an ambiguous PostgreSQL operation marks the lease lost before returning the error. |
| 74 | + |
| 75 | +## 4. Acquire before deserializing the node |
| 76 | + |
| 77 | +Change the built-in PostgreSQL path in `src/builder.rs`: |
| 78 | + |
| 79 | +1. Construct `PostgresStore`, which acquires the lease and starts renewing it before returning. |
| 80 | +2. Only then build the node through the ordinary store path. |
| 81 | +3. Install the store's lease handle on the fully constructed node before returning it. Check for loss before and after installing the containment handler. |
| 82 | + |
| 83 | +Acquiring in `Node::start()` is too late because `build_with_store_internal` already reads and may migrate persisted wallet and LDK state. |
| 84 | + |
| 85 | +Add: |
| 86 | + |
| 87 | +```rust |
| 88 | +BuildError::NodeLeaseUnavailable |
| 89 | +``` |
| 90 | + |
| 91 | +This lets a supervisor distinguish normal standby contention from database setup or corrupted-state failures. |
| 92 | + |
| 93 | +Leave `build_with_store` unchanged. Directly constructed `PostgresStore` instances still fence their mutations, but the generic builder path does not install node-level lease-loss containment or notification. Applications needing failover must use `build_with_postgres_store`. Adding a new generic lease trait is unnecessary for the first implementation. |
| 94 | + |
| 95 | +## 5. Treat runtime lease loss as process-fatal |
| 96 | + |
| 97 | +Add the lease handle to `Node` in `src/lib.rs`. |
| 98 | + |
| 99 | +Required semantics: |
| 100 | + |
| 101 | +- Confirmed ownership loss, confirmed expiration, or failure to renew before the conservative lease deadline atomically changes `lease_lost` from `false` to `true`. This transition is one-way. |
| 102 | +- `ensure_lease_active()` checks at API entry are not a safety boundary because leadership can be lost immediately afterward. Do not add them to public operations. |
| 103 | +- Every leased PostgreSQL write and removal is the authoritative fence. It locks and validates the companion lease row, renews it, performs the KV mutation, and commits in one transaction. |
| 104 | +- `PostgresStore` calls the one-way `mark_lost()` before returning a persistence error whenever ownership is missing, expired, or ambiguous after the conservative local deadline. |
| 105 | +- The transition stops background processing, disconnects peers, and only then notifies ldk-server. This containment begins even when an intermediate caller converts or ignores the persistence error. |
| 106 | +- The application must treat the notification as fatal and terminate the process immediately, without calling `Node::stop()`. The same process must never use or restart that `Node` after lease loss. |
| 107 | +- Higher layers continue to expose `Error::PersistenceFailed` for the failed operation. Safety does not depend on classifying lease loss in the public error enum or changing the node's ordinary running state. |
| 108 | +- The PostgreSQL store rejects every later write and removal after the terminal transition. Reads remain unfenced. |
| 109 | +- Expose a lease-loss notification, such as `Node::wait_for_lease_loss()`, for ldk-server. Do not use the persistent event queue for this notification because persistence is no longer available after lease loss. |
| 110 | +- Correctness does not depend on how quickly the application handles the notification. The store has already rejected further mutations; prompt process exit limits unfenced external activity after loss. |
| 111 | +- `Node::stop()` does not release the lease. The current API permits stopping and restarting the same already-deserialized `Node`; releasing at `stop()` would make a later restart unsafe. |
| 112 | +- Dropping the `Node` performs a best-effort conditional lease release after normal shutdown. If release fails, the fallback waits for expiration. |
| 113 | +- Planned handoff therefore means `stop()`, then drop all references to the `Node`. |
| 114 | +- Process termination after lease loss skips destructors, final persistence, lease release, and reacquisition. |
| 115 | + |
| 116 | +## 6. Terminate and restart in ldk-server |
| 117 | + |
| 118 | +ldk-server waits for `Node::wait_for_lease_loss()` in a prioritized select branch. When notified, it immediately calls `std::process::exit(1)` without calling `Node::stop()` or dropping the node. Its process supervisor then starts a new ldk-server process, which constructs a fresh `Node` from PostgreSQL. |
| 119 | + |
| 120 | +Expose an optional `kv_table_name` in ldk-server's PostgreSQL configuration and pass it to the existing PostgreSQL build method. Repeated process starts by the supervisor call that build method with the same database and KV table configuration: |
| 121 | + |
| 122 | +```text |
| 123 | +NodeLeaseUnavailable => wait and retry |
| 124 | +success => start the newly deserialized Node |
| 125 | +other error => report/fail |
| 126 | +``` |
| 127 | + |
| 128 | +This avoids introducing a standby `Node`, a `wait_for_leadership` API, or state reload logic. A node is only constructed after it has leadership. ldk-node never terminates the host process itself because it is a library; ldk-server owns that policy. |
| 129 | + |
| 130 | +This does not make a committed database transaction atomic with a later peer message or Bitcoin transaction broadcast. A process can pause after the fenced commit and resume after takeover. Supporting arbitrary pauses would require fencing at the external system itself, which Lightning peers and Bitcoin transaction broadcast generally do not provide. |
| 131 | + |
| 132 | +## 7. Tests |
| 133 | + |
| 134 | +Add PostgreSQL integration tests covering: |
| 135 | + |
| 136 | +- A second builder receives `NodeLeaseUnavailable`. |
| 137 | +- Builders for distinct KV table names in the same database can acquire leases independently. |
| 138 | +- Exactly one contender acquires an expired lease. |
| 139 | +- A fallback acquires after renewal stops and the lease expires. |
| 140 | +- The original store cannot write or remove after fallback takeover. |
| 141 | +- A lost original node cannot renew or reacquire, even if database connectivity returns. |
| 142 | +- Lease loss runs the containment handler before its notification is delivered and before the failed operation returns its ordinary persistence error. |
| 143 | +- A lost store rejects all later writes and removals. |
| 144 | +- ldk-server exits without final persistence or conditional lease release. |
| 145 | +- ldk-server receives the lease-loss notification and exits with a failure status. |
| 146 | +- Dropping the leader permits immediate clean takeover. |
| 147 | +- `stop()` without dropping continues to hold the lease. |
| 148 | +- A build failure after acquisition releases the lease or leaves it to expire safely. |
| 149 | +- Connection loss and an ambiguous renewal result fail closed. |
| 150 | +- A KV mutation and takeover racing at expiration cannot both succeed out of order. |
0 commit comments