Feature/indexer service - #277
Conversation
|
|
||
| // verified event | ||
| if (topics[0] === "proof" && topics[1] === "verified") { | ||
| const holder = ev.source_account ?? ""; |
There was a problem hiding this comment.
Empty wallet written when
source_account is absent
ev.source_account is typed as optional (source_account?: string), so when Horizon omits it the fallback ?? "" writes a row with wallet = "". That row is unqueryable via /claims?wallet=G…, silently discarded for the user, and counted in /stats and /recent under a blank wallet. Any event submitted through a fee-bump or multi-signature path where the source isn't the holder will produce phantom rows.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/indexer/src/ingester.ts
Line: 162
Comment:
**Empty wallet written when `source_account` is absent**
`ev.source_account` is typed as optional (`source_account?: string`), so when Horizon omits it the fallback `?? ""` writes a row with `wallet = ""`. That row is unqueryable via `/claims?wallet=G…`, silently discarded for the user, and counted in `/stats` and `/recent` under a blank wallet. Any event submitted through a fee-bump or multi-signature path where the source isn't the holder will produce phantom rows.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| RPC_URL=https://soroban-testnet.stellar.org | ||
| NETWORK_PASSPHRASE=Test SDF Network ; September 2015 | ||
| PROOF_REGISTRY_CONTRACT_ID= | ||
|
|
There was a problem hiding this comment.
POLL_INTERVAL_MS in .env.example does not match POLL_INTERVAL_SECONDS read by config.ts
config.ts reads process.env["POLL_INTERVAL_SECONDS"] and multiplies by 1000. A developer who copies .env.example and sets POLL_INTERVAL_MS=5000 will have that value silently ignored — the service will use the hardcoded default of 6 s instead of 5 s. The variable name in the example file must be POLL_INTERVAL_SECONDS.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/indexer/.env.example
Line: 8
Comment:
**`POLL_INTERVAL_MS` in `.env.example` does not match `POLL_INTERVAL_SECONDS` read by `config.ts`**
`config.ts` reads `process.env["POLL_INTERVAL_SECONDS"]` and multiplies by 1000. A developer who copies `.env.example` and sets `POLL_INTERVAL_MS=5000` will have that value silently ignored — the service will use the hardcoded default of 6 s instead of 5 s. The variable name in the example file must be `POLL_INTERVAL_SECONDS`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
|
||
| # ── StellarCred frontend ───────────────────────────────────────────────── | ||
| frontend: | ||
| build: | ||
| context: . | ||
| dockerfile: docker/Dockerfile.dev | ||
| image: stellarcred-dev | ||
| working_dir: /workspace/frontend | ||
| volumes: | ||
| - ./frontend:/workspace/frontend | ||
| - frontend-node-modules:/workspace/frontend/node_modules | ||
| context: ./frontend | ||
| dockerfile: Dockerfile | ||
| restart: unless-stopped | ||
| environment: | ||
| NEXT_PUBLIC_STELLAR_NETWORK: ${NEXT_PUBLIC_STELLAR_NETWORK:-testnet} | ||
| NEXT_PUBLIC_RPC_URL: ${NEXT_PUBLIC_RPC_URL:-https://soroban-testnet.stellar.org} | ||
| NEXT_PUBLIC_NETWORK_PASSPHRASE: ${NEXT_PUBLIC_NETWORK_PASSPHRASE:-Test SDF Network ; September 2015} | ||
| NEXT_PUBLIC_ISSUER_ADDRESS: ${NEXT_PUBLIC_ISSUER_ADDRESS:-} | ||
| NEXT_PUBLIC_STELLARCRED_BASE_URL: ${NEXT_PUBLIC_STELLARCRED_BASE_URL:-} | ||
| NEXT_PUBLIC_ISSUER_REGISTRY_ID: ${NEXT_PUBLIC_ISSUER_REGISTRY_ID:-} | ||
| NEXT_PUBLIC_CREDENTIAL_VERIFIER_ID: ${NEXT_PUBLIC_CREDENTIAL_VERIFIER_ID:-} | ||
| NEXT_PUBLIC_PROOF_REGISTRY_ID: ${NEXT_PUBLIC_PROOF_REGISTRY_ID:-} | ||
| NEXT_PUBLIC_GATED_POOL_ID: ${NEXT_PUBLIC_GATED_POOL_ID:-} | ||
| # Indexer URL accessible inside the Next.js server (same docker network) | ||
| NEXT_PUBLIC_INDEXER_URL: ${NEXT_PUBLIC_INDEXER_URL:-http://indexer:3001} | ||
| ISSUER_PRIVATE_KEY: ${ISSUER_PRIVATE_KEY:-} | ||
| PERSONA_API_KEY: ${PERSONA_API_KEY:-} | ||
| PERSONA_KYC_TEMPLATE_ID: ${PERSONA_KYC_TEMPLATE_ID:-} | ||
| PLAID_CLIENT_ID: ${PLAID_CLIENT_ID:-} | ||
| PLAID_SECRET: ${PLAID_SECRET:-} | ||
| PLAID_ACCESS_TOKEN: ${PLAID_ACCESS_TOKEN:-} | ||
| PLAID_ENV: ${PLAID_ENV:-sandbox} | ||
| ports: | ||
| - "3000:3000" | ||
| command: ["sh", "-c", "pnpm install && pnpm dev -- -H 0.0.0.0"] | ||
|
|
||
| circuits: | ||
| build: | ||
| context: . | ||
| dockerfile: docker/Dockerfile.dev | ||
| image: stellarcred-dev | ||
| working_dir: /workspace/circuits | ||
| volumes: | ||
| - ./circuits:/workspace/circuits | ||
| command: ["nargo", "compile", "--workspace"] | ||
| - "${FRONTEND_PORT:-3000}:3000" | ||
| depends_on: | ||
| indexer: | ||
| condition: service_healthy | ||
|
|
||
| volumes: | ||
| frontend-node-modules: | ||
| indexer_data: |
There was a problem hiding this comment.
Existing
contracts and circuits services removed from Docker Compose
The original docker-compose.yml contained a contracts service (cargo test && cargo build) and a circuits service (nargo compile --workspace). Both have been dropped in this PR. The PR description only mentions adding the indexer service; removing these services appears unintentional and would break any CI pipelines or developer workflows that rely on docker compose up contracts or docker compose up circuits. These should either be restored or the removal should be explicitly acknowledged in the PR description.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docker-compose.yml
Line: 38-71
Comment:
**Existing `contracts` and `circuits` services removed from Docker Compose**
The original `docker-compose.yml` contained a `contracts` service (`cargo test && cargo build`) and a `circuits` service (`nargo compile --workspace`). Both have been dropped in this PR. The PR description only mentions adding the `indexer` service; removing these services appears unintentional and would break any CI pipelines or developer workflows that rely on `docker compose up contracts` or `docker compose up circuits`. These should either be restored or the removal should be explicitly acknowledged in the PR description.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // We use a simple numeric cursor derived from ledger*100_000 which is how | ||
| // Horizon builds paging_tokens for events in practice. | ||
| const cursorNum = lastLedger > 0 ? lastLedger * 100_000 : 0; | ||
| const cursor = config.startLedger > 0 && lastLedger === 0 | ||
| ? String(config.startLedger * 100_000) | ||
| : cursorNum > 0 | ||
| ? String(cursorNum) | ||
| : undefined; |
There was a problem hiding this comment.
Horizon paging cursor derived from
ledger * 100_000 may not match actual token format
The comment states this is "how Horizon builds paging_tokens for events in practice," but Horizon's actual event paging token is an opaque string. Using lastLedger * 100_000 as a cursor may point into the middle of a ledger's events, causing some events to be skipped on restart, or may replay already-processed records. For correctness on resume, consider storing and replaying from the actual paging_token of the last processed record rather than a synthetic numeric cursor.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/indexer/src/ingester.ts
Line: 228-235
Comment:
**Horizon paging cursor derived from `ledger * 100_000` may not match actual token format**
The comment states this is "how Horizon builds paging_tokens for events in practice," but Horizon's actual event paging token is an opaque string. Using `lastLedger * 100_000` as a cursor may point into the middle of a ledger's events, causing some events to be skipped on restart, or may replay already-processed records. For correctness on resume, consider storing and replaying from the actual `paging_token` of the last processed record rather than a synthetic numeric cursor.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| export function buildApp(db: Db): express.Application { | ||
| const app = express(); | ||
|
|
||
| // Security: no body parsing (read-only), conservative headers. | ||
| app.disable("x-powered-by"); | ||
| app.use((_req, res, next) => { | ||
| res.setHeader("X-Content-Type-Options", "nosniff"); | ||
| res.setHeader("Cache-Control", "no-store"); | ||
| next(); | ||
| }); |
There was a problem hiding this comment.
cors package listed in package.json but never applied
cors is a runtime dependency in package.json yet no import cors or app.use(cors(...)) appears in api.ts. If the frontend JavaScript fetches the indexer from a different origin, all requests will be blocked by the browser's same-origin policy. The dependency either needs to be wired in, or removed if server-side-only access is intended.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/indexer/src/api.ts
Line: 42-51
Comment:
**`cors` package listed in `package.json` but never applied**
`cors` is a runtime dependency in `package.json` yet no `import cors` or `app.use(cors(...))` appears in `api.ts`. If the frontend JavaScript fetches the indexer from a different origin, all requests will be blocked by the browser's same-origin policy. The dependency either needs to be wired in, or removed if server-side-only access is intended.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // BigInts → numbers (safe for u64 ledger values in our use-case). | ||
| if (typeof native === "bigint") { | ||
| return Number(native); | ||
| } |
There was a problem hiding this comment.
BigInt → Number conversion may silently lose precision
Soroban u64 values range up to 2^64 − 1, which exceeds Number.MAX_SAFE_INTEGER (2^53 − 1). Number(native) on a large bigint will silently round the value. For expiry timestamps this matters: a precision-losing conversion could make a credential appear to expire at the wrong time.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/indexer/src/ingester.ts
Line: 95-98
Comment:
**`BigInt` → `Number` conversion may silently lose precision**
Soroban u64 values range up to 2^64 − 1, which exceeds `Number.MAX_SAFE_INTEGER` (2^53 − 1). `Number(native)` on a large bigint will silently round the value. For expiry timestamps this matters: a precision-losing conversion could make a credential appear to expire at the wrong time.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
closes #74
Description
This pull request introduces a standalone, lightweight indexer service designed to ingest ProofRegistry events directly from the Stellar RPC event stream / Horizon API. This allows for fast, read-only querying of public on-chain data (such as wallet credentials and stats) without needing to hit the blockchain for every client request.
The indexer is idempotent, robust to restarts, and guarantees zero storage of identity/private data.
🚀 Features & Changes
Event Ingestion Engine (src/ingester.ts):
Polls Horizon API for proof verified and revoked events.
Idempotently stores them using cursor-based tracking (lastLedger).
Safely resumes from the last known ledger on restarts to prevent duplicate records.
Database Adapters (src/db.ts):
Implements two database drivers:
SQLite (better-sqlite3): Optimized for development and single-node setups.
PostgreSQL (pg): Optimized for production/multi-instance setups.
Runs automatic, idempotent schema migrations on startup.
REST API (src/api.ts):
Exposes the following read-only, public-data endpoints:
GET /health: Health status and current syncing ledger.
GET /claims?wallet=G...: Fetches all credentials associated with a specific wallet address.
GET /stats: Aggregates metrics (total, active, revoked) by credential type.
GET /recent?limit=X&page=Y: Paginated view of the most recent verifications.
Testing & Tooling:
Cleaned up obsolete compiled .js files from the src/ directory.
Implemented ts-jest for proper TypeScript test execution.
Full test coverage over API responses and database behaviors (10/10 tests passing).
Documentation:
Added an Indexer Service section to the root README.md.
Added a dedicated setup README.md inside services/indexer/.
🛠️ How to Test
Check out this branch: git checkout feature/indexer-service
Run the tests to verify integrity:
bash
cd services/indexer
npm install
npm test
Start the service using Docker Compose from the project root:
bash
docker compose up indexer
Query the endpoints locally (defaults to port 3001):
curl http://localhost:3001/health
curl http://localhost:3001/stats
🔒 Security Notes
Privacy First: The indexer intercepts public events only. Identity fields are intentionally omitted and never persist on-chain or off-chain.
Immutability: All exposed endpoints are read-only (GET). The indexer does not mutate chain state nor sign any transactions.
Greptile Summary
This PR adds a new standalone
services/indexerservice that polls the Stellar Horizon API forProofRegistrycontract events, persists public on-chain data to SQLite or PostgreSQL, and exposes a read-only REST API. The overall architecture is clean and well-tested, but several correctness issues need to be addressed before merging.lastLedger \u00d7 100,000rather than using the opaquepaging_tokenreturned by the API, so events within a ledger may be skipped or replayed on restart; additionally,holderfalls back to an empty string whensource_accountis absent, silently writing unqueryable rows..env.exampledocumentsPOLL_INTERVAL_MSbutconfig.tsreadsPOLL_INTERVAL_SECONDS, so the value a developer sets in.envis silently ignored.contracts(Rust build/test) andcircuits(Noir compilation) services fromdocker-compose.yml, which is outside the stated scope and would break existing CI and developer workflows.Confidence Score: 3/5
Not ready to merge in its current state — the cursor mechanism risks silently skipping or replaying events on every restart, and the Docker Compose change removes existing project infrastructure outside the PR's stated scope.
The ingester's event cursor relies on a synthetic lastLedger × 100,000 value rather than Horizon's actual opaque paging token, meaning the service may miss or re-process events each time it restarts. The empty-string holder fallback writes rows that are permanently hidden from the /claims endpoint. The .env.example documents the wrong variable name so polling interval customisation is silently broken. The unscoped removal of the contracts and circuits Docker services could break existing CI pipelines.
Files Needing Attention: services/indexer/src/ingester.ts (cursor logic and empty holder), docker-compose.yml (removed services), services/indexer/.env.example (wrong env var name)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Horizon as Horizon API participant Ingester as ingester.ts participant DB as db.ts (SQLite/PG) participant API as api.ts (Express) participant Client as Frontend / curl loop Every pollIntervalMs Ingester->>DB: getLastLedger() DB-->>Ingester: lastLedger Ingester->>Horizon: "GET /contracts/{id}/events?cursor=lastLedger*100000" Horizon-->>Ingester: HorizonEventsPage loop For each event Ingester->>Ingester: parseEvent() alt verified Ingester->>DB: upsertClaim(row) else revoked Ingester->>DB: revokeClaim(wallet, type) end end Ingester->>DB: setLastLedger(maxLedger) end Client->>API: "GET /claims?wallet=G..." API->>DB: claimsByWallet(wallet) DB-->>API: ClaimRow[] API-->>Client: "{ wallet, claims }" Client->>API: GET /stats API->>DB: stats() DB-->>API: StatsRow[] API-->>Client: "{ stats }" Client->>API: "GET /recent?limit=20&page=1" API->>DB: recent(limit, offset) DB-->>API: ClaimRow[] API-->>Client: "{ claims, limit, page }"Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge branch 'main' into feature/indexer..." | Re-trigger Greptile