docs: canonical activity log design (event-log epic) - #798
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a design document for a Canonical Activity Log, which proposes replacing the current derive-on-read activity feed with a persisted, sequence-numbered SQLite table. The review feedback highlights three critical design issues: SQLite's AUTOINCREMENT does not guarantee gap-free sequences, sorting pagination by updated_at_unix can lead to unstable cursors and duplicate/skipped entries when rows are updated in-place, and the update-in-place model conflicts with the resumable subscription mechanism (clients will miss updates to older rows since their sequence numbers do not change).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
93f89ba to
c061622
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c061622f54
ℹ️ 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".
Defines the C1 canonical store that replaces the derive-on-read activity feed: a current-state activity_entries projection for List, plus an append-only activity_events log with a monotonic event_seq for a resumable SubscribeWallet, mirroring the mailbox. Documents the stable-id contract for every entry kind and the additive migration. Catalogued in docs/index.md.
c061622 to
e1b3ec5
Compare
Tracks #774 (C1), the foundational child of the event-log epic #776.
Design doc only — no code. Defines the persisted store that replaces the derive-on-read activity feed (
swapwallet/history.golistActivity). After the review, the design uses two tables instead of one:activity_entries— a current-state projection, one row per operation keyed by a stable canonical id, that backsList(ACTIVITY). Paged by an immutable key (created_at_unix), so a row that transitions in place keeps its position and pagination never skips or duplicates (folds in A5 / walletdk: ACTIVITY paging lacks has_more + a stable cursor and has a bounded merge window #781).activity_events— an append-only log, one immutable row per lifecycle transition with its own monotonicevent_seq, that backsSubscribeWallet. This faithfully mirrors the mailbox (Append/Pullover immutable rows) and is the only model that gives a reconnecting subscriber a gap-tolerant, no-missed-update cursor — the property C4 needs. (A single update-in-place table with a frozenseqcannot: a client past thatseqnever sees the row's later transitions, which is the silent drop the epic exists to fix.)Stable canonical id (the crux, #610). Only
payment_hash(SEND-invoice/RECV) and the OORsession_idare stable today. The doc is explicit that the EXIT/DEPOSIT/on-chain-send ids are net-new daemon work, not the reuse of an existing handle: the walletsend_intent_idis an in-memory map entry with a 5-minute TTL, deleted before dispatch, and the daemonPendingIntentIDis derived from the consumed forfeit outpoints (the very handles #610 says vanish) and deleted at round adoption. C1 must mint a durable id at request acceptance, return it fromLeaveVTXOs/SendOnChain, and emit the eventual settlement txid correlated to it — mirroring #587's reservation pattern.Also folds in the inline review: money columns are
BIGINT(a bareINTEGERoverflows above ~21.47 BTC on the Postgres backend),event_seqis monotonic-but-not-contiguous (consumers treat anyevent_seq > cursoras new), andListpages by the immutablecreated_atrather than the mutableupdated_at.The §3.6 append-only-vs-update-in-place question is now resolved (two tables). The C3 balance bugs are explicitly out of scope: they are daemon-side missing-balance-bucket gaps in
GetBalancethat never touchlistActivity, so a client activity log does not fix them.Catalogued in
docs/index.md. Seeking review on the schema, the stable-id contract, and the migration/backfill plan before any implementation lands.