actor: wire up dead-letter handling with requeue, monitor, and operator surface - #1119
actor: wire up dead-letter handling with requeue, monitor, and operator surface#1119Roasbeef wants to merge 8 commits into
Conversation
|
@claude review this |
|
Claude finished @Roasbeef's task in 4m 0s —— View job Dead-letter handling reviewReviewed the DB layer (migration, queries,
Findings1. Monitor can silently miss dead letters that share a one-second boundary (medium)
-- db/actordelivery/queries/mailbox.sql:399
SELECT * FROM dead_letters
WHERE created_at >= $1
ORDER BY created_at ASC, id ASC
LIMIT $2;
Impact is bounded to the observation path: the Fix: cursor on 2. Per-actor metric label is an unbounded-cardinality risk (medium)Both 3. Error-level logging of dead letters vs. the project convention (discuss)
4.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42510e1457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A full batch may hide more rows behind it; continue the | ||
| // drain from the newest boundary we have recorded. | ||
| since = time.Unix(m.watermark, 0) |
There was a problem hiding this comment.
Advance scans with a composite cursor
When at least 100 dead letters share one created_at second, as can happen during a poison-message storm, this leaves since on the same inclusive boundary. ListDeadLettersSince then returns the identical first 100 rows on every batch and future tick; seen filters those rows but never lets the query reach the remaining or any newer entries, so their logs and counter hooks are permanently missed. Paginate by (created_at, id) or an equivalent keyset cursor rather than seconds alone.
AGENTS.md reference: baselib/actor/AGENTS.md:L42-L46
Useful? React with 👍 / 👎.
| cutoff := r.server.clk.Now().Add( | ||
| -time.Duration(req.OlderThanSeconds) * time.Second, | ||
| ) |
There was a problem hiding this comment.
Reject purge ages that overflow time.Duration
When a direct REST/gRPC caller supplies a positive age greater than math.MaxInt64 / int64(time.Second) (about 292 years), the conversion and multiplication wrap instead of representing an old cutoff. For example, math.MaxInt64 produces a cutoff one second in the future, causing PurgeDeadLetters to delete the entire queue even though the request specified an extremely old age. Reject values beyond the representable duration or calculate the cutoff with explicit overflow checks.
Useful? React with 👍 / 👎.
| cutoff := m.cfg.Clock.Now().Add(-m.cfg.Retention) | ||
|
|
||
| removed, err := m.cfg.Store.PurgeDeadLetters(m.ctx, cutoff) |
There was a problem hiding this comment.
Observe dead letters before applying retention
When Retention is shorter than ScanInterval and the hourly sweep falls between scan ticks—for example, a 17-second scan interval and 1-second retention—a row parked after the preceding scan can satisfy this cutoff and be deleted before the next scan invokes its log and hook. The message therefore never contributes to the observation counter, violating the monitor's exactly-once surfacing contract; scan through the cutoff before purging or otherwise prevent retention from removing unobserved rows.
AGENTS.md reference: baselib/actor/AGENTS.md:L42-L49
Useful? React with 👍 / 👎.
The dead_letters projection dropped every routing field of the source mailbox message: the ask plumbing (promise_id, callback_actor_id, correlation_id), the per-key FIFO tag, the priority, and the retry budget. A dead letter could be inspected but never faithfully reconstructed as a mailbox message, which forecloses the requeue path issue #705 calls for. In this commit, we add the missing columns to dead_letters via a new actor-delivery migration and widen MoveMailboxToDeadLetter to project them from the mailbox row. All new columns are nullable or defaulted, so rows dead-lettered before the migration remain readable and requeue falls back to the same defaults a fresh enqueue would get. The query surface also grows the operations the operator loop needs: a global ListDeadLetters with offset pagination, ListDeadLettersSince for the monitor's incremental scan, CountDeadLettersByActor for per-actor metrics, and CleanupOldDeadLetters now reports the number of rows the retention sweep removed. CountDeadLetters and the cleanup query existed in SQL but were unreachable from Go; the next commit lifts them into the store interface.
We have a full dead-letter store but no dead-letter handling: once the retry policy gives up, a message lands in dead_letters and nothing can ever act on it again. This commit adds the store half of the recovery path called for by #705. A new actor.DeadLetterStore interface carries the operator-facing operations: global and incremental enumeration, per-actor counts, atomic requeue, and a retention purge. It is deliberately separate from DeliveryStore, since the actor runtime only ever writes dead letters; widening DeliveryStore would force every test double in the tree to grow operations the runtime never calls. RequeueDeadLetter re-enqueues the original payload as a NEW mailbox message inside one transaction: fresh UUIDv7 ID, attempts reset, available immediately, and every routing field (priority, retry budget, ask plumbing, correlation key) preserved via the columns the previous commit added. The fresh ID is load-bearing rather than cosmetic. The retry-exhaustion path records the original ID in processed_messages before dead-lettering, so a same-ID requeue would be silently skipped by deduplication, and the enqueue's ON CONFLICT DO NOTHING would mask the collision. The ID is minted outside the transaction closure so a retried transaction re-inserts the same row instead of a second one. After commit, the store fires the targeted mailbox wake so a resident consumer picks the message up without waiting out its poll interval. The three dead-letter queries that existed in SQL but were unreachable from Go (CountDeadLetters, CountDeadLettersByActor via the new grouped query, CleanupOldDeadLetters) are lifted into the querier interface, and the row-to-projection mapping is centralized in deadLetterFromRow so the new routing fields flow through every read path.
A dead letter had no owner: once written, nothing in the process ever listed, logged, counted, or deleted it. This commit adds the observability and retention half of the dead-letter contract as a small background service in the actor framework, following the OutboxPublisher lifecycle shape. The monitor scans the store on an interval and surfaces each newly parked dead letter exactly once: a bounded number of detail log lines (a poison storm parks many at once, and one summary line carries the same alerting weight as a hundred detail lines), plus an OnDeadLetter hook the daemon points at metrics. The detail lines log at error level deliberately. The daemon durably accepted these messages and has now abandoned them after exhausting retries; whatever triggered the individual failures, dropping an accepted message is a breach of the delivery contract that requires operator action. The since-scan boundary is inclusive at second granularity, so the monitor keeps a boundary set of already-reported IDs and only prunes entries once the watermark moves strictly past their second. A restart summarizes the pre-existing backlog with a count instead of re-reporting each entry. The sweep applies the retention policy on a slower cadence. Retention defaults to disabled: dead letters can carry value-bearing messages, so aging them out is an explicit operator opt-in, never a default. The sweep also drives the store's CleanupExpired, which removes expired dedup entries and ask results and previously had no periodic caller anywhere in the daemon.
42510e1 to
d8ecf4a
Compare
In this commit, we wire the dead-letter monitor into the daemon and give operators the alerting surface issue #705 asks for. The monitor starts right after database initialization, since it depends only on the delivery store, and runs until shutdown independent of wallet or server-connection state -- dead letters accrue from any durable actor. Two knobs land under the deadletter.* config namespace: the scan interval (default 30s) and the retention window, which defaults to disabled so parked value-bearing messages are never silently aged out. Three metrics cover the two failure shapes. The scrape-driven waved_dead_letters gauge (explicitly zero when clean, so the series' absence means metrics-off rather than all-clear) and its per-actor waved_actor_dead_letters breakdown report what is parked right now; the monitor-driven waved_actor_dead_letters_observed_total counter is the monotone history that catches entries parked and cleared between scrapes. The metrics README gains a section mapping each expression to the failure it detects, mirroring the ingress-liveness alerting section from the serverconn backpressure work.
In this commit, we add the operator surface for dead-lettered actor messages to the DaemonService: ListDeadLetters (global or per-actor, paginated, payloads opt-in), GetDeadLetter (payload included), RequeueDeadLetter, and PurgeDeadLetters. Purge takes a mandatory positive age rather than an optional cutoff, so an empty request can never wholesale-delete the queue. The REST gateway selectors and regenerated stubs (including the devrpc registry) ride along.
The handlers follow the vHTLC-recovery shape: a nil-guard helper that returns Unavailable until the delivery store exists, typed store errors mapped onto gRPC codes (not-found -> NotFound, non-mailbox source -> FailedPrecondition), and an info-level log line on each mutating operator action. The hand-written REST client grows the four matching methods so channel-agnostic callers keep working.
The deadletter subtree joins the dev-gated advanced group next to recovery, with the same posture: hidden from default --help, always runnable, and absent from the schema/MCP surfaces. list/inspect are read-only; requeue and purge follow the recovery-escalate consent contract, requiring --yes on non-interactive stdin so an agent can never hang on a y/N prompt or mutate the queue by accident. The purge verb additionally requires an explicit --older-than duration, mirroring the RPC's refusal to purge without a positive age.
The architecture doc gains a Dead-Letter Contract section covering the four framework paths that write dead letters, the observe/recover/ retain ownership split between the monitor and the operator surface, the fresh-ID requeue semantics and why they are a correctness requirement, and how the durable contract relates to the weaker in-memory DLO ref (which logs and drops, and is advisory only) -- the per-actor-class clarification #705 asks for. The schema doc picks up the widened dead_letters projection, and the per-package CLAUDE/AGENTS pairs pick up the new store surface, monitor, metrics, and CLI subtree.
d8ecf4a to
1584a2c
Compare
|
Addressed the review findings (plus an independent adversarial audit that flagged the same top two): #1 (same-second scan livelock): fixed with strict #2 (counter cardinality): the observed counter is now unlabelled ( #3 (error-level logging): keeping error level deliberately, with the rationale in the code comment: the daemon durably accepted these messages and then abandoned them, and dropping an accepted message is a breach of the delivery contract requiring operator action regardless of what triggered the individual retries (same posture as the #1093 ingress-deferral episodes). Happy to demote to warn if the maintainer prefers the strict reading of the log-level rule. #4: #5: #6: migrations CLAUDE/AGENTS updated for Also from the independent audit: a purge-age bound ( |
|
Doc advisory: this PR's Go changes leave the per-package docs for What's missing
diff --git a/cmd/wavecli/waveclicommands/AGENTS.md b/cmd/wavecli/waveclicommands/AGENTS.md
index 393611d0..ae6a1822 100644
--- a/cmd/wavecli/waveclicommands/AGENTS.md
+++ b/cmd/wavecli/waveclicommands/AGENTS.md
@@ -16,15 +16,15 @@ default `--help` face:
wavewalletrpc-backed.
2. **Daemon introspection (group "Introspection")** — getinfo, schema, mcp
(the built-in `help` command is grouped here too).
-3. **Advanced subtrees (`ark`, `dev`, `recovery`)** — raw
+3. **Advanced subtrees (`ark`, `dev`, `recovery`, `deadletter`)** — raw
waverpc/devrpc commands for power users and operator runbooks.
Hidden from the default `--help` via cobra `Hidden` (not a build tag),
so they stay compiled and fully runnable in the shipped binary;
`WAVELENGTH_DEV=1` reveals them under an "Advanced" group. The env var only
changes visibility — it never gates execution. `ark.*` stays on the
`schema`/MCP surfaces; `dev` is reachable only as the generated (hidden)
- CLI subtree — it is not registered in `schema`/MCP — and `recovery` is
- exposed on neither.
+ CLI subtree — it is not registered in `schema`/MCP — and `recovery` and
+ `deadletter` are exposed on neither.
The `swap.*` verbs were retired: `send`/`recv --offchain` and `activity`
cover them (the swapruntime daemon runtime that powers those verbs is
@@ -148,7 +148,8 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/cmd/wave
- **Depends on**:
- `rpc/wavewalletrpc` (generated stubs for the top-level wallet verbs;
`WalletService` / `WalletInspectionService` clients).
- - `waverpc` (generated stubs for `ark.*`, `recovery.*`, getinfo).
+ - `waverpc` (generated stubs for `ark.*`, `recovery.*`, `deadletter.*`,
+ getinfo).
- `cmd/wavecli/waveclicommands/devrpc` (the generated `dev`
subtree; its registry references `swapclientrpc` service
descriptors).
@@ -190,6 +191,12 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/cmd/wave
- `recovery escalate` refuses to run on non-interactive stdin unless
`--yes` is passed — it never blocks on a y/N prompt an agent can't
answer.
+- `deadletter requeue` and `deadletter purge` carry the same posture: both
+ prompt on a TTY and exit with the confirmation-required code on
+ non-interactive stdin without `--yes`. `purge` additionally requires an
+ explicit positive `--older-than`; the flag defaults to zero and a zero or
+ negative duration is rejected client-side, so an operator can never
+ wholesale-delete the queue by omitting an argument.
- `ark vtxos refresh` is gated on fee consent: a real refresh fetches
the dry-run estimate and prompts with it on a TTY, and refuses on
non-interactive stdin without `--yes` (same posture as `leave --all`
diff --git a/cmd/wavecli/waveclicommands/CLAUDE.md b/cmd/wavecli/waveclicommands/CLAUDE.md
index 393611d0..ae6a1822 100644
--- a/cmd/wavecli/waveclicommands/CLAUDE.md
+++ b/cmd/wavecli/waveclicommands/CLAUDE.md
@@ -16,15 +16,15 @@ default `--help` face:
wavewalletrpc-backed.
2. **Daemon introspection (group "Introspection")** — getinfo, schema, mcp
(the built-in `help` command is grouped here too).
-3. **Advanced subtrees (`ark`, `dev`, `recovery`)** — raw
+3. **Advanced subtrees (`ark`, `dev`, `recovery`, `deadletter`)** — raw
waverpc/devrpc commands for power users and operator runbooks.
Hidden from the default `--help` via cobra `Hidden` (not a build tag),
so they stay compiled and fully runnable in the shipped binary;
`WAVELENGTH_DEV=1` reveals them under an "Advanced" group. The env var only
changes visibility — it never gates execution. `ark.*` stays on the
`schema`/MCP surfaces; `dev` is reachable only as the generated (hidden)
- CLI subtree — it is not registered in `schema`/MCP — and `recovery` is
- exposed on neither.
+ CLI subtree — it is not registered in `schema`/MCP — and `recovery` and
+ `deadletter` are exposed on neither.
The `swap.*` verbs were retired: `send`/`recv --offchain` and `activity`
cover them (the swapruntime daemon runtime that powers those verbs is
@@ -148,7 +148,8 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/cmd/wave
- **Depends on**:
- `rpc/wavewalletrpc` (generated stubs for the top-level wallet verbs;
`WalletService` / `WalletInspectionService` clients).
- - `waverpc` (generated stubs for `ark.*`, `recovery.*`, getinfo).
+ - `waverpc` (generated stubs for `ark.*`, `recovery.*`, `deadletter.*`,
+ getinfo).
- `cmd/wavecli/waveclicommands/devrpc` (the generated `dev`
subtree; its registry references `swapclientrpc` service
descriptors).
@@ -190,6 +191,12 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/cmd/wave
- `recovery escalate` refuses to run on non-interactive stdin unless
`--yes` is passed — it never blocks on a y/N prompt an agent can't
answer.
+- `deadletter requeue` and `deadletter purge` carry the same posture: both
+ prompt on a TTY and exit with the confirmation-required code on
+ non-interactive stdin without `--yes`. `purge` additionally requires an
+ explicit positive `--older-than`; the flag defaults to zero and a zero or
+ negative duration is rejected client-side, so an operator can never
+ wholesale-delete the queue by omitting an argument.
- `ark vtxos refresh` is gated on fee consent: a real refresh fetches
the dry-run estimate and prompts with it on a TTY, and refuses on
non-interactive stdin without `--yes` (same posture as `leave --all`
diff --git a/waved/AGENTS.md b/waved/AGENTS.md
index b778f21c..202b3ee1 100644
--- a/waved/AGENTS.md
+++ b/waved/AGENTS.md
@@ -11,7 +11,8 @@ system with a gRPC API.
For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<Symbol>`.
- `Server` — main daemon. Owns the wallet, DB, chainsource actor, gRPC
- server, and `ActorSystem`. Caches `localMailboxID` (pubkey-derived),
+ server, `ActorSystem`, the `outboxPublisher`, and the
+ `deadLetterMonitor`. Caches `localMailboxID` (pubkey-derived),
`authSigHex` (Schnorr auth), and a single `clk` (`clock.Clock`) shared by
all sub-stores for deterministic time injection.
- `RPCServer` — implements the gRPC `DaemonService`. Most write RPCs
@@ -19,12 +20,22 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<S
validate input locally then `Ask` the relevant actor; `GetRound` and
`ListVTXOs` merge live actor state with persisted SQL rows, while
`GetFeeHistory` and `ListTransactions` are pure SQL reads
- (`rpc_fees.go`).
+ (`rpc_fees.go`). The dead-letter operator RPCs (`ListDeadLetters`,
+ `GetDeadLetter`, `RequeueDeadLetter`, `PurgeDeadLetters` in
+ `rpc_deadletter.go`) read and mutate the delivery store's
+ `actor.DeadLetterStore` surface directly, bypassing the actor system.
- `Config` — daemon configuration: wallet backend selection, mailbox/chain
backend wiring, `OORConfig`/`OORLimitsConfig` (receive safety caps),
- `UnrollConfig` (unilateral-exit fee-bump cadence and cap), and
+ `UnrollConfig` (unilateral-exit fee-bump cadence and cap),
+ `DeadLetterConfig` (dead-letter monitor cadence and retention), and
`MaxOperatorFeeSat` (the #270 seal-time fee-cap validated in
`Config.Validate()`).
+- `DeadLetterConfig` — value-typed dead-letter monitor knobs
+ (`deadletter.scan-interval`, `deadletter.sweep-interval`,
+ `deadletter.retention`). Zero scan/sweep intervals fall back to the
+ monitor's defaults (30s / 1h); zero retention is the default and disables
+ the purge entirely. All three are rejected by `Config.Validate()` when
+ negative.
- `WalletState` — `None` / `Locked` / `Ready` wallet lifecycle.
- `UnrollConfig` / `OORConfig` — subsystem tunables; see `Config.Validate()`
for the invariants each enforces.
@@ -43,6 +54,24 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<S
- `Server.run` registers a deferred `actorSystem.Shutdown()` **before** the
deferred `db.Close()` so in-flight actor DB transactions drain before the
connection pool tears down.
+- The dead-letter monitor (`startDeadLetterMonitor`) starts as soon as the
+ delivery store exists and runs until shutdown, independent of wallet or
+ server-connection state, because dead letters accrue from any durable
+ actor. It fails the daemon start if the delivery store does not implement
+ `actor.DeadLetterStore`. It also owns the only periodic call to the
+ store's expired-entry cleanup (dedup records and ask results), so
+ disabling it strands that GC. Its `OnDeadLetter` hook increments
+ `metrics.DeadLettersObservedTotal`; incrementing that unregistered
+ collector when the metrics server is off is a safe no-op.
+- Dead-letter RPCs live behind the `deadletter` macaroon entity
+ (`rpc_auth.go`): `ListDeadLetters`/`GetDeadLetter` need read,
+ `RequeueDeadLetter`/`PurgeDeadLetters` need write. `ListDeadLetters`
+ clamps `limit` to `maxDeadLetterListLimit = 1000` (default 100) and
+ rejects `offset > 0` combined with an `actor_id` filter, since the
+ per-actor listing has no offset pagination. `PurgeDeadLetters` requires a
+ positive `older_than_seconds` bounded by `maxPurgeAgeSeconds`, so an
+ epoch timestamp passed as an age cannot overflow the duration negative
+ and push the cutoff into the future.
- Wallet transitions `None → Locked → Ready` (or direct to `Ready` if a seed
is provided). Three wallet backends: LND, lightweight (`lwwallet`), or
neutrino-backed (`btcwallet` via `btcwbackend`).
diff --git a/waved/CLAUDE.md b/waved/CLAUDE.md
index b778f21c..202b3ee1 100644
--- a/waved/CLAUDE.md
+++ b/waved/CLAUDE.md
@@ -11,7 +11,8 @@ system with a gRPC API.
For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<Symbol>`.
- `Server` — main daemon. Owns the wallet, DB, chainsource actor, gRPC
- server, and `ActorSystem`. Caches `localMailboxID` (pubkey-derived),
+ server, `ActorSystem`, the `outboxPublisher`, and the
+ `deadLetterMonitor`. Caches `localMailboxID` (pubkey-derived),
`authSigHex` (Schnorr auth), and a single `clk` (`clock.Clock`) shared by
all sub-stores for deterministic time injection.
- `RPCServer` — implements the gRPC `DaemonService`. Most write RPCs
@@ -19,12 +20,22 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<S
validate input locally then `Ask` the relevant actor; `GetRound` and
`ListVTXOs` merge live actor state with persisted SQL rows, while
`GetFeeHistory` and `ListTransactions` are pure SQL reads
- (`rpc_fees.go`).
+ (`rpc_fees.go`). The dead-letter operator RPCs (`ListDeadLetters`,
+ `GetDeadLetter`, `RequeueDeadLetter`, `PurgeDeadLetters` in
+ `rpc_deadletter.go`) read and mutate the delivery store's
+ `actor.DeadLetterStore` surface directly, bypassing the actor system.
- `Config` — daemon configuration: wallet backend selection, mailbox/chain
backend wiring, `OORConfig`/`OORLimitsConfig` (receive safety caps),
- `UnrollConfig` (unilateral-exit fee-bump cadence and cap), and
+ `UnrollConfig` (unilateral-exit fee-bump cadence and cap),
+ `DeadLetterConfig` (dead-letter monitor cadence and retention), and
`MaxOperatorFeeSat` (the #270 seal-time fee-cap validated in
`Config.Validate()`).
+- `DeadLetterConfig` — value-typed dead-letter monitor knobs
+ (`deadletter.scan-interval`, `deadletter.sweep-interval`,
+ `deadletter.retention`). Zero scan/sweep intervals fall back to the
+ monitor's defaults (30s / 1h); zero retention is the default and disables
+ the purge entirely. All three are rejected by `Config.Validate()` when
+ negative.
- `WalletState` — `None` / `Locked` / `Ready` wallet lifecycle.
- `UnrollConfig` / `OORConfig` — subsystem tunables; see `Config.Validate()`
for the invariants each enforces.
@@ -43,6 +54,24 @@ For field-level detail, use `go doc github.com/lightninglabs/wavelength/waved.<S
- `Server.run` registers a deferred `actorSystem.Shutdown()` **before** the
deferred `db.Close()` so in-flight actor DB transactions drain before the
connection pool tears down.
+- The dead-letter monitor (`startDeadLetterMonitor`) starts as soon as the
+ delivery store exists and runs until shutdown, independent of wallet or
+ server-connection state, because dead letters accrue from any durable
+ actor. It fails the daemon start if the delivery store does not implement
+ `actor.DeadLetterStore`. It also owns the only periodic call to the
+ store's expired-entry cleanup (dedup records and ask results), so
+ disabling it strands that GC. Its `OnDeadLetter` hook increments
+ `metrics.DeadLettersObservedTotal`; incrementing that unregistered
+ collector when the metrics server is off is a safe no-op.
+- Dead-letter RPCs live behind the `deadletter` macaroon entity
+ (`rpc_auth.go`): `ListDeadLetters`/`GetDeadLetter` need read,
+ `RequeueDeadLetter`/`PurgeDeadLetters` need write. `ListDeadLetters`
+ clamps `limit` to `maxDeadLetterListLimit = 1000` (default 100) and
+ rejects `offset > 0` combined with an `actor_id` filter, since the
+ per-actor listing has no offset pagination. `PurgeDeadLetters` requires a
+ positive `older_than_seconds` bounded by `maxPurgeAgeSeconds`, so an
+ epoch timestamp passed as an age cannot overflow the duration negative
+ and push the cutoff into the future.
- Wallet transitions `None → Locked → Ready` (or direct to `Ready` if a seed
is provided). Three wallet backends: LND, lightweight (`lwwallet`), or
neutrino-backed (`btcwallet` via `btcwbackend`).
diff --git a/waverpc/AGENTS.md b/waverpc/AGENTS.md
index 608abb8e..2b878fcf 100644
--- a/waverpc/AGENTS.md
+++ b/waverpc/AGENTS.md
@@ -2,10 +2,11 @@
## Purpose
-Daemon gRPC API definitions for wallet, boarding, round, OOR, unroll, and
-VHTLC-recovery operations. Proto source: `waverpc/daemon.proto`. Generated
-gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
-(`errors.go`) for structured wallet-lifecycle errors.
+Daemon gRPC API definitions for wallet, boarding, round, OOR, unroll,
+VHTLC-recovery, and dead-letter operator operations. Proto source:
+`waverpc/daemon.proto`. Generated gRPC, REST-gateway, and mailbox-RPC stubs
+plus one hand-written helper file (`errors.go`) for structured
+wallet-lifecycle errors.
## Key Types
@@ -19,6 +20,14 @@ gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
- `IsWalletNotReadyError(err)` / `WalletNotReadyState(err)` — Match and unpack
the structured error produced above; callers should key off these instead of
matching on message text.
+- `DeadLetter` and the `ListDeadLetters` / `GetDeadLetter` /
+ `RequeueDeadLetter` / `PurgeDeadLetters` request/response pairs — the
+ operator surface over messages a durable actor abandoned after exhausting
+ delivery retries. `DeadLetter` carries the full routing identity
+ (`promise_id`, `callback_actor_id`, `correlation_id`, `correlation_key`,
+ `priority`, `max_attempts`) so a requeue can reconstruct the original
+ mailbox message; `payload` is populated only on `GetDeadLetter` or when
+ `ListDeadLettersRequest.include_payload` is set.
## Relationships
@@ -39,3 +48,9 @@ gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
- `errors.go` is hand-written and not regenerated; callers must match wallet
lifecycle errors via `IsWalletNotReadyError`/`WalletNotReadyState`, never by
parsing the error message string.
+- Every RPC needs a matching `http` rule in `daemon.yaml` (POST
+ `/v1/daemon/<kebab-name>`, `body: "*"`) or it is unreachable over the REST
+ gateway, and a hand-written method on `rpc/restclient.DaemonServiceClient`
+ or that transport stops satisfying `DaemonServiceClient`.
+- `PurgeDeadLettersRequest.older_than_seconds` must be positive; the daemon
+ rejects zero so an empty request can never wholesale-delete the queue.
diff --git a/waverpc/CLAUDE.md b/waverpc/CLAUDE.md
index 608abb8e..2b878fcf 100644
--- a/waverpc/CLAUDE.md
+++ b/waverpc/CLAUDE.md
@@ -2,10 +2,11 @@
## Purpose
-Daemon gRPC API definitions for wallet, boarding, round, OOR, unroll, and
-VHTLC-recovery operations. Proto source: `waverpc/daemon.proto`. Generated
-gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
-(`errors.go`) for structured wallet-lifecycle errors.
+Daemon gRPC API definitions for wallet, boarding, round, OOR, unroll,
+VHTLC-recovery, and dead-letter operator operations. Proto source:
+`waverpc/daemon.proto`. Generated gRPC, REST-gateway, and mailbox-RPC stubs
+plus one hand-written helper file (`errors.go`) for structured
+wallet-lifecycle errors.
## Key Types
@@ -19,6 +20,14 @@ gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
- `IsWalletNotReadyError(err)` / `WalletNotReadyState(err)` — Match and unpack
the structured error produced above; callers should key off these instead of
matching on message text.
+- `DeadLetter` and the `ListDeadLetters` / `GetDeadLetter` /
+ `RequeueDeadLetter` / `PurgeDeadLetters` request/response pairs — the
+ operator surface over messages a durable actor abandoned after exhausting
+ delivery retries. `DeadLetter` carries the full routing identity
+ (`promise_id`, `callback_actor_id`, `correlation_id`, `correlation_key`,
+ `priority`, `max_attempts`) so a requeue can reconstruct the original
+ mailbox message; `payload` is populated only on `GetDeadLetter` or when
+ `ListDeadLettersRequest.include_payload` is set.
## Relationships
@@ -39,3 +48,9 @@ gRPC, REST-gateway, and mailbox-RPC stubs plus one hand-written helper file
- `errors.go` is hand-written and not regenerated; callers must match wallet
lifecycle errors via `IsWalletNotReadyError`/`WalletNotReadyState`, never by
parsing the error message string.
+- Every RPC needs a matching `http` rule in `daemon.yaml` (POST
+ `/v1/daemon/<kebab-name>`, `body: "*"`) or it is unreachable over the REST
+ gateway, and a hand-written method on `rpc/restclient.DaemonServiceClient`
+ or that transport stops satisfying `DaemonServiceClient`.
+- `PurgeDeadLettersRequest.older_than_seconds` must be positive; the daemon
+ rejects zero so an empty request can never wholesale-delete the queue.How to apply: save the block above and This check is advisory and does not fail CI. Run: https://github.com/lightninglabs/wavelength/pull/1119/checks |
In this commit, we fix the architecture doc's false claim that the OutboxPublisher folded path bypasses DurableMailbox.Send (it does not; the exemption is now real and implemented via the outbox-ID context marker), document the control-priority exemption and the boot messages that carry it, describe the hardened probe (single-flight, ambient-tx stripping), and state the known residual honestly: an in-turn Tell into a saturated peer fails the sender's turn and burns the inbound message's delivery attempts, with postpone semantics as the planned structural fix and the dead-letter tooling from #1119 as the interim recovery path.
|
@Roasbeef, remember to re-request review from reviewers when ready |
1 similar comment
|
@Roasbeef, remember to re-request review from reviewers when ready |
In this PR, we close the dead-letter half of #705: we had a full dead-letter store but no dead-letter handling. When a durable actor exhausted its
TellRetryPolicy, the framework wrote the message todead_lettersand then nothing ever looked at it again. No log, no metric, no operator surface, no recovery path. For value-bearing subsystems (OOR, rounds, unroll) a silently parked message can mean stuck or invisible funds.The contract this PR establishes: a dead letter is never silent and never final by default. The daemon surfaces it, an operator can recover it, and nothing ages out unless explicitly opted into.
Faithful requeue needs the full routing identity
The original
dead_lettersprojection dropped every routing field of the source mailbox row (priority, retry budget, ask plumbing, and the per-key FIFO tag), which made reconstruction impossible. A new actor-delivery migration widens the table andMoveMailboxToDeadLetternow projects the full identity. Rows written before the migration stay readable and requeue with fresh-enqueue defaults.RequeueDeadLetterthen re-enqueues the payload as a new mailbox message in one transaction: fresh UUIDv7, attempts reset, available immediately, routing preserved, dead letter deleted. The fresh ID is a correctness requirement rather than a convenience: the retry-exhaustion path marks the original ID processed before dead-lettering, so a same-ID requeue would be silently skipped by dedup (and masked by the enqueue'sON CONFLICT DO NOTHING). A successful requeue also fires the targeted mailbox wake, so a resident consumer picks the message up without waiting out its poll interval.The operator ops live on a new
actor.DeadLetterStoreinterface, deliberately kept out ofDeliveryStore: the runtime only ever writes dead letters, and widening the runtime interface would force every test double in the tree to grow methods the runtime never calls.A monitor that owns observation and retention
A daemon-owned
DeadLetterMonitor(OutboxPublisher lifecycle shape) scans for newly parked entries and surfaces each exactly once: error-level detail lines (bounded per scan, since a poison storm parks many at once and one summary line carries the same alerting weight), a startup summary of the backlog left by previous runs, and anOnDeadLetterhook feeding metrics. The since-scan boundary is inclusive at second granularity, so the monitor keeps a boundary set of reported IDs and prunes it as the watermark advances.Retention defaults to disabled: dead letters can carry value-bearing messages, so aging them out is an explicit
deadletter.retentionopt-in. The sweep also drives the store'sCleanupExpired(dedup entries + ask results GC), which previously had no periodic caller anywhere in the daemon.Three metrics cover the two failure shapes: the scrape-driven
waved_dead_lettersgauge (explicitly zero when clean, so series absence means metrics-off rather than all-clear) with its per-actor breakdown, and the monotonewaved_actor_dead_letters_observed_totalcounter that catches park-and-clear churn between scrapes. The metrics README gains an alerting section mapping each expression to the failure it detects, mirroring the ingress-liveness section from #1093.Operator surface
Four new
DaemonServiceRPCs (ListDeadLetters,GetDeadLetter,RequeueDeadLetter,PurgeDeadLetters) under a newdeadlettermacaroon entity, plus awavecli deadlettersubtree in the dev-gated advanced group next torecovery, with the same consent posture: requeue and purge require--yeson non-interactive stdin, and purge requires an explicit--older-thanage both at the CLI and the RPC, so an empty request can never wholesale-delete the queue.The docs gain a Dead-Letter Contract section covering who writes dead letters (the four framework paths), who owns them afterwards, and how the durable contract relates to the weaker in-memory DLO ref (advisory log-and-drop), which is the per-actor-class clarification #705 item 5 asks for.
See each commit message for a detailed description w.r.t the incremental changes.
Closes #705.