From 887a6b9c4bc4d11892cad5cffd2e59595f4fed3d Mon Sep 17 00:00:00 2001 From: Chidimj <165854539+Chidimj@users.noreply.github.com> Date: Sun, 30 Aug 2026 03:12:41 +0100 Subject: [PATCH 01/36] docs: add documentation index and map (#589) Adds docs/README.md as the single entry point to the repository's documentation. Every .md file in the repo (excluding generated output and dependency directories) is listed exactly once with a one-line description. Documents are grouped by reader intent rather than by directory: new contributor, backend developer, frontend developer, contract developer, operator, security reviewer, and AI/data developer. A document needed by two roles is listed under both, since each section is meant to stand alone. Includes a "Start here" path for first-time contributors that ends at the contribution guidelines, and a maintenance section stating that the index must be updated whenever a document is added, moved, or removed. The root README now links to the index and to the architecture overview. Note: the issue refers to a top-level CONTRIBUTING.md and a docs/adr/ directory. Neither exists in this repository, so the "Start here" path ends at the Contributing section of the root README and calls out that it should be repointed if a dedicated CONTRIBUTING.md is added. --- README.md | 11 +++ docs/README.md | 212 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 docs/README.md diff --git a/README.md b/README.md index f5e35b06..c793e859 100644 --- a/README.md +++ b/README.md @@ -152,8 +152,19 @@ pnpm test ## πŸ“š Documentation +**➑️ [Documentation index](docs/README.md) β€” every document in this repository, grouped by +reader: new contributor, backend, frontend, contract developer, operator, security reviewer.** +Start there; it links to everything below and is kept exhaustive. + +New to the project? Read the [system architecture overview](docs/architecture-overview.md) +first β€” one diagram of all four apps and the external services, plus two end-to-end traces. + +Frequently opened: + - [Devices & Prekeys API](apps/backend/docs/api-devices.md) β€” every `/devices` and `/user-devices` route: request/response shapes, ownership checks, prekey upload contract, and revocation side effects - [E2EE Onboarding Sequence](apps/backend/docs/e2ee-onboarding.md) β€” device registration and prekey upload flow for first-contact DM setup +- [Contract events reference](contracts/docs/contracts-events.md) β€” every on-chain event, its payload, and which ones the backend listener consumes +- [WASM size and resource budget](contracts/docs/concepts-resource-budget.md) β€” the 100 KB per-contract CI gate, current sizes, and headroom --- diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 00000000..85915a46 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,212 @@ +# Documentation Index + +Every document in this repository, grouped by what you are trying to do rather than by +where the file happens to live. Each entry has a one-line description so you can pick the +right document without opening several first. + +If you are new here, follow [Start here](#start-here) and ignore the rest until you need it. + +> **Maintenance:** this index must be updated whenever a document is added, moved, or +> removed. A new `.md` file that is not listed here is effectively invisible β€” the whole +> point of this file is that it is exhaustive. See [Keeping this index honest](#keeping-this-index-honest). + +--- + +## Start here + +A first-time contributor should read these four, in this order. It is roughly an hour and +it is enough to make a scoped change and open a pull request. + +1. [Root README](../README.md) β€” what Clicked is, the tech stack, and how to install and + run the whole monorepo locally with `pnpm` and `docker compose`. +2. [System architecture overview](architecture-overview.md) β€” one diagram showing how the + four apps and the external services fit together, plus two end-to-end request traces. +3. The one app doc for the area you are touching β€” pick your entry point from + [By role](#by-role) below. +4. [Contributing guidelines](../README.md#-contributing) β€” branch naming, commit style, + and the pull request process. + +> **Note:** the contribution guidelines currently live in the "Contributing" section of the +> root README rather than in a top-level `CONTRIBUTING.md`. If a dedicated `CONTRIBUTING.md` +> is added later, this step and the link above should point at it instead. + +--- + +## By role + +### New contributor + +You want orientation and the shortest path to a working local environment. + +| Document | What it gives you | +| --- | --- | +| [Root README](../README.md) | Project pitch, tech stack, prerequisites, install, run, and test commands. | +| [System architecture overview](architecture-overview.md) | The single diagram of all four apps and every external service, with two traced end-to-end paths. | +| [Runbook](runbook.md) | Day-two operations: what to do when a service is unhealthy, and how to restart pieces safely. | +| [Observability](observability.md) | Which metrics, logs, and traces exist and where they are emitted, so you can see what your change did. | + +### Backend developer + +The Node.js gateway in `apps/backend`: REST, WebSockets, Postgres, Redis, and the chain +listener. + +**Architecture and concepts** + +| Document | What it gives you | +| --- | --- | +| [Gateway architecture](../apps/backend/docs/concepts-gateway-architecture.md) | Socket.IO connection lifecycle, room semantics, and how the gateway scales horizontally over Redis pub/sub. | +| [Delivery fan-out and receipts](../apps/backend/docs/concepts-delivery-fanout.md) | How one sent message reaches every recipient device, how receipts flow back, and which services are not actually wired into the live path. | +| [Storage and push jobs](../apps/backend/docs/concepts-storage-push-jobs.md) | Object storage layout and the background jobs that expire files, devices, and envelopes. | + +**API reference** + +| Document | What it gives you | +| --- | --- | +| [Auth API](../apps/backend/docs/api-auth.md) | Wallet-signature login, JWT issuance, and session refresh endpoints. | +| [Users API](../apps/backend/docs/api-users.md) | User profile read and update routes. | +| [Devices and prekeys API](../apps/backend/docs/api-devices.md) | Every `/devices` and `/user-devices` route: ownership checks, prekey upload contract, and revocation side effects. | +| [Conversations API](../apps/backend/docs/api-conversations.md) | Creating conversations, managing membership, and reading history. | +| [Messages and sync API](../apps/backend/docs/api-messages-sync.md) | Message history pagination and the cross-device sync cursor. | +| [Files and uploads API](../apps/backend/docs/api-files-uploads.md) | Encrypted attachment upload, download, and lifecycle. | +| [Push API](../apps/backend/docs/api-push.md) | Push subscription registration and notification dispatch. | +| [Treasury API](../apps/backend/docs/api-treasury.md) | REST routes for treasury proposals and votes, plus how they relate to the on-chain contracts. | +| [WebSocket events](../apps/backend/docs/api-websocket-events.md) | Every Socket.IO event the gateway emits and accepts, with direction and payload. | + +**Contracts and schemas** + +| Document | What it gives you | +| --- | --- | +| [JWT auth contract](../apps/backend/docs/contracts-jwt-auth.md) | Token claim shape, signing algorithm, and expiry rules. | +| [REST schemas](../apps/backend/docs/contracts-rest-schemas.md) | Request and response body schemas shared across the REST surface. | +| [WebSocket payloads](../apps/backend/docs/contracts-websocket-payloads.md) | Payload shapes for each WebSocket event, as validated on the wire. | + +**Encryption and migrations** + +| Document | What it gives you | +| --- | --- | +| [E2EE onboarding](../apps/backend/docs/e2ee-onboarding.md) | Device registration and prekey upload flow for first-contact DM setup. | +| [MLS key packages](../apps/backend/docs/mls-key-packages.md) | Key package publication, consumption, and replenishment. | +| [MLS group membership](../apps/backend/docs/mls-group-membership.md) | Adding and removing members from an MLS group and the resulting epoch changes. | +| [MLS group files](../apps/backend/docs/mls-group-files.md) | How file keys are distributed to an MLS group. | +| [Message encryption migration](../apps/backend/docs/message-encryption-migration.md) | Migrating stored messages onto the current encryption scheme. | +| [Signal migration](../apps/backend/docs/signal-migration.md) | Moving the double-ratchet implementation onto the Signal protocol. | +| [Backend security hardening](../apps/backend/docs/security-hardening.md) | Backend-specific hardening measures and the threats each one closes. | + +### Frontend developer + +The Next.js client in `apps/web`. It holds the private keys, so most of its documentation +is about encryption and local state. + +**Concepts** + +| Document | What it gives you | +| --- | --- | +| [Web app README](../apps/web/README.md) | Running the Next.js client on its own, its scripts, and its environment variables. | +| [E2EE architecture](../apps/web/docs/concepts-e2ee-architecture.md) | Where keys live in the browser, how sessions are established, and what never leaves the device. | +| [Message pipeline](../apps/web/docs/concepts-message-pipeline.md) | The client-side path from typed text to an encrypted envelope on the wire, and back. | +| [Auth and device lifecycle](../apps/web/docs/concepts-auth-device-lifecycle.md) | Wallet connection, device registration, session persistence, and revocation handling. | +| [File encryption](../apps/web/docs/concepts-file-encryption.md) | How attachments are encrypted client-side before upload. | +| [Local search](../apps/web/docs/concepts-local-search.md) | The on-device search index over decrypted message content. | +| [Push subscription](../apps/web/docs/concepts-push-subscription.md) | Service worker registration and push permission handling. | +| [Wallet and treasury UI](../apps/web/docs/concepts-wallet-treasury-ui.md) | How the wallet and treasury screens are composed and what they read from chain versus the backend. | + +**Client APIs and types** + +| Document | What it gives you | +| --- | --- | +| [REST client](../apps/web/docs/api-rest-client.md) | The typed wrapper around the backend REST surface. | +| [WebSocket client](../apps/web/docs/api-websocket-client.md) | Socket lifecycle, reconnection, and event subscription on the client. | +| [Soroban client](../apps/web/docs/api-soroban-client.md) | How the web app builds, signs, and submits Soroban contract invocations. | +| [Auth session contract](../apps/web/docs/contracts-auth-session.md) | The shape of the persisted session and what invalidates it. | +| [IndexedDB schemas](../apps/web/docs/contracts-indexeddb-schemas.md) | Every IndexedDB object store, its keys, and its migration history. | +| [Response types](../apps/web/docs/contracts-response-types.md) | Shared TypeScript response types used across the client. | +| [MLS integration notes](../apps/web/src/lib/mls-integration.md) | Implementation notes co-located with the MLS integration code. | +| [Search module README](../apps/web/src/lib/search/README.md) | Implementation notes for the local search module. | + +### Contract developer + +The Soroban workspace in `contracts/`: `token_transfer`, `group_treasury`, and `proposals`. + +| Document | What it gives you | +| --- | --- | +| [Contracts README](../contracts/README.md) | Workspace layout, toolchain, and how to build and test the contracts. | +| [Deployment and invocation](../contracts/docs/api-deployment-invocation.md) | Deploying each contract, initialising it, and invoking it from the CLI, including required environment variables. | +| [Contract events reference](../contracts/docs/contracts-events.md) | Every published event across all three contracts, its topic and data shape, the state change it signals, and whether the backend listener consumes it. | +| [Proposals API](../contracts/docs/api-proposals.md) | The `proposals` contract surface: creating, voting, finalising, and executing. | +| [Token transfer API](../contracts/docs/api-token-transfer.md) | The `token_transfer` contract surface, including the memo field used to correlate a transfer with a chat message. | +| [Proposal lifecycle](../contracts/docs/concepts-proposal-lifecycle.md) | Every proposal status, the transitions between them, and what triggers each one. | +| [Token transfer flow](../contracts/docs/concepts-token-transfer-flow.md) | The end-to-end flow of an in-chat payment through the contract. | +| [Token transfer storage](../contracts/docs/contracts-token-transfer-storage.md) | Storage keys and value types used by `token_transfer`. | +| [WASM size and resource budget](../contracts/docs/concepts-resource-budget.md) | The 100 KB per-contract CI gate, current sizes and headroom, and the levers available when a contract approaches the limit. | + +### Operator + +Running and monitoring a deployment. + +| Document | What it gives you | +| --- | --- | +| [Runbook](runbook.md) | Operational procedures: health checks, restarts, and incident response steps. | +| [Observability](observability.md) | Metrics, logs, and traces exposed by the services, and how to reach them. | +| [Deployment and invocation](../contracts/docs/api-deployment-invocation.md) | Contract deployment steps and the environment variables the backend needs to watch the chain. | +| [Storage and push jobs](../apps/backend/docs/concepts-storage-push-jobs.md) | The background jobs that run on a schedule and the storage they clean up. | + +### Security reviewer + +Threat model, hardening, and the crypto protocol documents. + +| Document | What it gives you | +| --- | --- | +| [Threat model](threat-model.md) | Assets, adversaries, trust boundaries, and the mitigations claimed for each threat. | +| [Security fixes summary](../SECURITY_FIXES_SUMMARY.md) | A log of security issues found and the fixes applied for each. | +| [Audit logging](security/audit-logging.md) | What is audit-logged, in what format, and what is deliberately excluded. | +| [Rate limits](security/rate-limits.md) | Every rate limit in the system, its scope, and its threshold. | +| [TLS and pinning](security/tls-and-pinning.md) | Transport security requirements and certificate pinning behaviour. | +| [Backend security hardening](../apps/backend/docs/security-hardening.md) | Backend hardening measures and the threats each one closes. | +| [Signal integration](signal-integration.md) | How the Signal protocol is integrated and which guarantees it provides. | +| [Group epoch sync](group-epoch-sync.md) | How MLS group epochs stay synchronised across devices and what happens when they diverge. | +| [E2EE architecture](../apps/web/docs/concepts-e2ee-architecture.md) | The client-side key model β€” the basis for any claim that the server cannot read messages. | + +### AI / data developer + +The FastAPI service in `apps/ai_agent`. + +| Document | What it gives you | +| --- | --- | +| [AI agent README](../apps/ai_agent/README.md) | Running the service locally with `uv`, and its environment variables. | +| [Chat API](../apps/ai_agent/docs/api-chat.md) | The assistant chat endpoint: request, response, and system prompt behaviour. | +| [Index and search API](../apps/ai_agent/docs/api-index-search.md) | Indexing documents into the vector store and querying them. | +| [Proposals summarise API](../apps/ai_agent/docs/api-proposals-summarise.md) | Summarising a governance proposal into a short digest. | +| [Transfers analyse API](../apps/ai_agent/docs/api-transfers-analyse.md) | Risk-scoring a transfer and the flagging threshold. | +| [RAG search architecture](../apps/ai_agent/docs/concepts-rag-search-architecture.md) | Retrieval-augmented search design: chunking, embedding, and retrieval. | +| [Transfer risk analysis](../apps/ai_agent/docs/concepts-transfer-risk-analysis.md) | The heuristics behind transfer risk scoring. | +| [Pydantic models](../apps/ai_agent/docs/contracts-pydantic-models.md) | Request and response model definitions for the service. | +| [Weaviate schema](../apps/ai_agent/docs/contracts-weaviate-schema.md) | The vector store collection schema and its properties. | + +--- + +## Repository meta + +Documents about the repository itself rather than about the product. + +| Document | What it gives you | +| --- | --- | +| [Pull request template](../.github/pull_request_template.md) | The checklist every pull request is opened against. | +| [PR notes](../pr.md) | Scratch notes for an in-flight pull request; not a reference document. | + +--- + +## Keeping this index honest + +This file is the only entry point into the documentation, which means a document missing +from it is a document nobody will find. + +- **Adding a document:** add a row to the section matching the *reader* who needs it, not + the directory it lives in. If two roles need it, list it under both β€” duplication across + role sections is intentional, since each section is meant to be read on its own. +- **Moving or deleting a document:** update or remove its row in the same commit. A broken + link here is worse than no link. +- **Adding a new area:** if a new app or subsystem arrives with its own `docs/` directory, + give it its own role section rather than appending to an existing one. + +The scope of this index is every `.md` file in the repository except generated output and +dependency directories (`node_modules/`, `target/`, `.venv/`, build artefacts). From b1a55fd9fa25841c07b8cd90fe8d541b99c3928a Mon Sep 17 00:00:00 2001 From: Chidimj <165854539+Chidimj@users.noreply.github.com> Date: Sun, 30 Aug 2026 03:18:38 +0100 Subject: [PATCH 02/36] docs: add system architecture overview (#587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds docs/architecture-overview.md as the orientation entry point for the system: one Mermaid diagram showing all four apps, the stateful infrastructure (Postgres, Redis, object storage, Weaviate), the Stellar network, and the external LLM provider, plus a table naming the protocol on every edge. Each component gets a short paragraph covering its responsibility and, explicitly, what it must never do β€” the invariants that are easy to break accidentally (the gateway must not acquire a decryption path, Redis must not be treated as durable, the AI agent must not gate a transfer). Two end-to-end paths are traced concretely against the current code: sending an encrypted message (per-device envelope encryption through deliveryPipeline fan-out to delivery receipts) and executing a treasury withdrawal (on-chain propose/vote/execute with the listener maintaining an off-chain mirror). The document is explicitly scoped as orientation only and defers to the per-app documents, which it links throughout. Notes a real defect found while tracing path 2: the backend listener subscribes to a `proposal_executed` topic that no contract publishes β€” `proposals` emits the truncated symbols `executed` and `execut`. The per-event detail belongs in the contract events reference (#581); this document links to it. The issue refers to IMPLEMENTATION_DOCS.md as the existing long-form design spec. No such file exists in this repository, so this document links to the per-app docs as the deeper reference instead. --- docs/architecture-overview.md | 291 ++++++++++++++++++++++++++++++++++ 1 file changed, 291 insertions(+) create mode 100644 docs/architecture-overview.md diff --git a/docs/architecture-overview.md b/docs/architecture-overview.md new file mode 100644 index 00000000..5357dd3d --- /dev/null +++ b/docs/architecture-overview.md @@ -0,0 +1,291 @@ +# System Architecture Overview + +**Scope: orientation only.** This document exists so that someone who has never seen the +codebase can understand how the pieces fit together in about ten minutes. It deliberately +stays shallow β€” every component section ends with a link to the document that actually +specifies it. When this document and a per-app document disagree, the per-app document is +right and this one needs fixing. + +For the exhaustive design specification, see the per-app documents linked throughout. For +a list of every document in the repository, see the [documentation index](README.md). + +--- + +## The whole system in one diagram + +```mermaid +flowchart TB + subgraph client["Web client β€” apps/web (Next.js, browser)"] + UI["React UI"] + CRYPTO["E2EE key store
(IndexedDB, non-extractable keys)"] + SOR["Soroban client
(@stellar/stellar-sdk)"] + end + + subgraph gateway["Backend gateway β€” apps/backend (Node.js)"] + REST["REST API (Express)"] + WS["WebSocket server (Socket.IO)"] + LISTEN["Stellar listener
services/stellarListener.ts"] + JOBS["Background jobs
(file / device / envelope GC)"] + end + + subgraph ai["AI agent β€” apps/ai_agent (FastAPI, Python)"] + AGENT["/chat, /transfers/analyse,
/proposals/summarise, /index, /search"] + end + + subgraph data["Stateful infrastructure"] + PG[("PostgreSQL
ciphertext, metadata, proposals")] + REDIS[("Redis
pub/sub, presence, rate limits")] + OBJ[("Object storage
MinIO / S3 / R2")] + VEC[("Weaviate
vector store")] + end + + subgraph chain["Stellar network"] + RPC["Soroban RPC"] + TT["token_transfer"] + GT["group_treasury"] + PROP["proposals"] + end + + LLM["LLM API
(external)"] + + UI --> CRYPTO + UI -->|"HTTPS/JSON"| REST + UI <-->|"WebSocket (Socket.IO)"| WS + UI --> SOR + SOR -->|"JSON-RPC, signed tx"| RPC + + REST --> PG + REST --> OBJ + WS --> PG + WS <-->|"pub/sub + presence"| REDIS + WS -->|"HTTP POST /chat"| AGENT + JOBS --> PG + JOBS --> OBJ + + LISTEN -->|"getEvents polling"| RPC + LISTEN --> PG + LISTEN -->|"treasury_proposal_updated"| WS + + AGENT --> VEC + AGENT --> LLM + + RPC --- TT + RPC --- GT + RPC --- PROP + PROP -->|"cross-contract call"| GT + TT -.->|"SEP-41 transfer"| GT +``` + +### Protocols on each edge + +| From | To | Protocol | +| --- | --- | --- | +| Web client | Backend REST | HTTPS, JSON, `Authorization: Bearer ` | +| Web client | Backend gateway | WebSocket (Socket.IO), JWT in the handshake `auth.token` | +| Web client | Soroban RPC | JSON-RPC over HTTPS, transactions signed in the browser wallet | +| Backend | PostgreSQL | TCP, via Drizzle ORM | +| Backend | Redis | RESP, pub/sub for cross-instance fan-out and presence | +| Backend | Object storage | S3 API (path-style against MinIO, virtual-host against S3/R2) | +| Backend | AI agent | HTTP POST, plaintext JSON | +| Backend listener | Soroban RPC | JSON-RPC `getEvents`, cursor-based polling every 5 s | +| AI agent | Weaviate | Weaviate Python client over HTTP | +| AI agent | LLM API | HTTPS to an external provider | +| `proposals` | `group_treasury` | Soroban cross-contract invocation | + +--- + +## Components + +### Web client β€” `apps/web` + +**Responsibility.** The Next.js browser application. It is the only component that holds +user private keys: it derives them, stores them in IndexedDB, encrypts every message and +attachment before it goes over the wire, and decrypts everything that comes back. It also +builds and signs Soroban transactions directly against the user's wallet, so payments do +not pass through the backend. + +**It must never** send a private key, a plaintext message body, or an unencrypted +attachment to the backend, and it must never trust the backend to tell it who a message +was from β€” sender identity is verified against the cryptographic session, not the +server-supplied metadata. + +See: [E2EE architecture](../apps/web/docs/concepts-e2ee-architecture.md), +[message pipeline](../apps/web/docs/concepts-message-pipeline.md), +[Soroban client](../apps/web/docs/api-soroban-client.md). + +### Backend gateway β€” `apps/backend` + +**Responsibility.** A Node.js process running Express for REST and Socket.IO for realtime. +It authenticates devices, stores and routes ciphertext, fans messages out to every +recipient device, tracks presence and delivery receipts, brokers uploads to object +storage, and runs the chain listener and the scheduled cleanup jobs. Multiple instances +run behind the Redis adapter, so any device may be connected to any instance. + +**It must never** be able to read message content β€” it stores and forwards opaque +ciphertext envelopes and must not acquire a decryption path. It must never accept a +client's claim about its own identity without verifying the JWT and confirming the device +row is present and unrevoked, and it must never hold custody of user funds; it observes +the chain, it does not sign for it. + +See: [gateway architecture](../apps/backend/docs/concepts-gateway-architecture.md), +[delivery fan-out](../apps/backend/docs/concepts-delivery-fanout.md), +[WebSocket events](../apps/backend/docs/api-websocket-events.md). + +### AI agent β€” `apps/ai_agent` + +**Responsibility.** A FastAPI service exposing chat assistance, transfer risk analysis, +proposal summarisation, and vector indexing/search. It calls an external LLM provider and +stores embeddings in Weaviate. + +**It must never** receive end-to-end encrypted message content that the user has not +explicitly submitted to it. Anything it is given leaves the E2EE trust boundary β€” it goes +to an external LLM provider in plaintext β€” so the boundary must stay explicit and +user-initiated. It is also not an authorisation component: nothing it returns should gate +a transfer or a withdrawal, since its output is advisory. + +See: [AI agent README](../apps/ai_agent/README.md), +[RAG search architecture](../apps/ai_agent/docs/concepts-rag-search-architecture.md), +[transfer risk analysis](../apps/ai_agent/docs/concepts-transfer-risk-analysis.md). + +### Soroban contracts β€” `contracts/` + +Three contracts make up the on-chain surface. + +- **`token_transfer`** β€” routes a SEP-41 token transfer between two addresses and emits a + `transfer` event carrying an opaque `memo`. The memo is how an on-chain payment is + correlated back to a chat message. +- **`group_treasury`** β€” holds pooled group funds. Tracks members and per-token balances, + and runs a threshold-approval withdraw-proposal flow. +- **`proposals`** β€” DAO-style governance: create a proposal, vote yes/no until expiry, + finalise on vote count, then execute β€” which, for a withdrawal, calls into + `group_treasury`. + +**They must never** trust a caller's claimed identity without `require_auth`, and must +never emit an event before the corresponding state write is committed β€” the backend +listener treats an event as proof that the state change happened. + +See: [contracts README](../contracts/README.md), +[contract events reference](../contracts/docs/contracts-events.md), +[proposal lifecycle](../contracts/docs/concepts-proposal-lifecycle.md), +[WASM size and resource budget](../contracts/docs/concepts-resource-budget.md). + +### PostgreSQL + +**Responsibility.** The system of record for everything off-chain: users, devices and +their public prekeys, conversations and membership, message ciphertext and per-device +envelopes, delivery receipts, file metadata, and the off-chain mirror of treasury +proposals and votes. + +**It must never** hold plaintext message bodies or user private keys. Ciphertext at rest +is the design; a schema change that lands readable content in a column is a break in the +threat model, not an optimisation. + +### Redis + +**Responsibility.** Cross-instance coordination β€” the Socket.IO adapter's pub/sub backbone, +presence state, per-device delivery channels, device-revocation notifications, and rate +limit counters. + +**It must never** be treated as durable. Everything in Redis is reconstructible from +PostgreSQL or from a reconnecting client; a flushed Redis must degrade the system, not +lose data. + +### Object storage (MinIO / S3 / R2) + +**Responsibility.** Stores encrypted file attachments. The backend brokers access; the +same S3 client path serves local MinIO and production S3 or R2, differing only by +environment variables. + +**It must never** receive an unencrypted attachment β€” files are encrypted in the browser +before upload β€” and must never be exposed as a public bucket, since object keys would then +be the only thing standing between an attacker and every stored file. + +### Stellar network / Soroban RPC + +**Responsibility.** Consensus, settlement, and the event stream. The backend reaches it +only through Soroban RPC's `getEvents`, polled on a cursor. + +**It must never** be assumed synchronously consistent with the backend's database. Events +arrive seconds later, may be re-delivered on reconnect, and the ledger is the authority +whenever the two disagree. + +See: [deployment and invocation](../contracts/docs/api-deployment-invocation.md). + +--- + +## End-to-end path 1: sending an encrypted message + +The point of this trace is that the backend never holds anything readable. + +1. **Compose.** The user types into the React UI. The client loads the cryptographic + session for the conversation from IndexedDB. +2. **Encrypt, per device.** The client encrypts the plaintext once per recipient *device* β€” + not per user. A conversation of three users with two devices each produces one envelope + per active device. Private keys never leave the browser. +3. **Emit.** The client emits `send_message` over its authenticated Socket.IO connection, + carrying the message id, content type, and the map of per-device ciphertext envelopes. +4. **Validate and persist.** The gateway validates the payload shape and size (16 KB cap), + applies the per-socket rate limit, and writes the message row plus its envelopes to + PostgreSQL. It stores opaque bytes; it cannot decrypt them. +5. **Fan out.** `services/deliveryPipeline.ts` loads the conversation's active, + non-revoked devices and emits a `message_envelope` event to each `device:${deviceId}` + room containing only that device's envelope, plus a ciphertext-free `new_message` to the + conversation room for unread counts. Recipients connected to a different gateway + instance are reached through the Redis adapter. +6. **Decrypt.** Each recipient client receives its envelope, decrypts it with its own key + material, advances the ratchet, and renders the message. +7. **Receipt.** The recipient acknowledges; `services/deliveryAggregation.ts` marks that + device delivered and, once every active device of a recipient user has acknowledged, + notifies the sender with `message_fully_delivered`. + +> The full trace, including which services are implemented but *not* wired into the live +> send path, is in [delivery fan-out](../apps/backend/docs/concepts-delivery-fanout.md). +> Read that before changing anything in this path. + +## End-to-end path 2: executing a treasury withdrawal + +The point of this trace is that authority lives on-chain and the backend only observes. + +1. **Propose.** A treasury member creates a withdrawal proposal. The proposal is recorded + on-chain β€” `group_treasury::propose_withdraw` for the treasury's own threshold flow, or + `proposals::create_proposal` for the governance flow β€” and the client signs the + transaction with the user's wallet. The backend's REST treasury routes maintain an + off-chain mirror in PostgreSQL for querying and UI, but that mirror is not the source of + truth. +2. **Observe.** The contract emits `proposal_created`. Within about five seconds the + Stellar listener's `getEvents` poll picks it up, upserts on + `(contractId, proposalId)`, and emits `treasury_proposal_updated` into the linked + conversation room. The upsert is what makes re-reading a page after a reconnect safe. +3. **Vote.** Each member signs an approve or reject transaction. `group_treasury` records + one vote per member per proposal, emits `withdraw_vote` for every vote, and separately + emits `proposal_approved` once approvals reach the configured threshold β€” or + `proposal_rejected` once rejections reach the blocking minority, the point at which + enough approvals can no longer be gathered. +4. **Execute.** Once the proposal has passed, execution moves the funds. In the governance + flow, `proposals::execute_withdraw` checks the caller is a treasury member, checks the + balance, calls `group_treasury::withdraw` cross-contract, and flips the status to + `Executed`. +5. **Settle.** `group_treasury` emits `withdraw` as the tokens move, and the ledger closes. + The listener picks up the events and updates the mirror; connected clients see the + status change pushed over WebSocket. + +> **Known gap.** The backend listener subscribes to a `proposal_executed` topic, but no +> contract publishes that topic β€” `proposals` publishes the truncated symbols `executed` +> and `execut` instead. An executed proposal therefore does not currently transition to +> `executed` in the off-chain mirror through the listener. This is documented per-event in +> the [contract events reference](../contracts/docs/contracts-events.md). + +--- + +## Where to go next + +| You want to… | Read | +| --- | --- | +| Change how messages are delivered | [delivery fan-out](../apps/backend/docs/concepts-delivery-fanout.md) | +| Change the WebSocket surface | [WebSocket events](../apps/backend/docs/api-websocket-events.md), [payloads](../apps/backend/docs/contracts-websocket-payloads.md) | +| Change client-side crypto | [E2EE architecture](../apps/web/docs/concepts-e2ee-architecture.md) | +| Change or deploy a contract | [deployment and invocation](../contracts/docs/api-deployment-invocation.md) | +| Consume a new on-chain event | [contract events reference](../contracts/docs/contracts-events.md) | +| Understand the security posture | [threat model](threat-model.md) | +| Operate a deployment | [runbook](runbook.md), [observability](observability.md) | +| Find any other document | [documentation index](README.md) | From d831d2b8dc32a81745e1fd59aa121399d2b13f7c Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:19:19 +0100 Subject: [PATCH 03/36] docs(contracts): document contract upgrade and versioning Adds contracts/docs/concepts-upgrades.md covering token_transfer's upgrade(env, new_wasm_hash) entrypoint end to end. - Authorization: the single admin.require_auth() gate, that the admin is fixed at initialize with no rotation path, and what a compromised admin key means (total, unrecoverable loss of control over the contract). - Procedure: build with the pinned release profile, upload to obtain the wasm hash, verify that hash locally with sha256sum and via an independent reproduction, invoke as the admin, then confirm by reading state back. - Storage compatibility: soroban-sdk encodes contracttype enums by variant-name symbol and structs as name-keyed sorted maps, so renames and type changes corrupt or orphan state while reordering is harmless. Tables split safe from unsafe changes on that basis, and note the rename of DataKey::Admin as the unrecoverable case. - Records that only token_transfer is upgradeable; group_treasury and proposals have no upgrade entrypoint, so changing them means a fresh deployment and repointing every consumer. - Notes the absence of any version() entrypoint or upgrade event: the wasm hash is the only on-chain version identifier. --- contracts/docs/concepts-upgrades.md | 337 ++++++++++++++++++++++++++++ 1 file changed, 337 insertions(+) create mode 100644 contracts/docs/concepts-upgrades.md diff --git a/contracts/docs/concepts-upgrades.md b/contracts/docs/concepts-upgrades.md new file mode 100644 index 00000000..fb42a060 --- /dev/null +++ b/contracts/docs/concepts-upgrades.md @@ -0,0 +1,337 @@ +# Contract upgrades & versioning + +How a deployed Soroban contract in this repo is replaced with new code, who is +allowed to do it, and which changes to the contract's storage layout are safe +across that swap. + +The whole mechanism is a single function, `upgrade`, on **one** contract: +`token_transfer`. Everything below is anchored to +[`contracts/contracts/token_transfer/src/lib.rs`](../contracts/token_transfer/src/lib.rs). + +--- + +## 1. Which contracts are upgradeable + +| Contract | Path | Upgradeable | Mechanism | +| ---------------- | ------------------------------------ | ----------- | -------------------------------------------------------------------------- | +| `token_transfer` | `contracts/contracts/token_transfer` | **Yes** | `upgrade(env, new_wasm_hash)` β€” admin-gated `update_current_contract_wasm` | +| `group_treasury` | `contracts/contracts/group_treasury` | **No** | No upgrade entrypoint exists | +| `proposals` | `contracts/contracts/proposals` | **No** | No upgrade entrypoint exists | + +`group_treasury` and `proposals` expose no function that calls +`env.deployer().update_current_contract_wasm(...)`. Their deployed wasm is +immutable for the life of the contract instance. Changing their behaviour means +**deploying a new contract instance** and repointing every consumer at the new +contract ID (see [Β§7](#7-what-to-do-for-a-non-upgradeable-contract)). + +`proposals` carries an admin address that its source comments describe as a hook +"a future upgrade can wire" for governance parameters. That is a note about +future work, not an upgrade path β€” there is no upgrade entrypoint on that +contract today. + +--- + +## 2. Authorization: who may call `upgrade` + +```rust +pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + let admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .expect("not initialized"); + admin.require_auth(); + env.deployer().update_current_contract_wasm(new_wasm_hash); +} +``` + +The gate is exactly one thing: `admin.require_auth()`, where `admin` is the +address written to `DataKey::Admin` by `initialize` and **never written again**. +`token_transfer` has no `set_admin`, no admin-transfer function, and no +multi-signer or timelock logic. The admin is fixed at initialization for the +lifetime of the contract instance. + +Consequences that follow directly from that: + +- **Any caller who can produce a valid signature for the admin address can + replace the contract's entire code.** There is no second approval, no delay, + and no on-chain veto. +- **The admin key cannot be rotated.** If the key is compromised there is no + in-contract remedy β€” the attacker and the legitimate operator have identical + authority, and the attacker can upgrade the contract to wasm that removes the + legitimate operator's access. +- **A compromised admin key is total loss of control over the contract.** The + attacker can upgrade to wasm that redirects every `transfer` to an address + they control, or that drains any allowance the contract holds. Existing + storage (`Admin`, `TokenContract`) survives the swap, so the new code starts + with the contract's full state. +- **`set_token_contract` shares the same key.** The same compromise lets an + attacker point the router at a malicious token contract without upgrading + anything. + +Because a compromised admin is unrecoverable, the admin address should be held to +the same standard as a treasury key β€” hardware-backed, or a Stellar multisig +account so that no single signer can act alone. This is an operational control; +the contract itself enforces nothing beyond a single signature. + +### Verified by tests + +[`contracts/contracts/token_transfer/src/test.rs`](../contracts/token_transfer/src/test.rs) +covers both sides of the gate: + +- `test_upgrade_requires_admin_auth` β€” with the admin's auth mocked, the call + passes the auth check and fails later at the wasm lookup, proving admin auth + is _sufficient_. +- `test_upgrade_non_admin_panics` β€” with only a non-admin address authorized, + the call panics at `require_auth()`, proving admin auth is _necessary_. + +--- + +## 3. End-to-end upgrade procedure + +All commands run from `contracts/` and use the Stellar CLI, matching +[the deployment guide](./api-deployment-invocation.md). Replace `testnet` with +your target network. + +### 3.1 Build the new wasm + +```bash +cargo build -p token_transfer --target wasm32-unknown-unknown --release +``` + +Output: `target/wasm32-unknown-unknown/release/token_transfer.wasm`. + +Build with the workspace's release profile as committed β€” `contracts/Cargo.toml` +pins `opt-level = "z"`, `lto = true`, and `strip = "symbols"`. Changing profile +flags changes the binary and therefore its hash, so a hash produced under a +different profile will not match one anybody else builds. + +Run the same gates CI runs before uploading anything: + +```bash +cargo test -p token_transfer +cargo fmt --all -- --check +cargo clippy --workspace --target wasm32-unknown-unknown -- -D warnings -A dead_code -A clippy::too-many-arguments +``` + +### 3.2 Produce the wasm hash + +Uploading the binary installs the code on the ledger without deploying an +instance, and returns its hash β€” this is the value `upgrade` takes: + +```bash +stellar contract upload \ + --wasm target/wasm32-unknown-unknown/release/token_transfer.wasm \ + --source \ + --network testnet +# β†’ 6ddb28e0980f643bb97350f7e3bacb0ff1fe74d846c6d4f2c625e766210fbb5b +``` + +The 32-byte value it prints is the `BytesN<32>` that `upgrade` expects. + +### 3.3 Verify the hash before invoking + +The hash is the _only_ thing binding the on-chain code to the source that was +reviewed. Verify it locally rather than trusting the upload output: + +```bash +# The uploaded hash is the sha256 of the wasm file bytes. +sha256sum target/wasm32-unknown-unknown/release/token_transfer.wasm +``` + +That value must equal the hash printed by `stellar contract upload`. If they +differ, the binary that was uploaded is not the one on disk β€” stop. + +A reviewer should independently reproduce the build from the reviewed commit and +confirm they get the same hash. Because the release profile is pinned in +`Cargo.toml` and the toolchain is pinned in +[`contracts/rust-toolchain.toml`](../rust-toolchain.toml), an independent build +of the same source should reproduce it. A mismatch means the toolchain or the +source differs, and the upgrade should not proceed until that is explained. + +### 3.4 Invoke `upgrade` + +```bash +stellar contract invoke \ + --id \ + --source \ + --network testnet \ + -- upgrade --new_wasm_hash +``` + +`--source` must be the admin identity β€” any other signer panics at +`require_auth()`. + +### 3.5 Confirm the upgrade took effect + +The contract emits no upgrade event, so confirmation is by observation: + +```bash +# 1. State survived the swap β€” this must still return the configured token. +stellar contract invoke --id --source --network testnet \ + -- token_contract + +# 2. New behaviour is live β€” call a function whose behaviour changed, +# or one that only exists in the new wasm. +stellar contract invoke --id --source --network testnet \ + -- +``` + +A read of `token_contract` that panics with `not initialized` after an upgrade +means the new wasm is reading a different storage key than the old wasm wrote β€” +see [Β§4](#4-storage-layout-compatibility). Treat that as a live incident: the +contract still holds the old data, but the new code cannot see it. + +The contract ID does **not** change across an upgrade. Backend and frontend +configuration (`TOKEN_TRANSFER_CONTRACT_ID`, the corresponding `NEXT_PUBLIC_*`) +needs no update. Clients holding a generated contract client **do** need +regenerating if function signatures changed. + +--- + +## 4. Storage layout compatibility + +`update_current_contract_wasm` swaps the code and **leaves storage untouched**. +The new wasm inherits every key the old wasm wrote, byte for byte. Nothing +migrates automatically and nothing is validated β€” a mismatch is silent until a +read fails or, worse, succeeds with the wrong data. + +`token_transfer` stores two instance-scoped keys, defined in +[`storage.rs`](../contracts/token_transfer/src/storage.rs): + +```rust +#[contracttype] +pub enum DataKey { + TokenContract, // Address of the SEP-41 token this contract routes through + Admin, // Address permitted to upgrade / set the token contract +} +``` + +Both are read via `env.storage().instance()`. There is no persistent or +temporary storage, so TTL and rent do not enter into the compatibility question +here. + +### 4.0 How the key encoding actually works + +The compatibility rules below follow from one fact about `#[contracttype]`, and +it is worth stating plainly because the intuition from other chains is wrong +here: **Soroban encodes by name, not by position.** + +- A `#[contracttype]` **enum** encodes each variant as an `ScSymbol` of the + variant's _identifier_ β€” `DataKey::Admin` is stored under the symbol + `"Admin"`. Declaration order plays no part in the encoding. +- A `#[contracttype]` **struct** encodes as an `ScMap` keyed by the field names, + sorted by name. Declaration order plays no part there either. + +So the dangerous edits are the ones that change a **name** or a **type**, not the +ones that change an order. (`#[contracterror]` / `contracttype` _integer_ enums +are the exception β€” those do encode by discriminant β€” but `token_transfer` uses +neither.) + +### 4.1 Safe changes + +| Change | Why it is safe | +| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Adding a **new** `DataKey` variant | Existing variants keep their own name symbols, so old keys still resolve. The new key simply has no value yet β€” reads must handle absence. | +| **Reordering** existing `DataKey` variants | Variants encode by name, so order is not part of the key. Harmless, though a pointless diff. | +| **Reordering** fields in a stored struct | Struct fields encode as a name-keyed map sorted by name, so declaration order is not part of the encoding. | +| Adding, removing, or changing contract **functions** | Functions are not storage. Callers of a removed function break, but stored state is unaffected. | +| Changing function **bodies** (logic, validation, events emitted) | No storage encoding is involved. | +| Adding a field to an **event** struct such as `TransferEvent` | Events are emitted, never read back from storage. Off-chain consumers must tolerate the new shape, but no on-ledger state is corrupted. | +| Adding a field to a **stored** struct, if reads tolerate its absence | Old values decode without the new key. The decode fails unless the field is optional or the read path handles the missing key, so this needs a deliberate migration path β€” see [Β§4.3](#43-when-a-layout-change-is-unavoidable). | +| Changing the release profile or the SDK **patch** version | Produces a different wasm hash, not a different storage layout. | + +### 4.2 Unsafe changes β€” these corrupt or orphan existing state + +| Change | What breaks | +| ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Renaming** a `DataKey` variant | The key is the variant name. Renaming `Admin` to `Owner` makes the new wasm read a key nothing was ever stored under: `upgrade` and `set_token_contract` panic with `not initialized`, and the old admin value is orphaned on the ledger permanently. | +| **Removing** a variant that still has a value on the ledger | The value stays on-ledger, unreachable and unrecoverable, and continues to occupy the instance entry. | +| Changing a stored value's **type** (`Address` β†’ `BytesN<32>`, `i128` β†’ `u64`) | The old bytes are decoded as the new type. This either panics on read or, where the encodings happen to overlap, silently yields a wrong value. | +| Changing an enum variant's **payload** (unit β†’ tuple, or its inner types) | `DataKey::Admin` and `DataKey::Admin(u32)` are different keys with different encodings. The old value becomes unreachable. | +| **Renaming** or **retyping** a field in a stored struct | Same as the enum: the map key is the field name. The old field's value is orphaned and the new field reads as missing. `token_transfer` stores no structs today, so this is a rule to preserve rather than a present hazard. | +| Moving a key between **storage scopes** (instance β†’ persistent/temporary) | The scopes are separate namespaces. The old value stays in instance storage and the new read finds nothing. | +| Re-running `initialize` after an upgrade | It panics β€” `initialize` guards on `DataKey::Admin` already existing. There is no re-initialization path, by design. | + +The worst case in that table is the rename, because it fails _closed_ in the most +expensive way possible: `Admin` renamed means nobody can authorize an `upgrade` +any more, so the mistake cannot be fixed by upgrading again. That is +unrecoverable for `token_transfer`. Treat any rename of a `DataKey` variant as a +change that must not ship. + +### 4.3 When a layout change is unavoidable + +There is no migration hook on this contract: no post-upgrade initializer, no +stored schema version, and no way to run code once at the moment of the swap. If +the layout must change, the migration has to be written into the new wasm itself +β€” an admin-only function invoked in the same operational window as the upgrade: + +1. Upgrade to a wasm that understands **both** layouts and exposes a `migrate` + function. +2. Invoke `migrate` as the admin. It reads the old keys, writes the new ones, and + records that it ran so it cannot run twice. +3. Optionally upgrade again to a wasm that has dropped the old-layout code. + +Rehearse the whole sequence on testnet, against a contract instance holding +representative state, before touching mainnet. + +--- + +## 5. Versioning + +The contract exposes no version. There is no `version()` function, no stored +version key, and no event announcing an upgrade β€” an observer cannot ask a +deployed `token_transfer` which build it is running. + +In practice the deployed version is identified by the **wasm hash**, which is +what the ledger records and what `stellar contract upload` returns. Keep an +operational record mapping each upgrade to the git commit it was built from, the +hash that build produced, the network, and the date. The chain does not keep that +mapping for you. + +Adding a `version()` entrypoint returning a compile-time constant is a safe +change under [Β§4.1](#41-safe-changes); storing the version under a new `DataKey` +variant is safe too β€” it is a new name, so nothing existing is disturbed. + +--- + +## 6. Pre-upgrade checklist + +- [ ] Change is reviewed and merged; the build is from a known commit. +- [ ] `cargo test -p token_transfer`, `cargo fmt --check`, and `cargo clippy` pass. +- [ ] Storage diff reviewed against [Β§4.2](#42-unsafe-changes--these-corrupt-or-orphan-existing-state) β€” no rename, removal, payload change, or type change of any `DataKey` variant. +- [ ] Local `sha256sum` matches the hash returned by `stellar contract upload`. +- [ ] A reviewer independently reproduced the build and got the same hash. +- [ ] Full sequence rehearsed on testnet against an initialized contract holding state. +- [ ] Admin key access confirmed and the signer identified before the invoke. +- [ ] Post-upgrade reads (`token_contract`, `balance`) planned as the confirmation step. + +--- + +## 7. What to do for a non-upgradeable contract + +`group_treasury` and `proposals` cannot be swapped in place. Changing them means: + +1. Deploy a new instance from the new wasm β€” `contracts/scripts/deploy_group_treasury.sh` + or `contracts/scripts/deploy_proposals.sh`. +2. Initialize it and re-establish its state. Members, thresholds, and balances do + not carry across. Funds held by a `group_treasury` instance must be withdrawn + through the old contract's own rules before the old instance is abandoned. +3. Update `GROUP_TREASURY_CONTRACT_ID` / `PROPOSALS_CONTRACT_ID` in the backend + env and the corresponding `NEXT_PUBLIC_*` values in the frontend, per + [Β§6 of the deployment guide](./api-deployment-invocation.md#6-how-a-deployed-contract-id-reaches-the-frontend--backend). +4. Accept that in-flight proposals on the old instance are stranded β€” they can + still be voted on and executed against the old contract, which no client is + pointed at any more. + +Because there is no in-place path, treat a `group_treasury` or `proposals` +deployment as final and get the logic right before mainnet. + +--- + +## 8. Related documents + +- [Contract build, deployment & invocation guide](./api-deployment-invocation.md) β€” toolchain, build, deploy, and the invoke syntax used above +- [Token transfer storage layout & token interface](./contracts-token-transfer-storage.md) β€” the storage keys these compatibility rules apply to +- [Token transfer API](./api-token-transfer.md) β€” the function surface an upgrade may change +- [Token transfer flow](./concepts-token-transfer-flow.md) β€” how the contract is used end to end From 54d3f9b19cc9d56ada29f8865b35096e174b778f Mon Sep 17 00:00:00 2001 From: Chidimj <165854539+Chidimj@users.noreply.github.com> Date: Sun, 30 Aug 2026 03:20:58 +0100 Subject: [PATCH 04/36] docs: add contract events reference (#581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds contracts/docs/contracts-events.md documenting all sixteen env.events().publish(...) call sites across token_transfer, group_treasury and proposals: topic tuple, data payload field by field, the state change each event signals, and whether the publish happens before or after the corresponding storage write. Emission order is called out per event because two of them β€” group_treasury's proposal_approved and proposal_rejected β€” publish before the proposal is persisted, setting the status on the in-memory struct first. Not observable to an off-chain consumer given transaction atomicity, but relevant when reading the contract. A summary table marks each event as consumed or unconsumed by the backend listener, and a dedicated section collects the five consumption gaps found while cross-referencing the contracts against stellarListener.ts: - the listener subscribes to `proposal_executed`, a topic no contract publishes. proposals emits `executed` and `execut` instead, so the status map entry is unreachable and a proposal never reaches the executed status in the off-chain mirror via the listener - group_treasury has no execution event tying a fund movement back to the proposal that authorised it - deposit and withdraw, the two value-movement events, have no consumer - only one contract id is watched for treasury topics, and both contracts publish a `proposal_created` with different payloads and different id widths (u32 vs u64) - vote-level events are unconsumed; on-chain votes are invisible to the backend until a transition event fires Also documents two payload gaps (DepositEvent and WithdrawEvent omit the token address despite per-token balance tracking) and notes that the proposer's auto-approval emits no withdraw_vote, so counting those events undercounts approvals by one. The `executed` / `execut` split is flagged as a naming defect rather than a constraint: symbol_short! permits 9 characters and "executed" is 8, so the truncation is unforced. Cross-links the backend chain listener source, the treasury API doc, and the deployment doc. --- contracts/docs/contracts-events.md | 484 +++++++++++++++++++++++++++++ 1 file changed, 484 insertions(+) create mode 100644 contracts/docs/contracts-events.md diff --git a/contracts/docs/contracts-events.md b/contracts/docs/contracts-events.md new file mode 100644 index 00000000..576ed72a --- /dev/null +++ b/contracts/docs/contracts-events.md @@ -0,0 +1,484 @@ +# Contract Events Reference + +Every `env.events().publish(...)` call across the three Soroban contracts, with its topic +tuple, data payload, the state change it signals, and whether the event is emitted before +or after that state is written. + +These events are the only mechanism by which off-chain systems learn that on-chain state +changed. The backend's chain listener +([`apps/backend/src/services/stellarListener.ts`](../../apps/backend/src/services/stellarListener.ts)) +polls Soroban RPC `getEvents` on a cursor and turns them into database writes and +WebSocket pushes β€” see the [Treasury API doc](../../apps/backend/docs/api-treasury.md) and +the [deployment and invocation doc](api-deployment-invocation.md) for how the listener is +configured and started. + +**Emission order matters.** A consumer treats an event as proof that the state change +happened. Every event below is annotated with whether the `publish` call runs before or +after the corresponding `env.storage()` write, because two events in this codebase are +emitted *before* their write and one payload is therefore a projection of intended state +rather than committed state. + +--- + +## Summary + +| Contract | Topic | Consumed by the backend listener? | +| --- | --- | --- | +| `token_transfer` | `transfer` | βœ… Yes | +| `group_treasury` | `member_added` | ❌ No | +| `group_treasury` | `member_removed` | ❌ No | +| `group_treasury` | `deposit` | ❌ No | +| `group_treasury` | `withdraw` | ❌ No | +| `group_treasury` | `proposal_created` | βœ… Yes | +| `group_treasury` | `proposal_approved` | βœ… Yes | +| `group_treasury` | `proposal_rejected` | βœ… Yes | +| `group_treasury` | `withdraw_vote` | ❌ No | +| `proposals` | `proposal_created` | ⚠️ Only if the contract id is configured as a treasury contract | +| `proposals` | `vote_cast` | ❌ No | +| `proposals` | `proposal_finalized` | ❌ No | +| `proposals` | `proposal_expired` | ⚠️ Same caveat as above | +| `proposals` | `executed` | ❌ No | +| `proposals` | `execut` | ❌ No | + +The listener also subscribes to a topic named **`proposal_executed`, which no contract +publishes.** See [Consumption gaps](#consumption-gaps). + +--- + +## `token_transfer` + +Source: [`contracts/token_transfer/src/lib.rs`](../contracts/token_transfer/src/lib.rs), +event structs in [`storage.rs`](../contracts/token_transfer/src/storage.rs). + +### `transfer` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "transfer"),)` β€” single-element | +| **Data type** | `TransferEvent` | +| **Emitted in** | `transfer()`, `lib.rs:56` | +| **Order** | **After** the token move. `token.transfer(...)` executes first, so the event follows the balance change. | +| **Consumed** | βœ… Yes | + +**Data payload** + +| Field | Type | Meaning | +| --- | --- | --- | +| `from` | `Address` | Sender; authorised via `from.require_auth()`. | +| `to` | `Address` | Recipient. | +| `amount` | `i128` | Amount in token units; guaranteed `> 0` (the call panics otherwise). | +| `memo` | `Bytes` | Opaque reference. When the transfer originated from a chat message this carries that message's UUID. | + +**State change signalled.** `amount` of the configured SEP-41 token has moved from `from` +to `to`. Note that `token_transfer` holds no balance state of its own β€” the authoritative +state change is in the token contract, and this event is the record that the routed +transfer succeeded. + +**How the backend consumes it.** `buildRpcFetcher` filters on `topics: [['transfer']]` for +the `TOKEN_TRANSFER_CONTRACT_ID`. `defaultPersistEvent` hex-decodes `memo`, and if it +parses as a UUID matching a row in `messages`, associates the transfer with that message's +conversation and sender. It then upserts into `token_transfers` keyed on `tx_hash`, so a +reconnect that re-reads a page produces no duplicates. + +> **Consumer caveat.** If the memo is absent or is not a UUID of an existing message, the +> listener falls back to *the first row* of `conversations` and `users` rather than +> skipping the row, because `conversationId` and `senderId` are non-nullable. Transfers +> not originating from a chat message are therefore attributed to an arbitrary +> conversation and user. + +--- + +## `group_treasury` + +Source: [`contracts/group_treasury/src/lib.rs`](../contracts/group_treasury/src/lib.rs), +event structs in [`storage.rs`](../contracts/group_treasury/src/storage.rs). + +This contract publishes nine events across membership, funds movement, and every proposal +transition. + +### Membership + +#### `member_added` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "member_added"),)` | +| **Data type** | `MemberAddedEvent` | +| **Emitted in** | `add_member()`, `lib.rs:79` | +| **Order** | **After** the write β€” the updated `Members` vector is persisted first. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `member` | `Address` | The address added to the member set. | +| `added_by` | `Address` | The admin that performed the change. | + +**State change.** `DataKey::Members` now contains `member`. Admin-only; re-adding an +existing member panics. + +#### `member_removed` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "member_removed"),)` | +| **Data type** | `MemberRemovedEvent` | +| **Emitted in** | `remove_member()`, `lib.rs:116` | +| **Order** | **After** the write. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `member` | `Address` | The address removed. | +| `removed_by` | `Address` | The admin that performed the change. | + +**State change.** `DataKey::Members` no longer contains `member`. Removing a non-member +panics. + +> **Note for consumers.** Membership changes the denominator of the rejection rule β€” the +> blocking minority is computed from the *current* member count at vote time. Since these +> two events are unconsumed, the backend's mirror of the member set can drift from chain. + +### Funds + +#### `deposit` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "deposit"),)` | +| **Data type** | `DepositEvent` | +| **Emitted in** | `deposit()`, `lib.rs:168` | +| **Order** | **After** both the token transfer and the balance write. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `from` | `Address` | Depositor; authorised via `require_auth()`. | +| `amount` | `i128` | Amount deposited; guaranteed `> 0`. | + +**State change.** Tokens moved into the contract and `DataKey::Balances[token]` increased +by `amount`. + +> **Payload gap.** `DepositEvent` does not carry the `token` address, even though the +> function takes one and balances are tracked per token. A consumer cannot tell which +> token was deposited from the event alone. + +#### `withdraw` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "withdraw"),)` | +| **Data type** | `WithdrawEvent` | +| **Emitted in** | `withdraw()`, `lib.rs:198` | +| **Order** | **After** both the token transfer and the balance write. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `to` | `Address` | Recipient of the funds. | +| `amount` | `i128` | Amount withdrawn. | + +**State change.** Tokens left the contract and `DataKey::Balances[token]` decreased by +`amount`. Admin-only, and panics if the balance is insufficient. + +This is also the event emitted when `proposals::execute_withdraw` calls into this contract +cross-contract, so it is the on-chain record that an approved governance withdrawal +actually moved funds. + +> **Payload gap.** As with `deposit`, `WithdrawEvent` omits the `token` address. + +### Proposal lifecycle + +#### `proposal_created` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_created"),)` | +| **Data type** | `ProposalCreatedEvent` | +| **Emitted in** | `propose_withdraw()`, `lib.rs:275` | +| **Order** | **After** the proposal and the proposer's auto-approval vote are written. | +| **Consumed** | βœ… Yes β†’ status `active` | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u32` | Proposal id, assigned from `ProposalCount`. | +| `proposer` | `Address` | Must be a member; auto-approves, so the proposal starts at `approvals = 1`. | +| `to` | `Address` | Withdrawal recipient if the proposal passes. | +| `token` | `Address` | Token to withdraw. | +| `amount` | `i128` | Amount requested; checked against the current balance at creation. | +| `expires_at` | `u64` | Unix timestamp, computed as `now + ttl_ledgers * 5` (β‰ˆ5 s per ledger). | + +**State change.** A new `WithdrawProposal` exists with `status = Active`, +`approvals = 1`, `rejections = 0`; `ProposalCount` incremented; the proposer's vote +recorded. + +#### `proposal_approved` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_approved"),)` | +| **Data type** | `ProposalApprovedEvent` | +| **Emitted in** | `approve_withdraw()`, `lib.rs:311` β€” **conditional**, only when the threshold is reached | +| **Order** | ⚠️ **Before** the write. The status is set on the in-memory struct and the event published, and only then is the proposal persisted (`lib.rs:320`). | +| **Consumed** | βœ… Yes β†’ status `approved` | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u32` | Proposal id. | +| `approvals` | `u32` | Approval count that met the threshold. | +| `threshold` | `u32` | The configured threshold, for context. | + +**State change.** The proposal transitioned `Active β†’ Passed`. It is now executable. + +Emitted at most once per proposal, only on the vote that crosses the threshold. Because +the whole call is one atomic transaction, the pre-write emission is not observable to an +off-chain consumer β€” if the transaction reverts, no event is delivered β€” but the ordering +is worth knowing when reading the contract. + +#### `proposal_rejected` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_rejected"),)` | +| **Data type** | `ProposalRejectedEvent` | +| **Emitted in** | `reject_withdraw()`, `lib.rs:361` β€” **conditional**, only at the blocking minority | +| **Order** | ⚠️ **Before** the write, same pattern as `proposal_approved` (persisted at `lib.rs:369`). | +| **Consumed** | βœ… Yes β†’ status `rejected` | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u32` | Proposal id. | +| `rejections` | `u32` | Rejection count that reached the blocking minority. | + +**State change.** The proposal transitioned `Active β†’ Rejected`. The blocking minority is +`member_count.saturating_sub(threshold) + 1` β€” the point at which the remaining members can +no longer reach `threshold` approvals. + +#### `withdraw_vote` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "withdraw_vote"),)` | +| **Data type** | `WithdrawVoteCastEvent` | +| **Emitted in** | `approve_withdraw()` `lib.rs:325` **and** `reject_withdraw()` `lib.rs:374` | +| **Order** | **After** the proposal write in both paths. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u32` | Proposal id. | +| `voter` | `Address` | The member who voted. | +| `approve` | `bool` | `true` = approve, `false` = reject. | + +**State change.** One vote recorded at `DataKey::Vote(id, voter)`, and the proposal's +`approvals` or `rejections` counter incremented by one. Each member may vote at most once. + +This is emitted on **every** vote, whereas `proposal_approved` / `proposal_rejected` fire +only on the transition. A vote that crosses the threshold therefore produces two events in +one transaction β€” the transition event first, then `withdraw_vote`. + +> **Note.** The proposer's auto-approval at creation does **not** emit a `withdraw_vote`, +> even though it is recorded as a vote. Counting `withdraw_vote` events undercounts +> approvals by one. + +--- + +## `proposals` + +Source: [`contracts/proposals/src/lib.rs`](../contracts/proposals/src/lib.rs), event +structs in [`storage.rs`](../contracts/proposals/src/storage.rs). Note this contract uses +`u64` proposal ids, whereas `group_treasury` uses `u32`. + +### `proposal_created` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_created"),)` | +| **Data type** | `ProposalCreatedEvent` (distinct from the `group_treasury` type of the same name) | +| **Emitted in** | `create_proposal()`, `lib.rs:100` | +| **Order** | **After** the proposal and `NextProposalId` are written. | +| **Consumed** | ⚠️ Only if this contract id is configured as `GROUP_TREASURY_CONTRACT_ID` | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id from `NextProposalId`. | +| `proposer` | `Address` | Creator; authorised via `require_auth()`. | +| `expires_at` | `u64` | Voting deadline; must be in the future. | +| `treasury` | `Address` | The `group_treasury` this proposal would withdraw from. | +| `token` | `Address` | Token to withdraw. | +| `to` | `Address` | Withdrawal recipient. | +| `amount` | `i128` | Amount requested; must be `> 0`. | + +**State change.** A new `Proposal` exists with `status = Active` and +`yes_votes = no_votes = 0`. Unlike `group_treasury`, the proposer does **not** auto-vote. + +> Note the shared topic name across two contracts with different payload shapes and +> different id widths. Consumers must disambiguate on the emitting contract id, not the +> topic alone. + +### `vote_cast` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "vote_cast"),)` | +| **Data type** | `VoteCastEvent` | +| **Emitted in** | `vote()`, `lib.rs:144` | +| **Order** | **After** both the vote key and the updated proposal are written. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id. | +| `voter` | `Address` | The voter. | +| `support` | `bool` | `true` = yes, `false` = no. | + +**State change.** `DataKey::Vote(id, voter)` set, and `yes_votes` or `no_votes` +incremented. One vote per address per proposal; voting after `expires_at` panics. + +### `proposal_finalized` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_finalized"),)` | +| **Data type** | `ProposalFinalizedEvent` | +| **Emitted in** | `finalize_proposal()`, `lib.rs:180` | +| **Order** | **After** the status write. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id. | +| `status` | `ProposalStatus` | The outcome: `Passed` or `Rejected`. | +| `yes_votes` | `u32` | Final yes tally. | +| `no_votes` | `u32` | Final no tally. | + +**State change.** `Active β†’ Passed` when `yes_votes > no_votes`, otherwise +`Active β†’ Rejected`. Callable by anyone, but only after `expires_at`. A tie rejects. + +> **Consumption gap.** This is the event that decides a governance proposal's outcome, and +> nothing consumes it. The backend has no listener-driven path to learn that a `proposals` +> vote passed. + +### `proposal_expired` + +| | | +| --- | --- | +| **Topic tuple** | `(Symbol::new(&env, "proposal_expired"),)` | +| **Data type** | `ProposalExpiredEvent` | +| **Emitted in** | `finalize_expired_proposal()`, `lib.rs:208` | +| **Order** | **After** the status write. | +| **Consumed** | ⚠️ Only if this contract id is configured as `GROUP_TREASURY_CONTRACT_ID` | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id. | + +**State change.** `Active β†’ Expired`. This is an alternative terminal path to +`finalize_proposal`: whichever is called first wins, since both require `Active`. + +### `executed` + +| | | +| --- | --- | +| **Topic tuple** | `(symbol_short!("executed"),)` | +| **Data type** | `ProposalExecutedEvent` | +| **Emitted in** | `execute_proposal()`, `lib.rs:228` | +| **Order** | **After** the status write. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id. | +| `executor` | `Address` | Whoever executed it. | + +**State change.** `Passed β†’ Executed`. This is the MVP execution path: it only flips the +status and emits, moving no funds. + +### `execut` + +| | | +| --- | --- | +| **Topic tuple** | `(symbol_short!("execut"),)` β€” note the truncated symbol | +| **Data type** | `ProposalExecutedEvent` | +| **Emitted in** | `execute_withdraw()`, `lib.rs:285` | +| **Order** | **After** the status write, and after the cross-contract withdrawal. | +| **Consumed** | ❌ No | + +| Field | Type | Meaning | +| --- | --- | --- | +| `id` | `u64` | Proposal id. | +| `executor` | `Address` | The treasury member who executed; must pass a membership check. | + +**State change.** `Passed β†’ Executed`, **and** funds have moved: the contract called +`group_treasury::withdraw` cross-contract before writing the status. That call causes +`group_treasury` to emit its own `withdraw` event in the same transaction. + +So an executed governance withdrawal produces two events from two contracts: +`group_treasury::withdraw` (funds moved) and `proposals::execut` (proposal closed). + +> **Naming defect.** Two different topics β€” `executed` and `execut` β€” signal the same +> logical transition with the same payload type. `symbol_short!` caps at 9 characters, so +> `"executed"` (8) was in range and the truncation in `execut` appears unintentional rather +> than forced. Consumers must match both. + +--- + +## Consumption gaps + +Collected here because these are the differences between what the contracts publish and +what the backend actually reacts to. Each is a real defect, not a design choice. + +### 1. The listener subscribes to a topic nothing publishes + +`stellarListener.ts` declares `TREASURY_TOPICS` as: + +``` +proposal_created, proposal_approved, proposal_rejected, proposal_executed, proposal_expired +``` + +**No contract publishes `proposal_executed`.** `proposals` publishes `executed` and +`execut`; `group_treasury` has no execution event at all. The listener's status map +contains `proposal_executed: 'executed'`, so the code path exists but is unreachable β€” +a proposal never reaches the `executed` status in the off-chain mirror by way of the +listener. The [treasury API doc](../../apps/backend/docs/api-treasury.md) and +[deployment doc](api-deployment-invocation.md) both list `proposal_executed` among the +watched events, which is accurate about the subscription and misleading about the effect. + +### 2. `group_treasury` has no execution event + +`group_treasury::withdraw` emits `withdraw`, but there is no +`proposal_executed`-equivalent tying a fund movement back to the proposal that authorised +it. A consumer watching only treasury topics cannot close the loop from +`proposal_approved` to "the money moved". + +### 3. Fund movement is entirely unconsumed + +`deposit` and `withdraw` β€” the two events that represent actual value movement β€” have no +consumer. Treasury balances shown off-chain cannot be maintained from the event stream as +it stands. + +### 4. Only one contract is watched for treasury topics + +The listener builds a single treasury fetcher from one `GROUP_TREASURY_CONTRACT_ID`. The +`proposals` contract is a separate deployment with its own id, so whichever id is +configured, the other contract's events are not polled. Since both contracts publish a +topic named `proposal_created` with different payloads and different id widths (`u32` vs +`u64`), pointing the listener at `proposals` would also mis-parse β€” the fetcher reads +`value.approvals` and `value.rejections`, which exist only on the `group_treasury` events. + +### 5. Vote-level events are unconsumed + +`withdraw_vote` and `vote_cast` are not consumed. Live vote tallies in the UI come from the +off-chain `proposalVotes` table written by the REST routes, not from chain. On-chain votes +cast directly against the contract are therefore invisible to the backend until a +transition event fires. + +--- + +## Related documents + +- [Backend chain listener source](../../apps/backend/src/services/stellarListener.ts) β€” the consumer +- [Treasury API](../../apps/backend/docs/api-treasury.md) β€” the REST surface and off-chain mirror +- [Deployment and invocation](api-deployment-invocation.md) β€” env vars that start the listener +- [Proposal lifecycle](concepts-proposal-lifecycle.md) β€” statuses and transitions in full +- [Token transfer flow](concepts-token-transfer-flow.md) β€” the in-chat payment path +- [System architecture overview](../../docs/architecture-overview.md) β€” how the listener fits the whole system From 46cb98f2396444257da8fc8394ae381f2ed0c7c1 Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:22:48 +0100 Subject: [PATCH 05/36] docs: add API versioning and deprecation policy Adds docs/api-versioning.md. States the current position plainly: REST routes are mounted unprefixed in app.ts, socket events are addressed by bare name with no version in the payload or handshake, and no deprecation machinery exists anywhere in apps/backend/src. Records the three breaking changes that already shipped without one - sequenceNumber dropped from GET /sync envelopes, device made required on POST /auth/verify, and JWT-only registration retired at POST /devices. Defines what breaks for REST responses, socket payloads, and database-backed shapes, including the case where a migration changes a response without any route being edited, and the asymmetry that strict request schemas make an added optional field breaking in the other direction. Gives the deprecation procedure: announce with a sunset date, mark per surface, ship the replacement first, hold the window, then retire behind an explicit error naming the replacement. The retired POST /devices handler, which still answers 403 pointing at the link-challenge flow, is the precedent the procedure generalises. Sets a 90-day minimum window, derived from the 7-day envelope retention floor plus stale tabs and cached service workers. Documents device capability negotiation as the mechanism that already ships protocol changes without breaking older clients, and names the properties that make it work: a universal sealed_box baseline, absence normalising to that baseline rather than erroring, unknown values preserved and ignored, and pairwise per-message selection. --- docs/api-versioning.md | 347 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 docs/api-versioning.md diff --git a/docs/api-versioning.md b/docs/api-versioning.md new file mode 100644 index 00000000..b9232ded --- /dev/null +++ b/docs/api-versioning.md @@ -0,0 +1,347 @@ +# API versioning & deprecation policy + +The backend's REST routes and socket events are consumed by a web client that +ships on its own schedule. Once a user has a tab open, or a service worker +cached, the client talking to production is not necessarily the client that was +built alongside the running server. This document states where the API stands on +compatibility today, what counts as a breaking change, and the procedure for +retiring a field, route, or event. + +--- + +## 1. Current position + +**REST routes are unversioned.** There is no `/v1` prefix, no `Accept` header +negotiation, and no version query parameter. Every router in +[`apps/backend/src/app.ts`](../apps/backend/src/app.ts) mounts at a bare path: + +```ts +app.use('/auth', authRouter); +app.use('/conversations', conversationsRouter); +app.use('/devices', devicesRouter); +app.use('/messages', messagesRouter); +app.use('/users', usersRouter); +app.use('/treasury', treasuryRouter); +app.use('/uploads', uploadsRouter); +app.use('/files', filesRouter); +app.use('/push', pushRouter); +app.use('/sync', syncRouter); +app.use('/user-devices', userDevicesRouter); +app.use('/security', securityRouter); +``` + +**Socket events are unversioned too.** Events are addressed by bare name +(`send_message`, `new_message`, `read_receipt`, …) with no version field in the +payload and none in the handshake. The `dispatch` envelope carries `type`, +`payload`, and `eventId` β€” no schema version. + +**There is no deprecation machinery.** The string `deprecat` does not appear +anywhere in `apps/backend/src`. No route sets a `Deprecation` or `Sunset` header, +no response carries a warning field, and no sunset dates are recorded anywhere. + +**Breaking changes have already shipped without a policy.** Three that are +visible in the current tree and its history: + +| Change | What broke for an older client | +| ------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sequenceNumber` removed from `GET /sync` envelopes | A client ordering an offline catch-up by `sequenceNumber` gets `undefined` on every envelope. `apps/backend/src/__tests__/sync.routes.test.ts` asserts the field's absence, so the removal is deliberate and enforced. The field still exists on socket receipt payloads, so it is gone from one surface and present on another. | +| `device` became **required** on `POST /auth/verify` | `VerifySchema` moved from `device: DeviceSchema.partial().optional()` to `device: DeviceSchema`. A client that authenticated without a device block now gets a 400 at login β€” the hardest possible failure, because it locks the user out entirely rather than degrading a feature. | +| `POST /devices` retired in favour of the link-challenge flow | JWT-only device registration is gone; devices must complete `POST /devices/link/challenge` then `POST /devices/link/verify`. | + +The third of those is the one that was handled well, and it is the model this +policy generalises β€” see [Β§4.1](#41-the-existing-precedent-post-devices). + +Note also that `VerifySchema` and the message schemas are `.strict()`: an +unrecognised field in a request body is rejected outright rather than ignored. +That is a deliberate safety property for key material (a client bug that sends +private state should be surfaced, not silently dropped) but it means **adding an +optional field to a strict request body is a breaking change for any client that +sends it to an older server**. Request bodies and response bodies do not follow +the same compatibility rules here. + +### The intended policy going forward + +1. **Additive-by-default.** New behaviour ships as new fields, new events, or new + routes. Existing shapes keep working. +2. **No silent removals.** Anything removed goes through the deprecation + procedure in [Β§4](#4-deprecation-procedure), with an announced sunset. +3. **Negotiate rather than version, where the change is in the protocol.** The + device capability mechanism ([Β§5](#5-capability-negotiation-the-working-precedent)) + already ships protocol changes without breaking older clients, and it is the + preferred tool. +4. **Version the path only as a last resort.** Introducing `/v2/` beside + the existing route is reserved for a change that cannot be made additive or + negotiated. When it happens, the unprefixed route remains the v1 route β€” it is + not retroactively moved β€” and it is deprecated on the normal schedule. + +The policy applies to what the server promises going forward. It does not +retroactively re-open the three changes above. + +--- + +## 2. What counts as a breaking change + +### 2.1 REST responses + +Breaking: + +- Removing a field, or making a previously always-present field optional. +- Renaming a field. (This is a removal plus an addition, and it is the most + common accidental break.) +- Changing a field's type or its representation β€” `number` β†’ `string`, an ISO + timestamp β†’ epoch millis, a bare id β†’ an object. +- Narrowing an enum-like value set, or adding a new value to one that clients + switch on exhaustively. +- Changing a success status code (`200` β†’ `204`), or turning a previously + successful request into an error. +- Changing pagination semantics β€” cursor format, page size limits, ordering. + `GET /sync` encodes its cursor as `:`; a client persists that + string across sessions, so changing the format strands every stored cursor. +- Moving a route to a different path, or adding a newly required parameter. +- Tightening validation on an existing field so previously accepted input is + rejected. + +Not breaking: + +- Adding a new optional field to a response. +- Adding a new route. +- Adding a new optional field to a **non-strict** request body. +- Making a previously required request field optional. +- Loosening validation. +- Changing an error _message_ string while the status code and error shape hold. + +### 2.2 Socket event payloads + +The same field-level rules apply, plus: + +Breaking: + +- Removing an outbound event, or renaming one. A client listening for the old + name simply never fires β€” there is no error, so this fails silently and is + worse than a REST break. +- Removing a field from an outbound payload, or changing its type. +- Requiring a new field on an inbound event. +- Changing which room or audience an event is broadcast to, so a client that + used to receive it no longer does. +- Changing the ack/response contract of an inbound event. +- Changing the handshake requirements. Auth already rejects tokens without + `deviceId` β€” `verifyToken` throws `Token missing deviceId β€” re-authentication +required` β€” which is a hard break for any pre-device-auth token. + +Not breaking: + +- Adding a new event. +- Adding an optional field to an outbound payload. +- Accepting a new optional field on an inbound event. + +Socket breaks deserve extra caution for two reasons: a long-lived connection +means an old client can be attached to a new server for hours, and a missing +event produces no error the client can detect or report. + +### 2.3 Database-backed shapes + +Some response shapes are the database schema in a thin wrapper, so a migration +can break the API without anybody editing a route. Treat a migration as an API +change whenever the column is reachable from a response: + +- Dropping or renaming a column that a route serialises is a **response-breaking + change**, even though the diff touches only `apps/backend/drizzle/`. +- Changing a column's type or nullability changes the response's type or + optionality. +- Changing a default changes what clients observe for existing rows. +- Removing an enum value from a column constraint narrows a value set clients may + already be switching on. + +Two rules follow: + +1. **Expand, migrate, contract.** Add the new column, dual-write, migrate readers, + and only then drop the old column β€” with the drop treated as a deprecation + under [Β§4](#4-deprecation-procedure) rather than as a schema tidy-up. +2. **Never serialise a table row directly.** Route handlers should map explicitly + to a response shape, so that a column rename is a compile error rather than a + silent contract change. `apps/backend/src/lib/messages.ts` already does this + deliberately for `content`, destructuring it out so a legacy plaintext column + can never reach a serialised response. + +--- + +## 3. Compatibility windows + +| Consumer | Assume it can be stale for | +| ---------------------------- | ------------------------------------------------------------------------- | +| Open browser tab | Hours to days β€” until reload | +| Cached service worker | Until the next activation | +| Device with queued envelopes | The envelope retention window, `ENVELOPE_TTL_SECONDS` (7 days by default) | + +The retention window is a floor, not a ceiling: a device offline for the whole +window reconnects, syncs, and immediately starts speaking whatever protocol it +knew a week ago. **Minimum support window for a deprecated field, event, or +route: 90 days** from the announcement in [Β§4.2](#42-procedure). Shorten it only +for a security fix, and say so explicitly when you do. + +--- + +## 4. Deprecation procedure + +### 4.1 The existing precedent: `POST /devices` + +When JWT-only device registration was retired, the route was not deleted. It was +left mounted, returning an explicit, actionable error: + +```ts +// ─── POST /devices β€” retired (#333) ────────────────────────────────────────── +// Kept only to return an explicit, actionable error: bare JWT-only device +// registration is gone. Clients must complete the link challenge instead. + +devicesRouter.post('/', (_req: AuthRequest, res) => { + res.status(403).json({ + error: + 'Device registration requires a fresh wallet signature. Use POST /devices/link/challenge then POST /devices/link/verify.', + }); +}); +``` + +An old client hitting it gets a status it can branch on and a message naming its +replacement, instead of a 404 that is indistinguishable from a typo or an outage. +That is the standard for every retirement: **the removed thing keeps answering, +and its answer says what to do instead.** + +### 4.2 Procedure + +1. **Announce.** Open an issue describing the change, the reason, the + replacement, and the sunset date (β‰₯ 90 days out). Record it in the + [deprecation register](#6-deprecation-register) in this file. +2. **Mark it.** How depends on what is being deprecated: + + | Surface | How it is marked | + | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | + | REST route | Respond with `Deprecation: true` and `Sunset: ` headers, plus a `Link: ; rel="deprecation"` pointing at the issue. Keep serving normally. | + | REST field | Keep returning it, populated. Document it as deprecated in the route's doc under `apps/backend/docs/` and note the replacement field. | + | Socket event | Keep emitting it alongside its replacement. There is no header channel on a socket, so the register in this file and the event catalog are the notice. | + | Socket field | Keep populating it alongside the replacement field in the same payload. | + | DB column | Stop reading it, keep writing it, and mark it deprecated in the schema. | + +3. **Ship the replacement first.** The new field, event, or route must be live and + documented before the old one is marked, so a client can migrate the moment it + sees the notice. +4. **Notify.** Update the relevant doc under `apps/backend/docs/` (the + [WebSocket event catalog](../apps/backend/docs/api-websocket-events.md) for + events, the matching `api-*.md` for routes), add the register entry here, and + flag it in the PR description. There is no runtime client-notification channel + today; adding a deprecation array to `GET /health` would be the natural place + if one is wanted. +5. **Wait out the window.** Both paths work for the full period. +6. **Retire.** Replace the implementation with an explicit error in the style of + Β§4.1 β€” a `410 Gone` for a removed route, `403` where a different flow must be + used β€” naming the replacement. Do not delete the handler. Move the register + entry to _Retired_. + +### 4.3 Emergency changes + +A security fix may skip the window. It must still: ship an explicit error rather +than a silent removal, get a register entry recording that the window was +skipped and why, and state what a stranded client should do. `verifyToken` +rejecting tokens without `deviceId` is an example of this class β€” the break is +deliberate and the error message tells the client to re-authenticate. + +--- + +## 5. Capability negotiation: the working precedent + +For protocol-level changes, the codebase already has a mechanism that ships +breaking protocol work without breaking older clients: +[`apps/backend/src/lib/capabilities.ts`](../apps/backend/src/lib/capabilities.ts). + +Each device advertises a small JSON document at registration, and can update it +later without re-registering its identity key: + +```ts +protocols; // e.g. ["sealed_box", "signal", "mls"] β€” schemes this device can decrypt +ciphersuites; // MLS/Signal ciphersuite identifiers, meaningful when "mls" is present +fileTransfer; // file-encryption scheme versions, e.g. ["file-v1"] +``` + +The properties that make it work are worth naming, because they are what any +future negotiation mechanism should copy: + +- **A universal baseline.** `sealed_box` is the scheme every device in the + codebase implements, and it is the floor of every negotiation. +- **Absence is a valid answer.** A device that never advertised capabilities β€” + including rows written before the column existed β€” normalises to the + sealed_box-only baseline instead of erroring. `normalizeCapabilities` returns + the baseline for `null`, `undefined`, and malformed input alike. +- **Unknown values are preserved and ignored.** `selectProtocol` walks a known + priority list rather than rejecting names it does not recognise, so an older + server meeting a newer client's advertisement degrades instead of failing. +- **Negotiation is pairwise and per-message.** `selectProtocol(a, b)` picks the + strongest scheme _both_ sides support, so a rollout proceeds device by device + with no flag day. +- **Advertising is a non-breaking addition.** `capabilities` is optional on + `DeviceSchema`; omitting it is well-defined. + +The same shape applies beyond encryption. `fileTransfer` already carries versioned +scheme identifiers (`file-v1`) checked by `supportsFileTransfer`, which is exactly +how a versioned payload format should be gated. + +**Use this first.** A protocol or payload-format change that can be expressed as +a capability should be, rather than as a versioned route: it needs no client +coordination, no sunset date, and no dual-serving window. + +Its limit is that it is scoped to devices and to encryption/file-transfer +concerns. It says nothing about REST response shapes, and there is no equivalent +negotiation for a client's understanding of a JSON field. Those still need the +deprecation procedure in [Β§4](#4-deprecation-procedure). + +A second, narrower precedent lives in the socket layer: the +[envelope-wrapper vs. legacy-raw-emit](../apps/backend/docs/api-websocket-events.md#envelope-wrapper-vs-legacy-raw-emit) +split, where both emission styles are supported concurrently and new client code +is directed at the envelope. That is the dual-serving half of Β§4.2 already in +practice β€” what it lacks is an announced sunset for the legacy style. + +--- + +## 6. Deprecation register + +The canonical list of what is deprecated, what replaces it, and when it goes. +Every deprecation adds a row here as step 1 of [Β§4.2](#42-procedure). + +### Active deprecations + +| Surface | Deprecated | Replacement | Announced | Sunset | +| --------------- | ---------- | ----------- | --------- | ------ | +| _none recorded_ | | | | | + +Legacy-raw-emit on the socket is a candidate for the first entry: it is already +dual-served and already documented as non-preferred, but has no announced sunset. + +### Retired + +| Surface | Retired | Replacement | Behaviour now | +| ------------------------------------ | ------------------------------------------------- | ------------------------------------------------------------------ | -------------------------------------------------------------------------- | +| `POST /devices` | JWT-only device registration (#333) | `POST /devices/link/challenge` β†’ `POST /devices/link/verify` | `403` naming the replacement flow | +| `GET /sync` β†’ `sequenceNumber` | Per-conversation sequence number as a sync cursor | Opaque `nextCursor` (`:`), stable across conversations | Field absent; asserted by `sync.routes.test.ts` | +| `POST /auth/verify` without `device` | Deviceless authentication | `device` block required on every verify | `400` from schema validation | +| JWT without `deviceId` | Pre-device-aware tokens | Re-authenticate | `verifyToken` throws `Token missing deviceId β€” re-authentication required` | + +--- + +## 7. Checklist for a change that touches the API + +- [ ] Diff reviewed against [Β§2](#2-what-counts-as-a-breaking-change) β€” response fields, socket payloads, and any migration. +- [ ] If breaking: can it be additive instead? Can it be a capability instead? +- [ ] If it must break: replacement shipped and documented first. +- [ ] Register row added with a sunset date β‰₯ 90 days out. +- [ ] Route doc under `apps/backend/docs/` updated; event catalog updated for socket changes. +- [ ] Retirement returns an explicit error naming the replacement β€” never a bare 404 or a silently missing event. +- [ ] Migrations reviewed as API changes, not just schema changes. + +--- + +## 8. Related documents + +- [WebSocket event catalog](../apps/backend/docs/api-websocket-events.md) β€” every event, its payload, and the envelope/legacy split +- [REST schemas](../apps/backend/docs/contracts-rest-schemas.md) β€” request/response shapes this policy governs +- [WebSocket payload contracts](../apps/backend/docs/contracts-websocket-payloads.md) +- [Devices & prekeys API](../apps/backend/docs/api-devices.md) β€” the routes carrying the retired `POST /devices` and the link flow that replaced it +- [Message sync API](../apps/backend/docs/api-messages-sync.md) β€” the cursor contract referenced in Β§2.1 +- [Message encryption migration](../apps/backend/docs/message-encryption-migration.md) and [Signal migration](../apps/backend/docs/signal-migration.md) β€” capability negotiation applied to a live protocol rollout From 4c34c6e6b1504044fc58cd8e18932822d1ff8d46 Mon Sep 17 00:00:00 2001 From: Chidimj <165854539+Chidimj@users.noreply.github.com> Date: Sun, 30 Aug 2026 03:23:09 +0100 Subject: [PATCH 06/36] docs: add WASM size and resource budget doc (#582) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds contracts/docs/concepts-resource-budget.md covering the deployed size constraint and the wider resource picture around it. Documents the gate as CI actually implements it: 102,400 bytes per contract, enforced in the "Report WASM binary sizes" step of contracts-ci.yml, run per-contract across a fail-fast:false matrix, measured against the raw cargo build output with no wasm-opt step, and posted to the PR as an update-in-place comment keyed on a marker comment. Notes that the gate is per-contract rather than cumulative and that the step reports the full table before failing. Records current sizes measured from a local release build, with headroom: group_treasury 31,843 B (31.1% of gate), proposals 25,931 B (25.3%), token_transfer 10,539 B (10.3%). Includes the command to reproduce them, since sizes drift with soroban-sdk upgrades. Documents each [profile.release] setting and the cost of changing it, including that overflow-checks = true costs size deliberately and should not be traded away in contracts that move funds. Gives concrete size-reduction techniques in effort order: profile with twiggy first, run stellar contract optimize, replace panic string literals with a #[contracterror] enum, keep formatting machinery out of the shipped path, deduplicate generic instantiations, split along a real boundary, and only then raise the threshold. Closes with the resources beyond raw size β€” CPU instructions, memory, ledger entry reads/writes, events, rent/TTL, and transaction size β€” and identifies the constraint that actually binds first in this codebase: all three contracts keep per-proposal and per-voter data in instance storage, which grows unbounded and is read and written whole on every call, and several functions do linear scans over it. --- contracts/docs/concepts-resource-budget.md | 244 +++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 contracts/docs/concepts-resource-budget.md diff --git a/contracts/docs/concepts-resource-budget.md b/contracts/docs/concepts-resource-budget.md new file mode 100644 index 00000000..f8a0ddcb --- /dev/null +++ b/contracts/docs/concepts-resource-budget.md @@ -0,0 +1,244 @@ +# WASM Size and Resource Budget + +Deployed contract size is enforced in CI. This document covers what the gate is, where it +lives, how much room each contract currently has, which build settings move the number, +what to do when a contract approaches the limit, and why raw size is only one of several +budgets a Soroban contract has to live within. + +--- + +## The gate + +**Every release WASM must be ≀ 102,400 bytes (100 KB). A contract over the limit fails +the build.** + +Enforced in [`.github/workflows/contracts-ci.yml`](../../.github/workflows/contracts-ci.yml), +in the `Report WASM binary sizes` step of the `test-and-build` job: + +```bash +THRESHOLD_BYTES=102400 # 100 KB +... +if [ "$bytes" -gt "$THRESHOLD_BYTES" ]; then + echo "::error file=${name}::WASM size ${bytes} bytes exceeds ${THRESHOLD_BYTES} byte limit" + FAILED=1 +fi +... +exit $FAILED +``` + +Mechanics worth knowing: + +- **Triggers** on any push or pull request touching `contracts/**` or the workflow file + itself, and on a weekly Monday 08:00 UTC schedule. +- **Runs per contract.** The job is a matrix over `token_transfer`, `group_treasury`, and + `proposals`, with `fail-fast: false`, so one oversized contract does not mask the others. +- **Measures the plain `cargo build --release` artifact** from + `target/wasm32-unknown-unknown/release/`. There is no post-processing step in CI β€” no + `wasm-opt`, no `stellar contract optimize` β€” so the number gated is the raw compiler + output. +- **Reports before it fails.** The step is `if: always()` and computes the whole size table + before exiting non-zero, so a failing run still shows every contract's size. +- **Posts a size table to the pull request** as an update-in-place comment, keyed on a + `` marker so repeated pushes edit one comment instead of + accumulating. The same table is written to the job summary. Comment failures are + swallowed with `|| true`; they do not fail the build. +- **The gate is per-contract, not cumulative.** Three contracts at 90 KB each all pass. + +> The size step shells out to `bc`. It is present on the `ubuntu-latest` runner but is not +> installed by default in every environment β€” worth knowing if you reproduce this step +> locally and it fails on a missing command rather than on size. + +--- + +## Current sizes and headroom + +Measured from `cargo build --release --target wasm32-unknown-unknown` at the current +workspace settings: + +| Contract | Size | % of gate | Headroom | +| --- | --- | --- | --- | +| `group_treasury` | 31,843 B (31.10 KB) | 31.1 % | 70,557 B (68.90 KB) | +| `proposals` | 25,931 B (25.32 KB) | 25.3 % | 76,469 B (74.68 KB) | +| `token_transfer` | 10,539 B (10.29 KB) | 10.3 % | 91,861 B (89.71 KB) | + +**Nothing is close to the limit.** The largest contract uses under a third of its budget, +and every contract could roughly triple in size before CI complains. Reproduce these +numbers with: + +```bash +cd contracts +cargo build --release --target wasm32-unknown-unknown +wc -c target/wasm32-unknown-unknown/release/*.wasm +``` + +Sizes will drift with `soroban-sdk` upgrades as much as with your own code, so treat the +table as a snapshot rather than a constant β€” the reproduction command above is the +authority. + +The relative ordering is what you would expect from the source: `group_treasury` carries +membership management, per-token balances, and a full proposal-voting flow; `proposals` +carries a voting flow plus a cross-contract client; `token_transfer` is a thin authorised +wrapper over a SEP-41 transfer. + +--- + +## Release profile settings that affect size + +From [`contracts/Cargo.toml`](../Cargo.toml): + +```toml +[profile.release] +opt-level = "z" +overflow-checks = true +debug = 0 +strip = "symbols" +debug-assertions = false +panic = "abort" +codegen-units = 1 +lto = true +``` + +Every one of these is already tuned for size. What each does, and what it costs to change: + +| Setting | Effect on size | Cost of changing it | +| --- | --- | --- | +| `opt-level = "z"` | Optimises for size over speed; the single biggest lever. | Moving to `"s"` trades a little size for a little speed; `3` inflates size significantly. On Soroban, CPU is metered separately and code size is charged at deploy, so `"z"` is usually right. | +| `lto = true` | Whole-program link-time optimisation; removes cross-crate dead code. Large win against a big dependency like `soroban-sdk`. | Slower builds. Disabling it inflates size noticeably. | +| `codegen-units = 1` | One codegen unit lets LLVM see everything at once, improving both inlining and dead-code elimination. | Slower, non-parallel builds. Purely a build-time cost. | +| `strip = "symbols"` | Drops the symbol table from the artifact. | Backtraces lose names β€” irrelevant here, since `panic = "abort"` means no unwinding anyway. | +| `debug = 0` | No debug info in the binary. | No source-level debugging of the deployed artifact. | +| `panic = "abort"` | No unwinding tables or landing pads. Required for Soroban regardless. | Not optional in practice. Panics trap; there is nothing to unwind into. | +| `debug-assertions = false` | Drops `debug_assert!` and related checks. | Those checks stop running in release. | +| `overflow-checks = true` | **Costs** size β€” it keeps arithmetic overflow checks in the release build. | **Do not turn this off to save bytes.** These contracts move funds; a silent `i128` wraparound in a balance is a critical bug. This is a deliberate trade of size for safety, and the current headroom means there is no reason to revisit it. | + +There is also a `release-with-logs` profile that inherits `release` and re-enables +`debug-assertions`, for local debugging. It is not what CI measures. + +--- + +## When a contract approaches the limit + +In rough order of effort-to-payoff. Measure after each step β€” guessing at what is large is +usually wrong. + +### 1. Find out what is actually big + +Before changing anything, look at the breakdown: + +```bash +cargo install twiggy +twiggy top -n 30 target/wasm32-unknown-unknown/release/group_treasury.wasm +twiggy dominators target/wasm32-unknown-unknown/release/group_treasury.wasm +``` + +`twiggy top` ranks by retained size; `dominators` shows which item's removal would actually +free space. Frequently the bulk is a formatting or serialisation path pulled in by a single +call site. + +### 2. Run the optimiser + +CI gates the raw build output, but a deployed contract does not have to be that artifact: + +```bash +stellar contract optimize --wasm target/wasm32-unknown-unknown/release/group_treasury.wasm +``` + +This runs `wasm-opt` and typically removes a meaningful fraction. Note the asymmetry β€” the +optimised artifact is smaller than the number CI gates, so a contract that passes CI is +comfortably deployable, and a contract that *fails* CI may still be deployable. Fix the +source rather than relying on that gap. + +### 3. Remove panic message strings + +Every distinct `panic!("...")` literal is bytes in the data section. These contracts panic +with descriptive strings throughout (`"insufficient funds"`, `"proposal not found"`, +`"already voted"`, and so on). Converting to a `#[contracterror]` enum with numeric codes +removes the strings and gives callers a typed error instead: + +```rust +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[repr(u32)] +pub enum Error { + InsufficientFunds = 1, + ProposalNotFound = 2, + AlreadyVoted = 3, +} +``` + +This is usually the largest available win in a contract of this style, and it improves the +client experience rather than degrading it. + +### 4. Avoid formatting machinery + +`format!`, `write!`, and `{:?}` pull in Rust's formatting infrastructure, which is large +relative to a 100 KB budget. `#[derive(Debug)]` on a type that is never actually printed +is usually free after LTO, but an actual `{:?}` call site is not. In `no_std` Soroban code +these should not appear in the shipped path at all β€” keep them behind `#[cfg(test)]`. + +### 5. Deduplicate generics and large call sites + +A generic function instantiated over many types produces a copy per instantiation. If a +helper is used across many `DataKey` variants, having it take a concrete type β€” or funnel +through one non-generic inner function β€” collapses several copies into one. The same +applies to a large function inlined at many call sites; `#[inline(never)]` can shrink the +total. + +### 6. Split the contract + +If one contract genuinely needs more than 100 KB of logic, that is a signal to split it +along a real boundary and call across contracts, as `proposals` already does with +`group_treasury`. Cross-contract calls cost CPU instructions at runtime, so this trades +deploy size for execution cost β€” make it a design decision, not a size workaround. + +### 7. Raise the gate β€” last resort + +`THRESHOLD_BYTES` in the workflow is a project policy, not a protocol limit. Raising it is +legitimate if a contract has genuinely outgrown the budget, but do it deliberately and in +its own commit with the reasoning, not as a way to make a red build green. + +--- + +## Size is not the only budget + +A contract that fits in 100 KB can still be too expensive to use. Soroban meters several +resources independently, and each has its own ceiling and its own fee component: + +- **CPU instructions.** Metered per invocation against a per-transaction ceiling. Loops + over unbounded collections are the usual way to exceed it. Several functions here iterate + the full member set (`is_member`, `add_member`, `remove_member`) or the full proposal + range (`list_proposals`, `get_pending_proposals`), so their cost grows linearly with the + treasury's membership and proposal history. These are the parts that will hit a ceiling + long before code size does. +- **Memory.** A per-invocation limit on linear memory. Materialising a large `Vec` β€” for + example building a list of every proposal ever created β€” is the usual cause. +- **Ledger entry reads and writes.** Both the number of entries touched and their total + byte size are metered, and writes cost considerably more than reads. Note that these + contracts keep everything in **instance** storage, including per-proposal entries and + per-voter vote keys, so the instance entry grows without bound as proposals and votes + accumulate β€” and the whole entry is read and written on every call that touches it. This + is the most likely resource problem in this codebase. +- **Events.** Event topics and data count toward the transaction's resource usage. The + events published here are small, but emitting one per vote in a large group adds up. +- **Rent / TTL.** Contract data expires unless its TTL is extended, and the cost of + extension scales with entry size and duration. A large, long-lived instance entry is a + recurring cost, not a one-off. This is the second reason to care about instance-storage + growth: it is charged for as long as the contract lives, whereas code size is charged + once at deploy. +- **Transaction size.** The signed transaction, including arguments and the authorisation + footprint, has its own limit, independent of the contract's size. + +**Practical upshot:** the 100 KB gate protects deployability, and at 31 % of budget it is +not the binding constraint today. The constraint that will bind first is unbounded growth +in instance storage and the linear scans over it. When optimising a contract here, look at +storage layout and iteration before looking at code size. + +--- + +## Related documents + +- [Contracts README](../README.md) β€” workspace layout, toolchain, build and test +- [Deployment and invocation](api-deployment-invocation.md) β€” deploying and initialising each contract +- [Token transfer storage](contracts-token-transfer-storage.md) β€” storage keys and value types +- [Proposal lifecycle](concepts-proposal-lifecycle.md) β€” statuses and transitions +- [Contract events reference](contracts-events.md) β€” every published event and its payload From f473432b9d47f8418240a99f13c87457de0f5274 Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:27:30 +0100 Subject: [PATCH 07/36] docs(ai_agent): add configuration and environment reference Adds apps/ai_agent/docs/configuration.md. Documents that OPENAI_API_KEY is the only environment variable main.py reads, with required status, no default, and no .env loading, and lists the variables the service does not read - model selection, Weaviate host/port/key, HOST and PORT - since setting those silently does nothing. The model choices, bind address, collection name, search limit, high-value threshold and per-call timeouts are tabulated as hardcoded values, with the note that the 0.0.0.0:8000 literals only apply to direct python main.py execution and not to fastapi/uvicorn launches. Documents the missing-API-key behaviour precisely: the key is read per-request inside _openai_client(), so the process starts clean, /health returns 200 and every OpenAI-backed endpoint returns 500. Both halves are pinned by tests. Spells out the load-balancer consequence - an instance with a bad key passes its liveness probe, joins the pool and serves nothing but 500s - and records the endpoint-by-endpoint matrix, including the high-value /transfers/analyse branch that answers 200 without ever constructing a client. Documents the Weaviate connection as an unconfigurable per-request connect_to_local() on the client's localhost defaults, the 503 failure mode, that infra/docker-compose.yml ships no Weaviate service, and that a containerised agent cannot reach a sibling container over loopback. Cross-links the repo-wide .env.example as the single environment reference. --- apps/ai_agent/docs/configuration.md | 289 ++++++++++++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 apps/ai_agent/docs/configuration.md diff --git a/apps/ai_agent/docs/configuration.md b/apps/ai_agent/docs/configuration.md new file mode 100644 index 00000000..e83c80f5 --- /dev/null +++ b/apps/ai_agent/docs/configuration.md @@ -0,0 +1,289 @@ +# AI agent configuration & environment reference + +Every setting the `ai_agent` service reads, what happens when one is missing, and +which settings are not configurable at all. + +The headline is short, and it is deliberately stated up front because it is the +opposite of what most services look like: **the service reads exactly one +environment variable, `OPENAI_API_KEY`.** Everything else β€” the model, the +Weaviate connection, the bind host and port β€” is hardcoded in +[`apps/ai_agent/main.py`](../main.py). Changing any of them is a code change, not +a deployment setting. + +--- + +## 1. Environment variables + +| Variable | Required | Default | Read at | Effect when unset | +| ---------------- | -------- | ------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `OPENAI_API_KEY` | Yes\* | _none_ | Per request, inside `_openai_client()` β€” not at import and not at boot | The process still starts and `/health` still returns 200; every OpenAI-backed endpoint returns 500. See [Β§3](#3-startup-and-missing-api-key-behaviour). | + +\* Required for the service to do anything useful. It is _not_ required for the +process to start, and that distinction is the single most important thing in this +document. + +It is the same key the backend uses β€” it is declared once in the repo-root +[`.env.example`](../../../.env.example) under `# AI Service`: + +```bash +# AI Service +OPENAI_API_KEY= +``` + +There is no `.env` loading in `main.py` β€” no `python-dotenv`, no +`pydantic-settings`. The variable must be present in the process environment. In +local development that is whatever your shell or process manager exports; in +Docker or Kubernetes it is the container environment. + +### Variables the service does **not** read + +Worth stating explicitly, because their absence is easy to mistake for a bug: + +| Variable | Status | +| -------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `OPENAI_MODEL` (or any model-selection variable) | Not read. Models are hardcoded β€” see [Β§2](#2-hardcoded-settings). | +| `WEAVIATE_URL` / `WEAVIATE_HOST` / `WEAVIATE_PORT` | Not read. The connection is `weaviate.connect_to_local()` β€” see [Β§4](#4-weaviate-connection). | +| `WEAVIATE_API_KEY` | Not read. The client connects unauthenticated. | +| `HOST` / `PORT` | Not read. See [Β§2](#2-hardcoded-settings). | +| `OPENAI_BASE_URL` | Not read. `OpenAI(api_key=...)` is constructed with no base URL override, so the SDK's own environment handling is the only way to redirect it. | + +Setting any of these has no effect on this service. + +--- + +## 2. Hardcoded settings + +| Setting | Value | Where | +| -------------------- | ------------------------------------------------------ | ---------------------------------------------------- | +| Chat model | `gpt-4o-mini` | `chat`, `analyse_transfer`, `summarise_proposal` | +| Embedding model | `text-embedding-3-small` | `index_message`, `search_messages` | +| Bind host | `0.0.0.0` | `uvicorn.run(...)` under `if __name__ == "__main__"` | +| Bind port | `8000` | same | +| Weaviate collection | `Message` | `index_message`, `search_messages` | +| Search result limit | `5` | `search_messages` | +| High-value threshold | `10_000.0` XLM | `_HIGH_VALUE_THRESHOLD`, used by `analyse_transfer` | +| Request timeouts | 30 s for `/chat`, 10 s for the two JSON-mode endpoints | passed per call as `timeout=` | +| System prompt | `_SYSTEM_PROMPT` | module level | + +Two consequences: + +- **Model changes require a code change and a deploy.** There is no way to move + to a different model, or to run two environments on different models, through + configuration. +- **The host and port literals only apply to `python main.py`.** Running the + module directly executes the `__main__` block and binds `0.0.0.0:8000`. Under + any ASGI server invoked against the app object β€” which is how the repo README + runs it (`uv run fastapi dev main.py`) and how a container normally runs it β€” + the `__main__` block never executes and the host/port come from that command: + + ```bash + # __main__ block runs: binds 0.0.0.0:8000 + python main.py + + # __main__ block does NOT run: fastapi/uvicorn decides the bind address + uv run fastapi dev main.py # defaults to 127.0.0.1:8000 + uv run uvicorn main:app --host 0.0.0.0 --port 9000 + ``` + + So the port is effectively set by the launch command, and the `8000` in + `main.py` is a default only for the direct-execution path. A deployment that + needs a different port sets it on the server command line. + +--- + +## 3. Startup and missing-API-key behaviour + +`OPENAI_API_KEY` is read **per request**, inside the `_openai_client()` helper: + +```python +def _openai_client(): + api_key = os.environ.get("OPENAI_API_KEY") + if not api_key: + raise HTTPException(status_code=500, detail="OPENAI_API_KEY is not configured") + if OpenAI is None: + raise HTTPException(status_code=500, detail="openai package is not installed") + + return OpenAI(api_key=api_key) +``` + +Nothing validates it at import time or at startup. The service therefore **starts +successfully with no API key at all** and fails only when an OpenAI-backed +endpoint is called. + +### The exact behaviour, endpoint by endpoint + +| Endpoint | With no `OPENAI_API_KEY` | +| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET /health` | **200** `{"status": "ok"}` | +| `POST /chat` | **500** `{"detail": "OPENAI_API_KEY is not configured"}` | +| `POST /transfers/analyse` | **500** β€” unless `amount > 10000`, which is answered by the rule-based branch **before** any OpenAI client is constructed, and still returns **200** | +| `POST /proposals/summarise` | **500** | +| `POST /index/message` | **500** if Weaviate is reachable (the embedding call needs the key); **503** if Weaviate is unreachable, because the connection is attempted first | +| `GET /search` | Same as above β€” **503** on a Weaviate failure, **500** on the missing key when Weaviate is up, and **200** with `{"results": []}` when the `Message` collection does not exist, because that path returns before any embedding is requested | + +### This is deliberate, and it is load-balancer-visible + +`/health` is a liveness probe: it answers whether the process is up and serving, +not whether every downstream dependency is configured. Because it does not touch +`_openai_client()`, a misconfigured deployment presents as: + +- **healthy** to any load balancer, orchestrator probe, or uptime monitor + pointed at `/health`, and +- **completely broken** to every real caller, with a 500 on each request. + +That combination is the failure mode to watch for. An instance rolled out with a +missing or misspelled key will pass its health check, be added to the pool, and +serve nothing but 500s. Kubernetes will not restart it; a load balancer will not +drain it. + +The test suite pins this behaviour rather than leaving it incidental β€” +`apps/ai_agent/tests/test_health.py::test_health_works_without_api_key` asserts +the 200, and `apps/ai_agent/tests/test_chat.py::test_missing_api_key_returns_500` +asserts the 500. Changing either is a deliberate contract change. + +**Operational recommendation.** Do not rely on `/health` to catch a +misconfiguration. Either: + +- assert the key is present at deploy time (a startup check in your orchestration, + or a required secret rather than an optional env var), or +- monitor the 5xx rate on `/chat` separately from the liveness probe, or +- add a readiness endpoint that checks configuration β€” deliberately distinct from + `/health`, which should keep its current semantics. + +### Related failure: the `openai` package missing + +`main.py` imports `openai` inside a `try/except ImportError` and sets +`OpenAI = None` when it is absent, so an environment without the dependency also +starts cleanly and fails per-request with **500** +`{"detail": "openai package is not installed"}`. Same shape of problem, same +`/health`-still-green consequence. + +--- + +## 4. Weaviate connection + +### How it connects + +```python +client = weaviate.connect_to_local() +``` + +No host, no port, no API key, no gRPC configuration. `connect_to_local()` from +`weaviate-client` (pinned at 4.22.0 in [`uv.lock`](../uv.lock), declared as +`>=4.0.0` in [`pyproject.toml`](../pyproject.toml)) uses its own defaults β€” +HTTP on `localhost:8080` and gRPC on `localhost:50051` β€” and this service +overrides none of them. + +A connection is opened **per request** on `POST /index/message` and `GET /search`, +and closed in a `finally` block. There is no pooled or long-lived client, and no +connection is attempted at startup. + +**There is no Weaviate service in [`infra/docker-compose.yml`](../../../infra/docker-compose.yml).** +That file brings up Postgres, Redis, and MinIO only. A Weaviate instance must be +run separately, and it must be reachable on the loopback defaults above from the +perspective of the `ai_agent` process β€” which means a containerised `ai_agent` +talking to a Weaviate in another container will not connect without host +networking or a code change, since `localhost` inside the container is the +container itself. + +### Failure mode when Weaviate is unreachable + +Both Weaviate-backed endpoints wrap the connection in `try/except` and translate +any failure to **503**: + +```python +try: + client = weaviate.connect_to_local() +except Exception: + raise HTTPException(status_code=503, detail="Weaviate connection failed") +``` + +| Condition | Response | +| ----------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| Weaviate down / unreachable | **503** `{"detail": "Weaviate connection failed"}` | +| Weaviate up, later operation fails | **503** with the underlying exception text as `detail` β€” the second `except Exception as e` block re-raises as 503 with `str(e)` | +| Weaviate up, `Message` collection missing | `GET /search` β†’ **200** `{"results": []}`; `POST /index/message` creates the collection and proceeds | +| Weaviate up, `OPENAI_API_KEY` missing | **500** β€” the embedding call fails after the connection succeeds | + +Two points that matter operationally: + +- **`/health` does not check Weaviate either.** A deployment with no Weaviate + reachable is green on its liveness probe and serves 503s on `/search` and + `/index/message`, exactly as with a missing API key. Chat and proposal + summarisation are unaffected. +- **The 503 detail can leak internals.** The second handler passes `str(e)` + straight into the response body, so a client can see raw client-library error + text. Treat that as an internal-facing detail, not a stable contract. + +`apps/ai_agent/tests/test_search.py::test_weaviate_connection_failure_returns_503` +pins the 503 behaviour, and `test_missing_collection_returns_empty_results` pins +the empty-result path, including that no query is issued. Note that the +equivalent failure on `POST /index/message` has no test of its own β€” the coverage +is on the `/search` side only. + +### Collection schema + +The collection is created on demand by `POST /index/message` with +`client.collections.create(name="Message")` β€” no property schema and no +vectoriser, because vectors are supplied explicitly from OpenAI embeddings. +Properties written per object are `conversationId`, `messageId`, `senderId`, and +`content`; searches filter on `conversationId` and return the top 5 by vector +similarity. See +[`contracts-weaviate-schema.md`](./contracts-weaviate-schema.md) for the full +shape and [`concepts-rag-search-architecture.md`](./concepts-rag-search-architecture.md) +for how it is used. + +--- + +## 5. Running the service + +From the repo root, as documented in the [main README](../../../README.md): + +```bash +cd apps/ai_agent && uv run fastapi dev main.py +``` + +With the API key set for the process: + +```bash +OPENAI_API_KEY=sk-... uv run fastapi dev main.py +``` + +Tests (Python 3.12+, per `requires-python` in `pyproject.toml`): + +```bash +cd apps/ai_agent +uv sync --group dev +uv run pytest +``` + +The suite sets `OPENAI_API_KEY=test-key` for every test through an autouse +fixture in [`tests/conftest.py`](../tests/conftest.py) and mocks both `OpenAI` +and `weaviate.connect_to_local`, so **no network access and no real key are +needed** β€” and, as a corollary, a real misconfiguration will never be caught by +running the tests. + +CI runs ruff, ruff format, mypy, and pytest with coverage via +[`.github/workflows/ai-agent-ci.yml`](../../../.github/workflows/ai-agent-ci.yml). + +--- + +## 6. Configuration checklist for a deployment + +- [ ] `OPENAI_API_KEY` is present in the process environment β€” verified by something other than `/health`. +- [ ] The `openai` package is installed in the runtime image (`uv sync`), or every request 500s. +- [ ] If `/search` or `/index/message` are used: a Weaviate instance is running and reachable at the client's local defaults **from the ai_agent process's own network namespace**. +- [ ] Bind host and port set on the server command line, not expected from env. +- [ ] Monitoring watches the 5xx rate on `/chat` and the 503 rate on `/search`, not just liveness. +- [ ] Model choice reviewed as a code change β€” `gpt-4o-mini` and `text-embedding-3-small` are compiled in. + +--- + +## 7. Related documents + +- [Repo-wide environment reference](../../../.env.example) β€” the single place every service's variables are declared, including the `OPENAI_API_KEY` this service shares with the backend. There is no separate env-reference document; `.env.example` is it. +- [Main README](../../../README.md) β€” how the AI service is started alongside the web and backend apps +- [`POST /chat`](./api-chat.md), [`POST /transfers/analyse`](./api-transfers-analyse.md), [`POST /proposals/summarise`](./api-proposals-summarise.md), [`/index` & `/search`](./api-index-search.md) β€” per-endpoint request/response contracts +- [Weaviate schema contract](./contracts-weaviate-schema.md) β€” the `Message` collection this service creates and queries +- [RAG search architecture](./concepts-rag-search-architecture.md) β€” how indexing and search fit together +- [Pydantic model contracts](./contracts-pydantic-models.md) β€” request/response models referenced above From 077fd46be4d9caf71f4cb0995d28da63741777eb Mon Sep 17 00:00:00 2001 From: Depo-dev Date: Sun, 30 Aug 2026 03:30:16 +0100 Subject: [PATCH 08/36] docs(web): add Tailwind and styling conventions Adds apps/web/docs/styling.md. Documents the Tailwind v4 setup: no tailwind.config file, the single @tailwindcss/postcss plugin, and globals.css as the only global stylesheet - with the distinction between its :root variable block and its @theme inline mapping, which is why both bg-card and bg-[var(--card)] compile. Gives guidance on which idiom to use, since the arbitrary-value form is roughly twice as common and is the only one that takes an opacity modifier. Documents the seven colour tokens with their roles and how heavily each is used, the spacing ladder and typography scale the existing screens actually follow, and the Badge tinted-surface recipe that stands in for absent status tokens. Records the ~30 off-token gray/slate and raw-hex occurrences as drift rather than precedent. Covers breakpoints - defaults, mobile-first, only sm/md/lg in use with zero xl or 2xl - and the hand-followed class order, noting that nothing enforces it since there is no prettier-plugin-tailwindcss and no Tailwind ESLint rule. States extraction rules in both directions, anchored on the three- occurrence threshold, variants, accessibility behaviour and non-utility CSS, and notes the absence of any cn/clsx helper. Records that dark mode does not exist: one :root palette that is itself dark, no prefers-color-scheme block, no darkMode config, no theme provider or toggle. The single dark: variant in CopyButton can never activate, so nobody should assume dark-mode support works or write new dark: variants. --- apps/web/docs/styling.md | 389 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 apps/web/docs/styling.md diff --git a/apps/web/docs/styling.md b/apps/web/docs/styling.md new file mode 100644 index 00000000..a29e7c0b --- /dev/null +++ b/apps/web/docs/styling.md @@ -0,0 +1,389 @@ +# Tailwind & styling conventions + +How styling works in `apps/web`: the Tailwind v4 setup, where global styles live, +the design tokens that are actually in use, the breakpoints in play, and the +conventions for writing and extracting classes. + +--- + +## 1. Setup + +| Thing | Value | +| ----------------- | ---------------------------------------------------------------------------------------------- | +| Tailwind version | **v4** β€” `tailwindcss: ^4` and `@tailwindcss/postcss: ^4` in [`package.json`](../package.json) | +| Config file | **None.** There is no `tailwind.config.js`/`.ts`. v4 configures itself from CSS. | +| PostCSS | [`postcss.config.mjs`](../postcss.config.mjs), one plugin: `@tailwindcss/postcss` | +| Global stylesheet | [`src/app/globals.css`](../src/app/globals.css), imported once from `src/app/layout.tsx` | +| Framework | Next.js 16 App Router, React 19 | +| Fonts | `Geist` and `Geist_Mono` via `next/font/google`, wired in `layout.tsx` | + +```js +// postcss.config.mjs β€” the entire build-side setup +const config = { + plugins: { + '@tailwindcss/postcss': {}, + }, +}; + +export default config; +``` + +The v4 detail worth internalising: **there is no JavaScript config to edit.** +Theme extension happens in CSS via `@theme`, and content scanning is automatic β€” +you do not maintain a `content: []` array. If you reach for +`tailwind.config.js`, you are working against the setup. + +### Global styles + +`globals.css` is short and holds everything global: + +```css +@import 'tailwindcss'; + +:root { + --background: #0a0a0f; + --foreground: #f0f0f5; + --accent: #7c5cfc; + --accent-light: #a78bfa; + --muted: #3f3f50; + --card: #13131f; + --border: #1e1e2e; +} + +@theme inline { + --color-background: var(--background); + --color-foreground: var(--foreground); + --color-accent: var(--accent); + --color-accent-light: var(--accent-light); + --color-muted: var(--muted); + --color-card: var(--card); + --color-border: var(--border); + --font-sans: var(--font-geist-sans); + --font-mono: var(--font-geist-mono); +} + +body { + background: var(--background); + color: var(--foreground); + font-family: var(--font-sans), Arial, Helvetica, sans-serif; +} + +html { + scroll-behavior: smooth; +} +``` + +Two layers, and the difference matters: + +- The `:root` block declares the **raw CSS variables**. These are what + `[var(--accent)]` reads. +- The `@theme inline` block maps those into **Tailwind's theme namespace**, which + is what generates utilities like `bg-accent`, `text-foreground`, and + `border-border`. + +Both layers exist, so both idioms work. See +[Β§3](#3-two-ways-to-reach-a-token-and-when-to-use-each). + +Anything not global belongs in a component. The only component-level CSS in the +codebase is styled-jsx in +[`Spinner.tsx`](../src/components/ui/Spinner.tsx) and +[`SkeletonLoader.tsx`](../src/components/ui/SkeletonLoader.tsx), both for +keyframe animations that utilities cannot express β€” including a +`prefers-reduced-motion` guard. That is the bar for reaching past utilities: +**a keyframe or a media query Tailwind has no utility for.** + +--- + +## 2. Design tokens + +Seven colour tokens, all defined in `globals.css`. There is no secondary palette, +no semantic status colours in the theme, and no spacing or typography tokens +beyond Tailwind's built-in scales. + +| Token | Value | Role | In use | +| ---------------- | --------- | --------------------------------------- | ------ | +| `--background` | `#0a0a0f` | Page background β€” near-black, blue-cast | ~23 | +| `--foreground` | `#f0f0f5` | Primary text | ~143 | +| `--accent` | `#7c5cfc` | Primary action / brand violet | ~71 | +| `--accent-light` | `#a78bfa` | Accent text on accent-tinted surfaces | ~10 | +| `--muted` | `#3f3f50` | Muted surface / de-emphasised element | ~15 | +| `--card` | `#13131f` | Raised surface β€” cards, panels | ~42 | +| `--border` | `#1e1e2e` | Hairline borders and dividers | ~71 | + +Counts are utility occurrences across `src/**/*.tsx`, summing both idioms in +[Β§3](#3-two-ways-to-reach-a-token-and-when-to-use-each). They are here to show +which tokens carry the UI, not as a target. + +**Status colours are not tokens.** Success, warning, and danger use Tailwind's +stock palette at fixed opacities, established by +[`Badge.tsx`](../src/components/ui/Badge.tsx): + +```ts +const VARIANT_CLASS: Record = { + default: 'border-[var(--accent)]/30 bg-[var(--accent)]/15 text-[var(--accent-light)]', + success: 'border-green-500/30 bg-green-500/15 text-green-300', + warning: 'border-yellow-500/30 bg-yellow-500/15 text-yellow-200', + danger: 'border-red-500/30 bg-red-500/15 text-red-300', +}; +``` + +That `/30` border, `/15` fill, `-300` text recipe is the house style for a tinted +status surface. Reuse `` rather than re-deriving it. + +### Spacing + +No custom spacing scale β€” Tailwind's default. What the existing screens actually +use, in descending frequency: + +| Purpose | Values in use | +| ----------------- | ------------------------------------------------------------------------------------------ | +| Control padding | `px-4 py-2` (most common), `px-3 py-2`, `px-5 py-2`, `px-6 py-3` | +| Container padding | `p-6` for cards and panels, `p-4` and `p-5` for tighter ones, `p-3`/`p-2` for compact rows | +| Flex/grid gaps | `gap-3` (most common), then `gap-4`, `gap-2`, `gap-1`, `gap-6` | +| Vertical rhythm | `space-y-4`, then `space-y-3` | +| Section padding | `py-32` on landing sections | + +Stay on this ladder β€” `2 / 3 / 4 / 6` covers nearly everything. Reach for an +arbitrary value only when matching a specific visual, and prefer the nearest step +otherwise. + +### Typography + +Two families, both from `next/font/google` and exposed through `@theme` as +`--font-sans` and `--font-mono`. `body` sets sans by default, so `font-sans` is +rarely written explicitly; `font-mono` is for addresses, hashes, and amounts. + +| Scale | Where it is used | +| -------------------- | --------------------------------------------------------------------------- | +| `text-xs` | Metadata, timestamps, badge-adjacent labels β€” very common | +| `text-sm` | **The default for body and UI copy** β€” the most-used size by a wide margin | +| `text-base` | Rare; `text-sm` is the norm, so reaching for `base` is a deliberate step up | +| `text-lg` | Sub-headings and emphasised rows | +| `text-xl`–`text-3xl` | Section and page headings; `text-3xl` is the common heading size | +| `text-4xl`+ | Landing page hero only | + +| Weight | Use | +| --------------- | -------------------------------------------------------- | +| `font-semibold` | The default for anything emphasised β€” most common by far | +| `font-medium` | Softer emphasis, secondary labels | +| `font-bold` | Headings and hero copy | + +Normal body text carries no weight class. If you find yourself writing +`font-normal`, something upstream is over-weighted. + +### Radius + +`rounded-full` (pills, avatars, badges) and `rounded-lg` dominate, with +`rounded-2xl` for cards and panels and `rounded-xl` for medium controls. Prefer +`rounded-2xl` for a new card and `rounded-full` for a new pill or button β€” that +is what surrounding screens do. + +### De-facto tokens that are not in `globals.css` + +Two things recur without being declared, and both are worth knowing before you +copy them: + +- **Opacity-modified tokens.** `bg-[var(--card)]/30`, + `text-[var(--foreground)]/50`, `border-[var(--accent)]/30` are used constantly + to derive a surface or a muted text colour from an existing token. This is + preferred over inventing a new colour: it stays on-palette automatically. +- **Off-token colours.** About 30 occurrences of `gray-*`/`slate-*` and a handful + of raw hex values (`[#13131f]`, `[#0F172A]`, `[#0C3F51]`) exist, mostly in + older components β€” `CopyButton.tsx` is entirely on a light-mode grey palette + that does not belong to this theme. **These are drift, not precedent.** Use the + tokens for new work, and prefer `[#13131f]` β†’ `[var(--card)]` when you are + already editing a file that has one. + +--- + +## 3. Two ways to reach a token, and when to use each + +Because `globals.css` declares both the raw variables and the `@theme` mapping, +both of these compile and both appear in the codebase: + +```tsx +
{/* theme utility */} +
{/* arbitrary value */} +``` + +The arbitrary-value form is roughly twice as common in the current tree +(~193 occurrences vs ~107). Neither is wrong. Pick on this basis: + +- **Need an opacity modifier?** Use `[var(--token)]/NN`. This is why the + arbitrary form dominates β€” `bg-[var(--card)]/30` has no short theme-utility + equivalent here. +- **Plain, full-opacity colour?** Either works; `bg-card` is shorter and reads + better. +- **Editing an existing file?** Match what that file already does. Mixing both + idioms inside one `className` is the thing to avoid. + +--- + +## 4. Responsive breakpoints + +Tailwind's defaults, unmodified: `sm` 640px, `md` 768px, `lg` 1024px, `xl` +1280px, `2xl` 1536px. + +Only `sm`, `md`, and `lg` are used. There are **zero** `xl:` or `2xl:` +occurrences in `src/`, so the widest layout you can currently rely on having been +designed is the `lg` one. `max-w-6xl` is the usual page container cap. + +The codebase is **mobile-first**: base classes describe the narrow layout and +breakpoint prefixes widen it. Follow that β€” never write a desktop base and narrow +it with `max-*` variants. + +Established patterns: + +| Pattern | Examples in use | +| ---------------------- | ------------------------------------------------------------------------- | +| Grid columns | `sm:grid-cols-2`, `md:grid-cols-2/3/5`, `lg:grid-cols-3/4/5` | +| Progressive disclosure | `hidden md:inline`, `hidden sm:flex`, `hidden md:flex`, `hidden lg:block` | +| Container width | `max-w-6xl` for page shells; `max-w-sm`/`md`/`lg` for cards and dialogs | + +Chat and conversation views assume a working narrow layout, so test any change +there at a phone width before a desktop one. + +--- + +## 5. Class ordering + +**Nothing enforces class order.** There is no `prettier-plugin-tailwindcss` in +[`package.json`](../../../package.json) and no Tailwind ESLint rule in +[`eslint.config.mjs`](../eslint.config.mjs) β€” the config is +`eslint-config-next` core-web-vitals plus TypeScript, nothing more. Ordering is a +convention you follow by hand, and no CI job will correct you. + +The order the existing components follow, and the one to write: + +1. **Layout & display** β€” `flex`, `grid`, `inline-flex`, `hidden`, `block` +2. **Box model / sizing** β€” `h-full`, `w-full`, `min-h-40`, `max-w-md` +3. **Flex & grid children** β€” `items-center`, `justify-between`, `flex-col`, `gap-3` +4. **Spacing** β€” `p-6`, `px-4`, `py-2`, `mt-2`, `space-y-4` +5. **Border & radius** β€” `rounded-2xl`, `border`, `border-dashed`, `border-[var(--border)]` +6. **Background** β€” `bg-[var(--card)]/30` +7. **Typography** β€” `text-sm`, `font-semibold`, `leading-relaxed`, `text-[var(--foreground)]` +8. **Effects** β€” `shadow-lg`, `blur-[120px]`, `transition-opacity` +9. **State & responsive variants last** β€” `hover:opacity-90`, `focus:ring-2`, `md:inline`, `lg:grid-cols-3` + +[`EmptyState.tsx`](../src/components/ui/EmptyState.tsx) is a clean reference: + +```tsx +className = + 'flex h-full min-h-40 w-full flex-col items-center justify-center gap-3 rounded-2xl border border-dashed border-[var(--border)] bg-[var(--card)]/30 p-6 text-center'; +``` + +Keeping variants at the end matters most in practice β€” it makes the base +(mobile) layout readable in one glance. + +--- + +## 6. When to extract a component + +There is **no `cn()`/`clsx`/`tailwind-merge` helper** in this codebase. Classes +are composed with template literals and an appended `className` prop: + +```tsx +className={`... base classes ... ${VARIANT_CLASS[variant]} ${className ?? ''}`} +``` + +Follow that pattern rather than introducing a class-merging utility for a single +component; if one is added, it should land as its own change with every call site +converted. + +### Extract when + +- **The same cluster appears three or more times.** Two occurrences are a + coincidence; three is a component. The tinted-pill recipe hit that bar and + became ``. +- **It has variants.** The moment you write a conditional that picks between + class strings, that belongs in a `Record` map inside a + component β€” the shape `Badge` and `Spinner` both use. +- **It carries accessibility or behaviour.** `Spinner` exists as much for its + `role="status"`/`aria-hidden` logic and reduced-motion handling as for its + look. Anything with ARIA, focus management, or an animation should not be + copy-pasted. +- **It needs non-utility CSS.** Keyframes or a media query utilities cannot + express force a component boundary anyway. +- **It is a recognisable UI noun.** Avatar, badge, modal, empty state, skeleton, + spinner β€” if you can name it, it belongs in + [`src/components/ui/`](../src/components/ui/). + +### Repeat the utilities when + +- **It is used once or twice.** Do not build an abstraction for a single caller. +- **The variation is layout, not identity.** A card that is `p-4` here and `p-6` + there with a different grid is two layouts, not one component with two props. +- **Extracting would need more props than the classes it saves.** A wrapper + taking six props to configure spacing is worse than the spacing. +- **It is page-specific composition.** Section layout in a route belongs in that + route. + +### Where components live + +| Directory | Contents | +| --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------- | +| [`src/components/ui/`](../src/components/ui/) | Generic primitives β€” `Avatar`, `Badge`, `CopyButton`, `EmptyState`, `Modal`, `ProposalCard`, `SkeletonLoader`, `Spinner` | +| `src/components//` | Feature-scoped components β€” `auth/`, `chat/`, `conversations/`, `landing/`, `messaging/`, `search/`, `treasury/`, `wallet/` | + +A component reused across two features moves to `ui/`; one that knows about a +domain type stays in its feature directory. + +--- + +## 7. Dark mode + +**There is no dark-mode support, and there is nothing to toggle.** The app is +**dark-only** by design: + +- The single `:root` palette in `globals.css` _is_ the dark theme β€” `#0a0a0f` + background, `#f0f0f5` foreground. There is no light palette anywhere. +- There is **no `@media (prefers-color-scheme: ...)` block** in the codebase. +- There is **no `darkMode` configuration** β€” v4 would configure it in CSS via a + custom variant, and none is declared. +- There is **no theme provider, no `next-themes`, and no theme toggle.** + `layout.tsx` wraps the tree in `WalletProvider`, `AuthProvider`, and + `ToastProvider` only. +- `` carries no `class="dark"` or `data-theme` attribute. + +Exactly **one** `dark:` variant exists in the whole of `src/` β€” +`text-emerald-600 dark:text-emerald-500` in +[`CopyButton.tsx`](../src/components/ui/CopyButton.tsx). Because no dark-mode +strategy is configured, **that variant never activates.** It is a leftover from a +component authored against light-mode assumptions, which is also why that file +uses `slate-50`, `gray-100`, and `#0C3F51` β€” colours that belong to no theme +here. + +Consequences for new work: + +- **Do not write `dark:` variants.** They are dead code. Style for the dark + palette directly. +- **Do not assume a light background.** A component built with light-mode + defaults will be illegible; `CopyButton` is what that looks like in practice. +- **Adding light mode is a project, not a patch.** It would need a light palette, + a variant strategy declared in `globals.css`, a theme provider with persistence, + and an audit of every hardcoded colour listed in + [Β§2](#de-facto-tokens-that-are-not-in-globalscss). Nothing today is written to + support it. + +--- + +## 8. Checklist for a new component + +- [ ] Colours come from the seven tokens β€” theme utility or `[var(--token)]`, with `/NN` for tints. No new raw hex, no `gray-*`/`slate-*`. +- [ ] Spacing is on the `2 / 3 / 4 / 6` ladder; padding matches the `px-4 py-2` / `p-6` conventions. +- [ ] Type is `text-sm` unless there is a reason; emphasis is `font-semibold`. +- [ ] Radius matches neighbours β€” `rounded-2xl` for cards, `rounded-full` for pills. +- [ ] Written mobile-first, with `sm:`/`md:`/`lg:` widening it. No `xl:`/`2xl:` unless you are also designing that width. +- [ ] Classes ordered per [Β§5](#5-class-ordering); variants last. +- [ ] No `dark:` variants. +- [ ] Third occurrence of the same cluster? Extract it to `src/components/ui/`. +- [ ] Non-utility CSS is styled-jsx inside the component, with a `prefers-reduced-motion` guard on any animation. + +--- + +## 9. Related documents + +- [Wallet & treasury UI](./concepts-wallet-treasury-ui.md) β€” the screens most of these conventions were established on +- [Auth & device lifecycle](./concepts-auth-device-lifecycle.md) +- [Message pipeline](./concepts-message-pipeline.md) β€” the chat views to test at narrow widths +- [`src/app/globals.css`](../src/app/globals.css) β€” the source of truth for every token above From 3478763c7d0dc412635a2fd5589c5202bac59a60 Mon Sep 17 00:00:00 2001 From: G-ELM Date: Sun, 30 Aug 2026 23:47:55 +0100 Subject: [PATCH 09/36] docs(contracts): add contract testing guide Documents the standard Soroban test scaffolding, the three levels of auth mocking and the risk of blanket mock_all_auths() hiding a missing require_auth(), how to test expiry via the virtual ledger clock, and the proposals -> group_treasury cross-contract test setup. --- contracts/docs/testing.md | 312 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 contracts/docs/testing.md diff --git a/contracts/docs/testing.md b/contracts/docs/testing.md new file mode 100644 index 00000000..21bb4438 --- /dev/null +++ b/contracts/docs/testing.md @@ -0,0 +1,312 @@ +# Contract testing guide + +How the Soroban test environment is used across the three contracts in this workspace +(`token_transfer`, `group_treasury`, `proposals`): the standard scaffolding, how +authorization is tested (and how to avoid hiding a missing `require_auth`), how +time-dependent behaviour like proposal expiry is exercised, and how cross-contract tests +wire `proposals` into `group_treasury`. + +All examples below are taken from the real test suites in this workspace β€” see +`contracts/contracts/*/src/test.rs`. + +--- + +## Standard test scaffolding + +Every contract test module is gated with `#![cfg(test)]` and starts from a fresh +`Env::default()`. There is no shared/global environment between tests β€” each `#[test]` +function builds its own. + +```rust +#![cfg(test)] + +use super::*; +use soroban_sdk::testutils::Address as _; +use soroban_sdk::testutils::Ledger; +use soroban_sdk::Env; + +#[test] +fn create_then_vote_then_pass_then_execute_happy_path() { + let env = Env::default(); + let (client, _admin, alice, bob, carol, ..) = setup(&env); + + let id = create_proposal_in(&env, &client, &alice, 1_000, /* ... */); + + client.vote(&alice, &id, &true); + client.vote(&bob, &id, &true); + client.vote(&carol, &id, &false); + + advance_time(&env, 1_001); + let status = client.finalize_proposal(&id); + assert_eq!(status, ProposalStatus::Passed); +} +``` + +### Address generation + +Test addresses are never hardcoded strings β€” they are generated per-test with +`Address::generate(&env)`, which requires `soroban_sdk::testutils::Address` to be in scope +(imported as `Address as _` since only the trait method is used, not the type name): + +```rust +use soroban_sdk::testutils::Address as _; + +let alice = Address::generate(&env); +let bob = Address::generate(&env); +``` + +Each call produces a fresh, unique address scoped to that `env`. Addresses are never reused +across tests. + +### The `setup()` helper + +Each contract's test module defines a `setup(&env)` helper that registers the contract(s), +generates the standard cast of addresses (an admin plus a small number of named actors β€” +`alice`, `bob`, `carol` are the convention here), and returns a tuple of the client plus +every address the test body will need. Keeping this in one function means every test starts +from the same known-good state and individual tests stay short. + +Where a contract needs a token to move, `setup()` also registers a minimal `mock_token` +module scoped to the test file (`#[contract] struct MockToken` with `mint`/`transfer`/ +`balance`) rather than depending on a real token contract. This keeps tests fast and +self-contained, and lets tests that only care about authorization use a token whose +`transfer` still calls real `require_auth()`. + +--- + +## Testing authorization + +Soroban's test host does not simulate real signatures. Instead, `Env` exposes mocking +utilities that make `require_auth()` calls succeed (or fail) without a signature ever being +produced. Three levels are used across this workspace, in increasing order of precision: + +### 1. `mock_all_auths()` β€” blanket mocking + +```rust +let env = Env::default(); +env.mock_all_auths(); +``` + +This makes **every** `require_auth()` / `require_auth_for_args()` call in the transaction +succeed, regardless of which address it was called on. It is the fastest way to get a +happy-path test running, and it is what most tests in this workspace use. + +**The risk:** `mock_all_auths()` proves nothing about *which* address is required to +authorize a call β€” only that *some* auth check, if present, would pass. A contract method +that is missing a `require_auth()` call entirely will pass a `mock_all_auths()`-based test +just as easily as a correct one. Blanket mocking is safe for tests whose purpose is +something other than authorization (e.g. asserting vote tallying, or balance arithmetic), +but it must never be the *only* test covering a security-sensitive entry point. + +### 2. `mock_all_auths_allowing_non_root_auth()` β€” cross-contract calls + +`proposals::execute_withdraw` calls `group_treasury::withdraw`, which itself calls +`admin.require_auth()` as a **nested** (non-root) invocation β€” the admin address never +appears in the root call's argument list. The default `mock_all_auths()` only mocks +auth for the root invocation, so cross-contract setups that need a nested `require_auth` +to succeed use the non-root variant instead: + +```rust +// execute_withdraw calls treasury.withdraw() as a nested (non-root) call, which +// itself calls admin.require_auth() β€” an address not present in the root +// invocation's argument list, so the non-root variant is required. +env.mock_all_auths_allowing_non_root_auth(); +``` + +### 3. `env.auths()` and `mock_auths()` β€” asserting the real requirement + +To actually prove that a specific address's authorization is required β€” not just that +*an* auth check exists β€” combine one of two techniques: + +**Assert which address was required**, after a successful call under blanket mocking: + +```rust +env.mock_all_auths(); +client.transfer(&sender, &receiver, &100, &memo); + +let auths = env.auths(); +assert!(auths.iter().any(|(addr, _)| *addr == sender)); +``` + +**Mock auth for one address only**, and confirm the call panics when the address that +should be required did not authorize: + +```rust +#[test] +#[should_panic] +fn test_upgrade_non_admin_panics() { + let env = Env::default(); + let admin = Address::generate(&env); + let contract_id = setup_with_admin(&env, &admin); + let client = TokenTransferContractClient::new(&env, &contract_id); + + // Only mock auth for a non-admin address. The contract must panic when + // require_auth() is invoked on `admin`, because `admin` never signed. + let intruder = Address::generate(&env); + env.mock_auths(&[soroban_sdk::testutils::MockAuth { + address: &intruder, + invoke: &soroban_sdk::testutils::MockAuthInvoke { + contract: &contract_id, + fn_name: "upgrade", + args: soroban_sdk::vec![&env], + sub_invokes: &[], + }, + }]); + + client.upgrade(&wasm_hash); // panics: admin never authorized +} +``` + +**Test with no mocking at all**, when the point of the test is that the call must fail +without authorization: + +```rust +#[test] +#[should_panic] +fn test_vote_without_auth_panics() { + let env = Env::default(); + let admin = Address::generate(&env); + let member = Address::generate(&env); + // ... register + initialize the contract ... + + env.mock_all_auths(); + client.add_member(&member); + // ... + env.set_auths(&[]); // clear mocked auths β€” the vote must now fail + + client.approve_withdraw(&member, &0); // panics: no auth present +} +``` + +`env.set_auths(&[])` is useful when setup steps genuinely need mocked auth (e.g. +initializing a contract as admin) but the assertion under test needs a clean slate with +none mocked. + +**Rule of thumb:** every entry point that calls `require_auth()` should have at least one +test that exercises the real requirement β€” via `env.auths()`, a scoped `mock_auths()`, or +an unmocked `should_panic` test β€” in addition to any `mock_all_auths()`-based happy-path +tests. A codebase with only blanket-mocked tests can silently lose a `require_auth()` call +in a refactor and no test will catch it. + +--- + +## Testing time-dependent behaviour (expiry) + +Soroban's test `Env` uses a virtual ledger clock (`env.ledger()`), which starts at a fixed +timestamp and only advances when a test explicitly moves it forward. This makes +expiry-style logic deterministic to test: no real waiting, no flaky timers. + +The convention in this workspace is a small `advance_time` helper: + +```rust +use soroban_sdk::testutils::Ledger; + +fn advance_time(env: &Env, seconds: u64) { + env.ledger() + .set_timestamp(env.ledger().timestamp() + seconds); +} +``` + +Usage β€” a proposal created with a 500-second voting window cannot be finalized before +expiry, and must be finalizable once the clock has passed it: + +```rust +#[test] +#[should_panic(expected = "cannot finalize before expiry")] +fn finalize_before_expiry_panics() { + let env = Env::default(); + let (client, .., alice, .., m, token_id) = setup(&env); + let id = create_proposal_in(&env, &client, &alice, 1_000, &m, &token_id, &alice, 1); + + client.finalize_proposal(&id); // no advance_time β€” still active +} + +#[test] +fn finalize_expired_success() { + let env = Env::default(); + let (client, .., alice, .., m, token_id) = setup(&env); + let id = create_proposal_in(&env, &client, &alice, 500, &m, &token_id, &alice, 1); + + advance_time(&env, 501); // past the 500s window + client.finalize_expired_proposal(&id); + + assert_eq!(client.get_proposal(&id).status, ProposalStatus::Expired); +} +``` + +Both the "too early" boundary (`should_panic`, no time advance) and the "just past expiry" +boundary (advance exactly past the window) should be covered for any time-gated state +transition β€” off-by-one errors in expiry math are easy to introduce and easy to catch this +way. + +--- + +## Cross-contract test setup: `proposals` calling into `group_treasury` + +`proposals::execute_withdraw` invokes `group_treasury::withdraw` as a real cross-contract +call inside the test host β€” not a mock of the treasury contract. The `proposals` test +`setup()` registers both contracts in the same `Env` and wires the treasury's address into +the proposal so the nested call resolves correctly: + +```rust +fn setup(env: &Env) -> ( /* ... */ ) { + // Nested require_auth() inside execute_withdraw needs the non-root variant. + env.mock_all_auths_allowing_non_root_auth(); + + let proposals_id = env.register(ProposalsContract, ()); + let proposals = ProposalsContractClient::new(env, &proposals_id); + proposals.initialize(&proposals_admin); + + let token_id = env.register(mock_token::MockToken, ()); + let token = MockTokenClient::new(env, &token_id); + token.mint(&treasury_member, &1_000_000); + + let treasury_addr = env.register(group_treasury::GroupTreasuryContract, ()); + let treasury = group_treasury::GroupTreasuryContractClient::new(env, &treasury_addr); + treasury.initialize(&treasury_admin, &token_id, &1); + treasury.add_member(&treasury_member); + treasury.deposit(&treasury_member, &token_id, &500); + + (proposals, proposals_admin, alice, bob, carol, treasury, treasury_admin, treasury_member, token_id) +} +``` + +Points worth keeping in mind when writing a test like this: + +- The `proposals` crate depends on `group_treasury` as a regular Rust dependency (see + `contracts/contracts/proposals/Cargo.toml` and `treasury_interface_client.rs`), so the + treasury client type is available directly β€” no WASM re-import step is needed inside + tests. +- Both contracts are registered against the **same** `env`, which is what makes the nested + invocation and its nested `require_auth()` resolvable at all. + `mock_all_auths_allowing_non_root_auth()` (not plain `mock_all_auths()`) is required for + exactly this reason β€” see [Testing authorization](#testing-authorization) above. + Registering the deposit is done through the treasury client's own `deposit()` method + (which itself calls the mock token's `transfer`) so the treasury has a real balance for + `execute_withdraw` to draw down, rather than writing to treasury storage directly. +- Assertions after `execute_withdraw` read state back through the *treasury's* client + (`treasury.balance(&token_id)`), confirming the effect actually crossed the contract + boundary rather than only checking the proposal's own status flipped to `Executed`. + +--- + +## The `test_snapshots/` directory + +Running `cargo test` against a Soroban contract writes ledger/state snapshot files under +each contract's `test_snapshots/` directory (e.g. +`contracts/contracts/proposals/test_snapshots/`). These capture the test host's storage +state at the end of each test run and are how Soroban's test harness detects unexpected +storage-footprint changes between runs. + +- **Generated, not hand-written.** Never edit files under `test_snapshots/` by hand β€” they + are regenerated automatically the next time the corresponding test runs. +- **Committed.** Despite being generated, these files are checked into the repository. They + are part of what makes a contract's storage footprint reviewable in a diff β€” a PR that + unexpectedly grows a snapshot is a signal worth looking at during review. +- **Prettier-ignored.** The root `.prettierignore` excludes `contracts/**/test_snapshots/` + under the "generated artifacts" section, alongside `apps/backend/drizzle/meta/` and + `**/*.d.ts`. This is intentional: reformatting a generated, machine-written file produces + noisy diffs and gets overwritten on the next test run regardless. + +If a snapshot diff shows up in a PR that didn't intentionally change contract storage +layout, treat it as a signal to re-check the change rather than committing it as noise. From cd47b5b5108742d508baa34c462e90bcae0714f3 Mon Sep 17 00:00:00 2001 From: G-ELM Date: Sun, 30 Aug 2026 23:48:44 +0100 Subject: [PATCH 10/36] docs(web): add service worker and offline behaviour guide Documents sw.js registration/update lifecycle via skipWaiting + clients.claim, the content-free push handler, click-to-route focusing an existing tab instead of opening a duplicate, and what does and does not work offline today (no precaching, no fetch interception, no background sync). --- apps/web/docs/concepts-service-worker.md | 202 +++++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 apps/web/docs/concepts-service-worker.md diff --git a/apps/web/docs/concepts-service-worker.md b/apps/web/docs/concepts-service-worker.md new file mode 100644 index 00000000..10924ef5 --- /dev/null +++ b/apps/web/docs/concepts-service-worker.md @@ -0,0 +1,202 @@ +# Service worker and offline behaviour + +This document covers `public/sw.js`: how it is registered and updated, what it does with +push events, how it routes notification clicks back into the app, and β€” plainly β€” what +does and does not work when the device is offline. + +For the push *subscription* flow (permission prompt, VAPID keys, backend registration), +see [Push notification subscription flow](concepts-push-subscription.md). This document +focuses on the service worker itself. + +--- + +## Registration lifecycle + +The service worker is registered by `hooks/usePushSubscription.ts`, not by Next.js or a +PWA plugin β€” there is no automatic registration on every page load. Registration happens +on mount of whatever component calls `usePushSubscription`: + +```ts +navigator.serviceWorker.register('/sw.js', { scope: '/' }); +``` + +Scope `/` means the worker controls every page on the origin, not just the page that +registered it. + +### Install and activate + +```js +self.addEventListener('install', () => { + self.skipWaiting(); +}); + +self.addEventListener('activate', (event) => { + event.waitUntil(self.clients.claim()); +}); +``` + +- **`install` β†’ `skipWaiting()`** β€” By default, a newly installed service worker sits in a + "waiting" state until every open tab controlled by the old worker is closed. Calling + `skipWaiting()` during install skips that wait: the new worker activates as soon as it + finishes installing, even while old tabs are still open. +- **`activate` β†’ `clients.claim()`** β€” By default, an activated worker only controls pages + loaded *after* activation. `clients.claim()` takes control of every already-open, + uncontrolled page on the origin immediately. + +### The update path β€” how a stale worker is replaced + +Together, `skipWaiting()` + `clients.claim()` mean this service worker updates itself +eagerly rather than waiting for a natural refresh cycle: + +1. The browser fetches `/sw.js` on navigation/periodically and byte-compares it against the + currently installed worker. +2. If it differs, the browser installs the new version in the background. `install` fires, + `skipWaiting()` runs immediately, and the new worker moves straight to "activating" + instead of "waiting". +3. `activate` fires, `clients.claim()` runs, and the new worker takes over **every open + tab** on the origin β€” including ones that were loaded under the old worker and never + reloaded. + +The practical effect: there is no user-visible "a new version is available, refresh to +update" step for the service worker itself. The trade-off is that a page whose in-memory +JS was loaded under the old worker can, in principle, keep running while a different tab +is now served by the new worker β€” the two do not roll forward in lockstep mid-session. +Since this worker carries no versioned caching logic (see [Offline behaviour](#offline-behaviour-today) +below), this has not been a practical problem: there is no cached asset list that could +go stale between the two workers. + +--- + +## The push handler + +```js +self.addEventListener('push', (event) => { + let data = {}; + try { + data = event.data ? event.data.json() : {}; + } catch { + // malformed payload β€” show a generic notification + } + + const conversationId = data.conversationId ?? null; + + event.waitUntil( + self.registration.showNotification('Clicked', { + body: 'You have a new message', + icon: '/icons/icon-192.png', + badge: '/icons/badge-96.png', + tag: conversationId ? `conv-${conversationId}` : 'new-message', + renotify: true, + data: { conversationId }, + }), + ); +}); +``` + +### Content-free by design + +The notification title is always the literal string `Clicked`, and the body is always the +literal string `You have a new message`. The only thing extracted from the push payload is +an optional `conversationId`, used purely for click-routing (see below) β€” it is never +rendered into the notification UI. A malformed or unparsable payload is caught and still +produces a generic notification rather than failing silently. + +This is the client-side half of a deliberate privacy design: the backend never puts message +content in a push payload in the first place. See +[`apps/backend/docs/api-push.md`](../../backend/docs/api-push.md) for how the backend +constructs push payloads and why content is excluded server-side. The two halves only work +together β€” a service worker that refuses to render content is not a substitute for a +backend that never sends it, and vice versa. + +`tag: conv-` (or `new-message` when there is no conversation ID) lets a +second push for the same conversation replace the existing OS notification instead of +stacking a duplicate; `renotify: true` re-alerts the user when that replacement happens. + +--- + +## Notification click routing + +```js +self.addEventListener('notificationclick', (event) => { + event.notification.close(); + + const conversationId = event.notification.data?.conversationId ?? null; + const target = conversationId + ? `/app/conversations/${conversationId}` + : '/app/messages'; + + event.waitUntil( + self.clients + .matchAll({ type: 'window', includeUncontrolled: true }) + .then((windowClients) => { + for (const client of windowClients) { + if (new URL(client.url).origin === self.location.origin) { + client.postMessage({ type: 'sw:sync', conversationId }); + client.focus(); + return; + } + } + return self.clients.openWindow(target); + }), + ); +}); +``` + +1. The clicked notification is closed immediately. +2. The worker looks for any open window on the app's origin via + `clients.matchAll({ type: 'window', includeUncontrolled: true })`. +3. **If one is found**, the worker does **not** navigate it directly β€” `postMessage` and + `navigate()` on a `WindowClient` both require the client to be controlled, and an + `includeUncontrolled` match may not be. Instead it posts `{ type: 'sw:sync', + conversationId }` to the page and calls `client.focus()` to bring the existing tab to + the front. The app's own layout listens for this message client-side: + + ```ts + // apps/web/src/app/app/layout.tsx + navigator.serviceWorker.addEventListener('message', (event) => { + if (event.data?.type !== 'sw:sync') return; + const { conversationId } = event.data; + router.push(conversationId ? `/app/conversations/${conversationId}` : '/app/messages'); + }); + ``` + + The Next.js router then performs the actual client-side navigation. This is the + "focus an existing tab rather than opening a duplicate" behaviour β€” a user with the app + already open in one tab never ends up with a second tab per notification. +4. **If no window is found**, the worker calls `self.clients.openWindow(target)`, opening a + fresh tab directly at `/app/conversations/` (or `/app/messages` with no ID) β€” there + is no existing page to `postMessage` into, so the destination URL does the routing work + instead. + +--- + +## Offline behaviour today + +This service worker registers no `fetch` handler and maintains no `caches` entries. It +exists solely for push notifications (`push`, `notificationclick`) and its own lifecycle +(`install`, `activate`). Concretely: + +**Works offline:** + +- Previously loaded pages that are still in the browser's own HTTP cache or Next.js's + client-side router cache may continue to render from memory until a hard navigation is + needed. +- Already-decrypted messages held in IndexedDB (see + [IndexedDB schemas](contracts-indexeddb-schemas.md)) remain readable if a view reads from + local storage rather than the network. +- Push notifications continue to be received and shown while offline is not the relevant + condition here β€” push delivery is the browser vendor's push service waking the service + worker, independent of whether the tab is open. + +**Does not work offline:** + +- No page, route, or static asset is precached β€” a hard reload or first navigation to an + unvisited route while offline fails exactly as it would with no service worker at all. +- No API requests are intercepted or served from a cache; `fetch` calls to the REST API and + the WebSocket connection simply fail offline (see + [Frontend error handling and user feedback](concepts-error-handling.md) for how those + failures surface to the user). +- There is no background sync / outbox queue β€” a message typed while offline is not queued + by the service worker for later delivery. +- Nothing is precached ahead of going offline, so there is no "app shell" guarantee β€” this + is not a full offline-first PWA today. From 75397a63cd21876529f4f7ebd581101cea38d9b8 Mon Sep 17 00:00:00 2001 From: G-ELM Date: Sun, 30 Aug 2026 23:51:54 +0100 Subject: [PATCH 11/36] docs(web): add frontend error handling and user feedback guide Documents the toast API vs inline error state, maps backend error responses (REST schema error format, per-endpoint docs) onto user-facing messages, states the hard rule that decryption failures render as UnavailableMessagePlaceholder rather than a generic crash, and covers current send-failure/offline behaviour including known gaps (silent socket errors, no send retry or offline queue). --- apps/web/docs/concepts-error-handling.md | 229 +++++++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 apps/web/docs/concepts-error-handling.md diff --git a/apps/web/docs/concepts-error-handling.md b/apps/web/docs/concepts-error-handling.md new file mode 100644 index 00000000..26ef2833 --- /dev/null +++ b/apps/web/docs/concepts-error-handling.md @@ -0,0 +1,229 @@ +# Frontend error handling and user feedback + +How failures reach the user in `apps/web`: REST API errors, WebSocket/socket errors, +wallet rejections, and decryption failures. Covers the toast system +(`lib/useToast.ts`), when a route uses inline error state instead, the socket `error` +event, and what happens to a message when sending it ultimately fails. + +There is **no global error interceptor** in this app β€” no fetch middleware, no error +boundary that turns every failure into the same UI. Each call site decides how to surface +its own failure. This document describes the two patterns in use, when to reach for each, +and the one case (decryption) where the presentation is a hard rule rather than a +per-call-site choice. + +--- + +## The toast API (`lib/useToast.ts`) + +```ts +const { notify, success, error, info, dismiss } = useToast(); + +success('Withdrawal proposal submitted successfully'); +error(body.error ?? 'Failed to submit proposal'); +info('Reconnecting…'); +``` + +`useToast()` reads a `ToastContext` provided by `` (mounted once in +`app/layout.tsx`, above the whole app). Calling it outside the provider throws +immediately β€” this is intentional, so a missing provider fails loudly in development +rather than showing nothing in production. + +Toasts: + +- Render bottom-right, stack, and auto-dismiss after 4 seconds (`ToastProvider.tsx`), or + can be dismissed early by the user or by calling `dismiss(id)`. +- Have three variants β€” `success`, `error`, `info` β€” each with its own colour and icon. +- Are **transient and global**. They are the right choice for the result of a discrete + action the user just took (submit a proposal, save a setting, a network error on submit) + where the relevant context (a modal, a form) may already be gone by the time the result + is known. + +### When to use a toast vs. inline error state + +| Use a toast when… | Use inline error state when… | +| --- | --- | +| The action is a one-off submission (form, modal, button click) and success/failure is transient feedback. | The error is about a specific field or a specific piece of persistent UI (a route, a panel) that stays on screen. | +| The user can immediately retry the same action from where they are. | The user needs to see *why*, next to *what*, for more than 4 seconds β€” e.g. a validation message under an input. | +| Example: `ProposeWithdrawalModal` toasts `'Network error β€” please try again'` on a failed `POST /treasury/propose`, while validation errors (bad recipient address) are set as component state and rendered under the field, not toasted. | Example: a route that fails to load its primary data (a conversation, a proposal list) renders an inline empty/error state in place of the content, since a toast would disappear while the broken screen remains. | + +Both can apply to the same failure at different layers: a form's *field-level* validation +error is inline, while the same form's *submit* failure (a rejected `fetch`) is a toast. +See `components/treasury/ProposeWithdrawalModal.tsx` for both patterns side by side in one +component. + +Toast adoption is not yet universal β€” some call sites currently only `console.error` a +failure with no user-visible feedback at all (see [Socket errors](#socket-errors) and +[Send failures](#send-failures-and-what-happens-to-the-message) below). Treat "toast or +inline state, chosen deliberately" as the standard for new code, not the current state of +every existing call site. + +--- + +## Mapping backend errors to user-facing messages + +There is no global response interceptor, so every call site is responsible for checking +`response.ok` and reading the backend's error body itself: + +```ts +const res = await apiFetch('/treasury/propose', { method: 'POST', body: /* ... */ }); + +if (!res.ok) { + const body = (await res.json().catch(() => ({}))) as { error?: string }; + toastError(body.error ?? 'Failed to submit proposal'); + return; +} +``` + +The backend's validation failures follow one consistent shape β€” +`{ error: string, issues?: [{ field, message }] }` at `400 Bad Request` β€” documented in +[REST schemas β€” Error Response Format](../../backend/docs/contracts-rest-schemas.md#error-response-format). +When a route surfaces a validation failure to the user, prefer showing `issues[].message` +next to the relevant field (inline) over the top-level `error` string in a toast, since the +top-level string is often generic (`"Validation failed"`) while `issues` names the actual +problem. + +For endpoint-specific error conditions (expired sessions, ownership checks, rate limits), +consult the relevant backend API doc β€” e.g. +[Auth API](../../backend/docs/api-auth.md), +[Devices and prekeys API](../../backend/docs/api-devices.md) β€” for the exact status codes +and error strings a given route can return, so the frontend message matches what actually +happened rather than a generic fallback. + +### 401 / expired session + +`lib/api.ts` does not intercept 401s globally (see +[REST client β€” 401 / expired-token handling](api-rest-client.md)). A 401 on any given call +surfaces through that call's own `!res.ok` branch like any other error; there is currently +no app-wide "your session expired, please sign in again" redirect triggered centrally β€” +each screen's own error handling is what the user sees. + +### Wallet rejections + +Wallet interactions (`connect()`, transaction signing) reject their promise when the user +declines in the wallet UI. The current pattern: + +```ts +try { + await connect(); +} catch (err) { + console.error('Wallet connection failed:', err); +} finally { + setIsConnecting(false); +} +``` + +A rejected wallet connection currently resets loading state but does not show the user a +toast or inline message distinguishing "you declined" from "the wallet extension isn't +installed" from "the connection timed out" β€” all three collapse to a silent no-op from the +user's point of view. This is a gap worth closing with a toast (`toastError('Connection +request was declined')` or similar) rather than a pattern to copy into new wallet-driven +flows. + +--- + +## Socket errors + +`lib/socket.ts` registers a generic handler on the shared socket: + +```ts +socket.on('error', (error) => console.error('Socket error:', error)); +``` + +This is diagnostic only β€” a socket-level error (auth rejected on connect, a malformed +server event) is logged to the console and otherwise invisible to the user. Reconnection +itself is handled separately by `socket.io-client`'s built-in `reconnection: true` option, +so transient network drops recover without user action; what is *not* currently surfaced is +a persistent failure (e.g. repeated reconnection failures) as a toast or a "reconnecting…" +indicator. If you are building UI that depends on socket delivery (typing indicators, +presence, live message arrival), do not assume the user will see any signal if the socket +is silently failing β€” treat this as a known gap rather than an existing pattern to rely on. + +--- + +## Decryption failures β€” never a generic crash + +**Rule:** a message that fails to decrypt, fails verification, or arrives before the local +session exists must never be rendered as a thrown error, a broken row, or a generic "something +went wrong" state. It always renders as `UnavailableMessagePlaceholder`, a calm, specific, +in-flow placeholder β€” never a toast, never an error boundary. + +```ts +export type UnavailableReason = 'pre-link' | 'undecryptable' | 'verification-failed'; +``` + +```tsx +// components/messaging/UnavailableMessagePlaceholder.tsx +const REASON_COPY: Record = { + 'pre-link': 'Waiting for secure session β€” message from before this device was linked.', + undecryptable: 'Unable to decrypt this message.', + 'verification-failed': 'Message could not be verified.', +}; + +
+ πŸ”’ {REASON_COPY[reason]} +
+``` + +Why this is a hard rule rather than a style preference: + +- **It is expected, not exceptional.** `pre-link` in particular happens routinely β€” any + message sent before the current device completed session setup is legitimately + undecryptable on this device by design, not a bug. Presenting it as an alarming failure + would train users to distrust normal E2EE behaviour. +- **It is isolated.** Crashing that row (or the whole thread) on one bad message would + take down an otherwise healthy conversation. +- **It is inline, in the message's position in the thread** (`InboundMessageRow` renders + it in place of the decrypted bubble), so the conversation's shape and ordering are + preserved β€” the user sees *that* a message exists and roughly when, just not its content. + +When adding a new code path that can fail to decrypt or verify a message, route it through +`UnavailableReason` / `UnavailableMessagePlaceholder` rather than letting the failure +propagate as a thrown exception into the render tree. + +--- + +## Retry and offline behaviour + +### Send failures and what happens to the message + +Sending an encrypted text message (`handleSendEncrypted` in +`app/app/conversations/[id]/page.tsx`) is not currently optimistic in the UI-state sense β€” +the composer's text is only cleared (`setSendText('')`) **after** the encrypt-and-send +call succeeds: + +```ts +async function handleSendEncrypted() { + if (!sendText.trim() || !socket || !token) return; + setSending(true); + try { + await sendEncryptedMessage({ /* ... */ }); + socket.emit('send_message', { /* ... */ }); + setSendText(''); + } catch (err) { + console.error('Failed to send encrypted message:', err); + } finally { + setSending(false); + } +} +``` + +Practically, this means: + +- **On failure**, the typed text is preserved in the composer (never cleared), so nothing + the user wrote is lost β€” but there is currently no toast or inline indicator telling the + user the send failed at all; the only signal today is a console error and the message + simply not appearing in the thread. The user's next reasonable action is pressing send + again, which works because the text is still there, but nothing prompts them to. +- **There is no automatic retry.** A failed send is not queued or retried by the app; it is + a dead end until the user notices and resends manually. +- **There is no offline queue.** The service worker does not intercept `fetch` or queue + failed sends (see + [Service worker β€” offline behaviour today](concepts-service-worker.md#offline-behaviour-today)). + A message typed while offline fails the same way any other network error does, with the + same silent-console-only signal β€” it is not saved for automatic delivery once + connectivity returns. + +If you are extending the composer, wiring `toastError('Message failed to send')` into that +`catch` block is a small, low-risk improvement consistent with the rest of this document's +[toast guidance](#the-toast-api-libusetoastts) β€” the current silent failure is a gap, not a +pattern to preserve. From 25483a8a7d6029baa48df698e8da4108c3c83919 Mon Sep 17 00:00:00 2001 From: G-ELM Date: Mon, 31 Aug 2026 00:02:05 +0100 Subject: [PATCH 12/36] docs(web): add frontend accessibility guide States the WCAG 2.1 AA target and that conformance is currently checked manually (no automated a11y tooling in the repo yet). Documents keyboard navigation for the conversation list and composer, contrasts Modal's full focus-trap/restore implementation against the safety-number panel's current gap, explains the aria-live approach for incoming messages without stealing focus, flags the low-opacity text contrast risk, and gives a pre-merge checklist. Also registers all four new docs (contract testing, service worker, error handling, accessibility) in the documentation index. --- apps/web/docs/accessibility.md | 231 +++++++++++++++++++++++++++++++++ docs/README.md | 4 + 2 files changed, 235 insertions(+) create mode 100644 apps/web/docs/accessibility.md diff --git a/apps/web/docs/accessibility.md b/apps/web/docs/accessibility.md new file mode 100644 index 00000000..c5e95fd4 --- /dev/null +++ b/apps/web/docs/accessibility.md @@ -0,0 +1,231 @@ +# Frontend accessibility guide + +The accessibility standard `apps/web` targets, and the patterns required to meet it: +keyboard navigation through the conversation list and message composer, focus management +in modals and the safety-number panel, screen-reader announcement of incoming messages, +and colour contrast. + +This document describes both the pattern to follow and, honestly, where the current code +already meets it and where it does not yet. Where a gap is called out, treat it as +something to fix when you touch that area, not as the intended design. + +--- + +## Target standard + +**WCAG 2.1 Level AA.** This is the conventional baseline for a web application handling +real user-to-user communication, and is the standard assumed throughout this document β€” +there is no stricter internal bar and no formal deviation from it. + +**How it is checked today: manually, not automatically.** There is no `eslint-plugin-jsx-a11y`, +no automated axe/Lighthouse run in CI, and no accessibility test suite in this repo as of +this writing. Conformance currently depends entirely on the patterns below being followed +by hand and reviewed in PRs. If you are adding accessibility tooling, wiring an automated +check (axe-core in CI, or `eslint-plugin-jsx-a11y` at minimum) closes a real gap rather +than adding redundant coverage β€” until then, the [pre-merge checklist](#pre-merge-checklist-for-a-new-interactive-component) +below is the actual enforcement mechanism. + +--- + +## Keyboard navigation + +### Conversation list + +`components/conversations/ConversationListSidebar.tsx` renders each conversation as a +plain ``: + +```tsx + + {/* avatar, title, preview */} + +``` + +There is no custom keyboard handling β€” no roving `tabindex`, no arrow-key list navigation. +Keyboard support comes entirely from using a real `` (an anchor): it is naturally +focusable, appears in Tab order, and activates on Enter, all for free. **This is the +required pattern for list items in this app** β€” a clickable row must be a real +link/button, never a `
`, precisely so keyboard support does not have to be +hand-built. Arrow-key roving-tabindex navigation (a full ARIA `listbox`/`menu` pattern) is +not implemented and is not required β€” sequential Tab order through the list is the +supported navigation model. + +### Message composer + +The composer (`app/app/conversations/[id]/page.tsx`) supports Enter-to-send / +Shift+Enter-for-newline on the message input: + +```tsx + setSendText(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter' && !e.shiftKey) { + e.preventDefault(); + void handleSendEncrypted(); + } + }} +/> +``` + +When adding a keyboard shortcut like this, always `preventDefault()` only on the branch +that consumes the key (here, plain Enter) and let every other key (including Shift+Enter) +fall through untouched β€” do not swallow keys you are not handling. + +The composer's send and attach-file buttons are real `