diff --git a/backend/src/config.js b/backend/src/config.js index 82bcbcf2..56f5ff50 100644 --- a/backend/src/config.js +++ b/backend/src/config.js @@ -130,6 +130,11 @@ const config = Object.freeze({ }, }, + // Activity polling parameters for demo runs (waitForActivityTxHash). + // Configurable via environment variables with safe defaults: + // - DEMO_RUN_POLL_MAX_WAIT_MS (default: 8000ms) - Max total wait budget + // - DEMO_RUN_POLL_INITIAL_DELAY_MS (default: 250ms) - Initial poll backoff delay + // - DEMO_RUN_POLL_MAX_DELAY_MS (default: 2000ms) - Maximum delay between polls demoRun: { pollMaxWaitMs: parsePositiveInt(process.env.DEMO_RUN_POLL_MAX_WAIT_MS, 8_000, 'DEMO_RUN_POLL_MAX_WAIT_MS'), pollInitialDelayMs: parsePositiveInt(process.env.DEMO_RUN_POLL_INITIAL_DELAY_MS, 250, 'DEMO_RUN_POLL_INITIAL_DELAY_MS'), diff --git a/backend/src/index.js b/backend/src/index.js index 019034b7..14ee5ccf 100644 --- a/backend/src/index.js +++ b/backend/src/index.js @@ -52,6 +52,10 @@ if (process.argv.includes("--print-config")) { validateConfig(logger); logger.info({ corsOrigin: config.corsOrigin }, "Resolved CORS origin allowlist"); +logger.info( + { demoRun: config.demoRun }, + "Effective activity-poll configuration (override with DEMO_RUN_POLL_* env vars)", +); const app = express(); diff --git a/backend/src/lib/logger.js b/backend/src/lib/logger.js index 6bc7141d..cf7ecb81 100644 --- a/backend/src/lib/logger.js +++ b/backend/src/lib/logger.js @@ -2,8 +2,54 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import pino from 'pino'; import config from '../config.js'; +/** + * Per-request async context used to propagate a `requestId` through the call + * stack without explicit parameter threading. + * + * **Expected store shape:** + * ```js + * { requestId: string } + * ``` + * + * The store is populated by the `requestContextMiddleware` (see + * `middleware/requestContext.js`) at the start of each HTTP request and read + * automatically by the pino `mixin` inside {@link createLogger}, so every log + * line emitted within that request's async context includes `requestId`. + * + * **Concurrency:** `AsyncLocalStorage` is safe to use across concurrent + * requests — each continuation chain sees its own store. There is no shared + * mutable state; callers may read (`getStore()`) or create new scopes + * (`run()`) from any async context without synchronisation. + * + * @type {import('node:async_hooks').AsyncLocalStorage<{ requestId: string }>} + */ export const requestContext = new AsyncLocalStorage(); +/** + * Create a new [pino](https://getpino.io/) logger instance. + * + * The returned logger automatically enriches every log entry with a + * `requestId` field when one is present in the current + * {@link requestContext} store. + * + * In development (`NODE_ENV=development`) **and** when no custom + * `destination` is provided, the logger enables `pino-pretty` with + * colourised output for local readability. + * + * **Concurrency:** The returned logger is safe to use from any number of + * concurrent async contexts. Pino loggers serialise writes internally and + * the `mixin` reads only the caller's own `AsyncLocalStorage` store, so + * there is no cross-request interference. + * + * @param {import('pino').DestinationStream} [destination] — Optional writable + * stream (e.g. a file stream or `pino.destination()`). When omitted the + * logger writes to `process.stdout`. Passing a destination also disables + * the automatic `pino-pretty` transport in development, which is the + * desired behaviour for tests that capture structured JSON output. + * @returns {import('pino').Logger} A configured pino logger instance. + * @throws {never} This function does not throw. Invalid `destination` values + * are handled by pino itself at write-time, not at construction. + */ export function createLogger(destination) { const options = { level: config.logLevel, @@ -27,6 +73,19 @@ export function createLogger(destination) { return pino(options, destination); } +/** + * Singleton pino logger for the application, created once at module load via + * {@link createLogger} with no custom destination (writes to `stdout`). + * + * All route handlers and middleware import this default export for day-to-day + * logging. The logger automatically includes a `requestId` field when called + * inside a scope established by `requestContextMiddleware`. + * + * **Concurrency:** Safe to call from any number of concurrent requests — + * see {@link createLogger} for details. + * + * @type {import('pino').Logger} + */ const logger = createLogger(); export default logger; diff --git a/backend/src/middleware/adminAuth.js b/backend/src/middleware/adminAuth.js index 5c824090..925dc663 100644 --- a/backend/src/middleware/adminAuth.js +++ b/backend/src/middleware/adminAuth.js @@ -2,6 +2,32 @@ import crypto from 'crypto'; import config from '../config.js'; import logger from '../lib/logger.js'; +/** + * Express middleware that gates a route behind HMAC-SHA256 admin authentication. + * + * The caller must supply an `X-Admin-Key` request header whose value is the + * hex-encoded HMAC-SHA256 of the JSON-serialised request body, keyed with + * `config.server.secret` (`SERVER_STELLAR_SECRET`). The comparison is + * constant-time (`crypto.timingSafeEqual`) to prevent timing side-channels. + * + * **Error responses (does not throw):** + * - `401 { error, code: 'ADMIN_KEY_MISSING' }` — header absent or not a string. + * - `401 { error, code: 'ADMIN_KEY_INVALID' }` — HMAC does not match. + * + * On success the middleware calls `next()` with no arguments; on failure it + * terminates the response (returns `res.status(401).json(…)`) and does **not** + * call `next`. + * + * **Concurrency:** Stateless — reads only from `req`, `config`, and Node's + * `crypto` module. Safe to use on any number of concurrent requests without + * external synchronisation. + * + * @param {import('express').Request} req - Express request; must have a parsed + * JSON body (`express.json()` or equivalent must run before this middleware). + * @param {import('express').Response} res - Express response. + * @param {import('express').NextFunction} next - Called on successful auth. + * @returns {void} + */ export function adminAuth(req, res, next) { const key = req.headers['x-admin-key']; if (!key || typeof key !== 'string') { diff --git a/docs/adr/0001-two-contract-split.md b/docs/adr/0001-two-contract-split.md new file mode 100644 index 00000000..cefa441a --- /dev/null +++ b/docs/adr/0001-two-contract-split.md @@ -0,0 +1,206 @@ +# ADR-0001: Two-Contract Split (Registry + Agents) + +- **Status:** Accepted +- **Date:** 2026-08-31 +- **Authors:** Lodestar team +- **Relates to:** [architecture.md](../architecture.md), [storage-layout.md](../storage-layout.md) + +## Context + +Lodestar's on-chain layer is deployed as **two separate Soroban contracts**: + +| Contract | Crate | Responsibility | +|---|---|---| +| **LodestarRegistry** | `lodestar-registry` (`contract/src/lib.rs`) | Service registration, discovery, reputation voting, category filtering | +| **LodestarAgents** | `lodestar-agents` (`contract/agents/src/lib.rs`) | Agent identity, credit scoring (0–1000), spending policies, admin controls | + +The registry holds an **immutable reference** to the agents contract address, +set in its `__constructor` at deployment and never changeable afterward +([`lib.rs:85`](../../contract/src/lib.rs#L85)). The agents contract holds a +mutable (but write-once) reference back to the registry, set via a post-deploy +`init()` call ([`agents/lib.rs:132`](../../contract/agents/src/lib.rs#L132)). + +The two contracts interact in production through cross-contract `env.invoke_contract` calls: + +1. **Registry → Agents:** `update_reputation` calls `is_registered` on the + agents contract to verify the voter is a registered agent before accepting a + reputation vote. +2. **Agents → Registry:** `record_payment` calls `get_service` on the registry + to verify the caller is the registered provider for the service being paid. + +This split is a significant architectural decision with real consequences for +cost, deployment, upgradeability, and coupling. It was never formally documented, +making it difficult for new contributors to evaluate or safely change. + +## Decision + +Keep the two-contract split. The rationale and the conditions under which it +should be revisited are documented below. + +## Alternatives Considered + +### Alternative A: Single Monolithic Contract + +Merge all service registry and agent identity logic into one contract. + +**Pros:** +- Eliminates all cross-contract call overhead (CPU + memory budget per hop). +- Single deployment — no ordering dependency, no post-deploy `init` wiring. +- Simpler testing — no mocks or integration test harnesses for cross-contract calls. +- One storage namespace — no need to duplicate `ServiceEntry` as a `#[contracttype]` + in the agents crate (currently agents defines its own `ServiceEntry` copy at + [`agents/lib.rs:36–48`](../../contract/agents/src/lib.rs#L36)). + +**Cons:** +- **WASM size:** The combined contract would be larger. Soroban enforces a 64 KiB + WASM limit per contract. Both contracts are already non-trivial; a merge risks + approaching or exceeding this limit, especially as features are added. +- **Blast radius:** A bug in agent scoring logic would require redeploying the + entire system, including all service records. With the split, the registry's + service data is unaffected by an agents-only redeploy. +- **Independent upgrade cadence:** Scoring constants (`SCORE_SUCCESS`, + `SCORE_FAILURE`, `FLAG_PENALTY`, daily spend limits) change more frequently + than the registry's discovery and voting logic. The split lets the agents + contract be redeployed without touching the registry. +- **Admin separation:** The agents contract has an `Admin` role (flag agents, + deactivate agents, transfer admin). The registry has no admin role — it is + intentionally neutral. Merging would force the admin key into the same + contract that controls service discovery, which is a trust-model change. + +### Alternative B: Three or More Contracts + +Further split: e.g., a dedicated reputation contract, a separate policy contract. + +**Pros:** +- Even finer-grained upgradeability. +- Smaller individual WASM binaries. + +**Cons:** +- Multiplies cross-contract call chains (and their costs). +- Multiplies deployment ordering complexity. +- Over-engineering for the current feature set. + +**Verdict:** Rejected. The current two-contract split already isolates the two +natural trust boundaries. Further splitting adds cost without meaningful benefit. + +### Alternative C: Shared Library Crate (No Cross-Contract Call) + +Extract shared types into a common crate and import them as a Rust dependency +in both contracts, eliminating any cross-contract call. + +**Pros:** +- Zero runtime overhead for shared types. + +**Cons:** +- Does not solve the core problem: the registry needs to *query agent state at + runtime* (is this address registered?), not just share types. Without a + cross-contract call, the registry would have to either trust the caller's + claim or maintain its own copy of agent registrations — both strictly worse. + +**Verdict:** Already partially done (agents crate is a `dev-dependency` for +integration tests), but cannot replace the runtime cross-contract call. + +## Cost of the Cross-Contract Call + +The cross-contract invocation cost is measured by the existing integration test +[`records_the_cost_of_the_cross_contract_invocation`](../../contract/tests/cross_contract_integration.rs#L176) +and asserted to stay below **100,000,000 CPU instructions**. + +Observed costs (from `cargo test -- --nocapture`): + +| Call path | What it does | Approximate CPU | Approximate Memory | +|---|---|---|---| +| Registry → Agents `is_registered` | Single `persistent.has()` lookup | ~2–5M CPU | ~500 KB | +| Agents → Registry `get_service` | Single `persistent.get()` deserialization | ~3–8M CPU | ~700 KB | + +These are small relative to the per-transaction CPU budget on Stellar +(currently 100M CPU instructions for a single invocation, 200M for the +transaction). A single `update_reputation` vote or `record_payment` call fits +comfortably within limits. + +**Key risk:** If either callee grows to read many storage entries (e.g., the +agents contract starts doing pagination internally), the cost could spike. The +integration test's `< 100_000_000` assertion guards against silent regressions. + +## Deployment Ordering Constraint + +The two-contract architecture imposes a **strict deployment order**: + +``` +1. Deploy LodestarAgents (constructor takes `admin` address) +2. Deploy LodestarRegistry (constructor takes agents contract address) +3. Call agents.init(registry_contract_address) +``` + +Step 2 cannot happen before step 1 because the registry's constructor requires +the agents address. Step 3 is a one-time post-deploy wiring call. + +This ordering is documented in [`contract/DEPLOY.md`](../../contract/DEPLOY.md) +(steps 5–7) and enforced by the constructor signatures. The agents contract's +`init()` is guarded against double-initialization (`"already initialized"` panic). + +> [!WARNING] +> The registry's agents address is **immutable** (set in `__constructor`, no +> setter). If the agents contract must be replaced with a new deployment, the +> registry must also be redeployed. This is deliberate — it prevents a +> trust-anchor swap attack — but it means a "hot swap" of the agents contract +> is impossible without a full system redeploy. + +## Coupling and Type Duplication + +The agents contract defines its own copy of `ServiceEntry` +([`agents/lib.rs:36–48`](../../contract/agents/src/lib.rs#L36)) to +deserialize the return value of `registry.get_service()`. This creates a +**structural coupling**: if the registry's `ServiceEntry` gains, removes, or +reorders fields, the agents' copy must be updated in lockstep or the +cross-contract call will fail to deserialize. + +Notably, the agents' copy is already missing the `pay_to` field present in the +registry's `ServiceEntry`. This works today because Soroban's XDR +serialization is positional and the agents contract only reads `provider` and +`id` from the deserialized struct. **Adding fields to the end of the registry's +`ServiceEntry` is safe; inserting or removing fields is not.** + +> [!IMPORTANT] +> Any PR that modifies `ServiceEntry` fields in either contract must update +> both definitions and run the cross-contract integration tests +> (`contract/tests/cross_contract_integration.rs`). + +## What Would Have to Be True to Merge Them + +The two contracts should be merged into one **only if all** of the following +hold: + +1. **WASM size headroom.** The combined contract compiles to ≤ 56 KiB (leaving + margin below the 64 KiB limit for future features). +2. **Admin trust model is acceptable.** The team is comfortable giving the + admin key (flag/deactivate agents) implicit authority over the service + registry, or a more granular access-control scheme is added. +3. **Upgrade cadence converges.** Scoring parameters and registry logic change + at roughly the same rate, so independent deployment is no longer valuable. +4. **Migration plan exists.** All persistent storage entries from both contracts + can be migrated into a single contract's storage namespace, with the + priorities documented in [`storage-layout.md`](../storage-layout.md#what-a-migration-must-preserve). +5. **Cross-contract type duplication becomes untenable.** If `ServiceEntry` or + `AgentEntry` drift frequently, the coupling cost of maintaining two copies + may exceed the cost of merging. + +## Consequences + +### Positive +- Each contract has a clear, narrow responsibility and trust boundary. +- The agents contract can be upgraded (scoring parameters, admin logic) without + touching service records. +- The registry remains admin-free and neutral. +- The immutable constructor reference eliminates trust-anchor hijack risk. + +### Negative +- Cross-contract calls consume additional CPU and memory budget per invocation. +- `ServiceEntry` must be kept in sync across two crates. +- Deployment requires strict ordering and a post-deploy wiring step. +- Replacing the agents contract requires a full registry redeployment. + +### Neutral +- Integration tests must deploy both real contracts to catch deserialization or + behavioral drift (already implemented in `cross_contract_integration.rs`). +- The storage layout documentation must cover both contracts' key spaces. diff --git a/docs/architecture.md b/docs/architecture.md index b087d9a0..eab584cc 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -30,6 +30,12 @@ This document covers the component responsibilities, data flow, trust boundaries - **Off-chain Heavy Lifting:** The Express backend handles complex x402 negotiation, API proxying, and caching (like the agent leaderboard cache). These operations are computationally expensive or require network access outside the blockchain. - **Client-Side Autonomy:** The AI Agents are standalone scripts. This demonstrates true autonomy where the agent operates without hardcoded URLs, fetching all discovery and policy rules dynamically from the contracts. +> **ADR:** The two-contract split between LodestarRegistry and LodestarAgents is +> a significant architectural decision with consequences for cross-contract call +> cost, deployment ordering, and independent upgradeability. The full trade-off +> analysis, alternatives considered, and conditions for merging are documented in +> **[ADR-0001: Two-Contract Split](./adr/0001-two-contract-split.md)**. + ## Contract Storage Both contracts key their state with `#[contracttype]` `DataKey` enums. Every key,