Skip to content

fix: dynamic contract registration for FPMM and VirtualPool#114

Merged
chapati23 merged 11 commits intomainfrom
fix/dynamic-contract-registration
Mar 31, 2026
Merged

fix: dynamic contract registration for FPMM and VirtualPool#114
chapati23 merged 11 commits intomainfrom
fix/dynamic-contract-registration

Conversation

@gisk0
Copy link
Copy Markdown
Collaborator

@gisk0 gisk0 commented Mar 30, 2026

Problem

The indexer had a hardcoded address list for FPMM and VirtualPool contracts. Any pool deployed after the config was last updated was silently missing all events.

Concrete impact found today:

  • Celo: EURm/USDm pool (0x1ad2ea06...) had 123 real Swap events on-chain but zero in the indexer
  • Celo: 0x3aa7c431... pool also missing
  • Would have affected every future pool deployment on any chain

Fix

Use Envio's contractRegister hook to dynamically register each pool as its factory emits a deploy event:

// fpmm.ts — added context.addFPMM
FPMMFactory.FPMMDeployed.contractRegister(({ event, context }) => {
  context.addFPMM(event.params.fpmmProxy); // ← was missing
  context.addERC20FeeToken(event.params.token0);
  context.addERC20FeeToken(event.params.token1);
});

// virtualPool.ts — added entire contractRegister (didn't exist before)
VirtualPoolFactory.VirtualPoolDeployed.contractRegister(({ event, context }) => {
  context.addVirtualPool(event.params.pool);
});

The hardcoded address lists in config.multichain.mainnet.yaml are replaced with address: [] — discovery is now fully automatic.

Note

The Envio hosted indexer will need a full reindex after deployment to backfill events for the previously-missing contracts. Push to the envio branch to trigger it.

Use contractRegister to auto-discover all factory-deployed pools instead of
maintaining a brittle hardcoded address list in the config.

- FPMMFactory.FPMMDeployed: add context.addFPMM(fpmmProxy) so every new
  FPMM pool is automatically indexed from its deployment block forward
- VirtualPoolFactory.VirtualPoolDeployed: add contractRegister +
  context.addVirtualPool(pool) for the same reason
- config.multichain.mainnet.yaml: replace all hardcoded FPMM/VirtualPool
  address lists with address: [] (dynamic registration handles discovery)

This fixes missing events for pools deployed after initial config authoring
(e.g. EURm/USDm on Celo: 123 real Swap events were silently dropped).
New pools on any chain are now indexed automatically without a config change.
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
monitoring-dashboard Ready Ready Preview, Comment Mar 31, 2026 7:37am

Request Review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR replaces hardcoded FPMM/VirtualPool address lists with dynamic registration via factory deploy events, which is the right direction for newly deployed pools. However, it introduces one data-loss regression path and lacks regression tests for the new indexing-critical behavior.

indexer-envio/config.multichain.mainnet.yaml

  • 🔴 CRITICAL (100, 106, 108, 125, 131, 133): FPMM/VirtualPool are now address: [], so indexing depends entirely on seeing deploy events after start_block. If ENVIO_START_BLOCK_CELO or ENVIO_START_BLOCK_MONAD is raised above initial deployments, previously deployed pools are never registered and all their events are silently dropped. Keep known pools as seed addresses (and still use dynamic registration for new ones), or implement deterministic backfill from factory logs.

indexer-envio/src/handlers/fpmm.ts

  • 🟡 IMPORTANT (100-103): context.addFPMM(...) is new indexing-critical logic, but this PR adds no regression test proving a pool absent from config becomes indexed after FPMMDeployed.

indexer-envio/src/handlers/virtualPool.ts

  • 🟡 IMPORTANT (23-27): same test gap for context.addVirtualPool(...); no regression test proves dynamically registered VirtualPools are indexed when not statically listed.

Final verdict: 🚫 Needs changes.

Open in Web View Automation 

Sent by Cursor Automation: PR Review

- 0xb285d4c7133d6f27bfb29224fb0d22e7ec3ddd2d # USDm/eb4663
- 0x462fe04b4fd719cbd04c0310365d421d02aaa19e # USDm/USDC
- 0x0feba760d93423d127de1b6abecdb60e5253228d # USDT/USDm
address: [] # Dynamically registered via FPMMFactory.FPMMDeployed.contractRegister
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 address: [] here removes the fallback static seed list.

With this change, pool indexing now requires seeing FPMMFactory.FPMMDeployed after start_block (ENVIO_START_BLOCK_CELO/ENVIO_START_BLOCK_MONAD). If an operator raises start block above first deployments, existing pools never get registered and their events are silently dropped.

Please keep known pools as seed addresses (while still using dynamic registration for new ones), or add an explicit factory-log backfill path.

// hardcoded address list in the config. Envio deduplicates addresses, so
// re-registering the same address on re-runs is harmless.
FPMMFactory.FPMMDeployed.contractRegister(({ event, context }) => {
context.addFPMM(event.params.fpmmProxy);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This is a behavior-critical change, but the PR adds no regression test that proves dynamic registration works when the pool is not in config.

Please add a test covering: empty FPMM address list -> process FPMMDeployed -> subsequent FPMM.Swap from deployed proxy is indexed.


// Dynamically register the deployed VirtualPool so Envio indexes its events
// (Swap, Mint, Burn, etc.) without a hardcoded address list in the config.
VirtualPoolFactory.VirtualPoolDeployed.contractRegister(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Same test-coverage gap as addFPMM: this adds indexing-critical runtime behavior without a regression test.

Please add a test covering: empty VirtualPool address list -> process VirtualPoolDeployed -> subsequent VirtualPool event is indexed.

- Delete config.celo.mainnet.yaml, config.monad.mainnet.yaml, config.celo.devnet.yaml
  (superseded by config.multichain.mainnet.yaml)
- Fix FPMM/VirtualPool address lists in config.celo.sepolia.yaml and
  config.monad.testnet.yaml → address: [] (dynamic registration applies to all configs)
- Fix bootstrap-worktree.sh: use config.multichain.mainnet.yaml instead of
  config.celo.devnet.yaml
- Update root AGENTS.md + indexer-envio/AGENTS.md: remove legacy pnpm commands
  (indexer:celo-mainnet:*, indexer:monad-mainnet:*), update file tree, fix codegen
  command references
- Add framework note to contractRegister callbacks explaining why they have no
  test coverage (Envio test harness limitation, not an oversight)
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR switches FPMM/VirtualPool indexing from hardcoded address lists to dynamic contract registration hooks, and removes legacy per-chain mainnet config files. The dynamic-registration direction is good, but there are merge-blocking regressions introduced in this diff.

indexer-envio/config.celo.mainnet.yaml

  • 🔴 CRITICAL (deleted L1-L93): Removing this file breaks existing CI validation that still runs codegen --config config.celo.mainnet.yaml in .github/workflows/indexer-envio.yml L66. The current PR already fails Quality Checks (indexer-envio) with EE104: Failed to resolve config path config.celo.mainnet.yaml.

indexer-envio/config.monad.mainnet.yaml

indexer-envio/config.celo.sepolia.yaml

  • 🟡 IMPORTANT (L8, L19, L32): FPMM and VirtualPool moved to address: [] while keeping the same start_block. This now hard-depends on FPMMDeployed / VirtualPoolDeployed events being within the indexed range. Any pool deployed before ENVIO_START_BLOCK will never be registered or indexed on a fresh sync.

bootstrap-worktree.sh

  • 🟢 SUGGESTION (L12-L14): The fallback path logs "trying devnet" but reruns config.multichain.mainnet.yaml; this fallback is effectively a no-op and the message is misleading.

AGENTS.md

  • 🟢 SUGGESTION (L47, L196): Documentation still references deleted configs (config.celo.devnet.yaml, config.celo.mainnet.yaml, config.monad.mainnet.yaml), so the guide is now inconsistent with the repo state.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

CI blocker: update workflow to validate config.multichain.mainnet.yaml,
config.celo.sepolia.yaml, config.monad.testnet.yaml (removed deleted
celo.mainnet and monad.mainnet configs).

Data integrity: restore known FPMM/VirtualPool seed addresses in
config.multichain.mainnet.yaml alongside dynamic registration. This ensures
pools are indexed even if ENVIO_START_BLOCK is raised above a pool's
deploy block. Dynamic registration via contractRegister still handles all
future deployments; Envio deduplicates so listing a pool in both places
is harmless.

Celo: 6 FPMM seeds (blocks 60668100 + 62725622) + 12 VirtualPool seeds
Monad: 3 FPMM seeds (block 60759432); no VirtualPools deployed yet

Also: remove no-op devnet fallback from bootstrap-worktree.sh (was
logging 'trying devnet' but running multichain config — misleading).
@blacksmith-sh

This comment has been minimized.

Remove seed addresses (per preference — dynamic registration should be
sufficient). Instead, throw a fatal error at module load time if
ENVIO_START_BLOCK is set above the FPMMFactory's first deployment block
for any chain.

If start_block is too high, FPMMDeployed events are never seen,
contractRegister never fires, and all pool events are silently dropped.
The startup check catches this misconfiguration immediately with a clear
error message rather than producing an empty database.

Known first-deploy blocks:
  Celo mainnet (42220):  60668100
  Monad mainnet (143):   60759432
  Celo Sepolia (11142220): 18946570
  Monad testnet (10143): 17932599
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR replaces hardcoded pool address lists with dynamic Envio contractRegister registration for FPMM/VirtualPool, and removes legacy per-chain mainnet/devnet config files in favor of multichain defaults. The direction is good, but there are blocking runtime and CI validation regressions.

indexer-envio/src/EventHandlers.ts

  • 🔴 CRITICAL

  • 31-32, 35-49: START_BLOCK_ENV maps both chain 11142220 and 10143 to the same ENVIO_START_BLOCK value. In single-chain Celo Sepolia runs, valid Sepolia overrides (including the repo script value ENVIO_START_BLOCK=18901381) are incorrectly checked against Monad testnet’s lower threshold (17932599) and can throw a fatal error for chain 10143. This can break pnpm indexer:celo-sepolia:dev even when Sepolia settings are valid.

  • 🟡 IMPORTANT

  • 31-32: The startup guard does not read ENVIO_START_BLOCK_CELO_SEPOLIA / ENVIO_START_BLOCK_MONAD_TESTNET, so it is not effective for the multichain testnet config that uses per-chain env overrides.

.github/workflows/indexer-envio.yml

  • 🟡 IMPORTANT
  • 63-65: The workflow says it validates every Envio config, but it never runs codegen for config.multichain.testnet.yaml. That leaves one active config without CI schema/config validation.
  • 63-65: Because generated output is overwritten each run, typecheck/tests now use the last codegen target (config.monad.testnet.yaml), not the default production config (config.multichain.mainnet.yaml). This weakens the signal from downstream checks.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

const START_BLOCK_ENV: Record<number, string | undefined> = {
42220: process.env.ENVIO_START_BLOCK_CELO,
143: process.env.ENVIO_START_BLOCK_MONAD,
11142220: process.env.ENVIO_START_BLOCK,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ENVIO_START_BLOCK is being reused for both Sepolia (11142220) and Monad testnet (10143). This causes false failures in single-chain Sepolia runs: a value that is valid for Sepolia can still be greater than Monad testnet’s first-deploy block and trigger a fatal error for the wrong chain.

Concrete example: the repo script uses ENVIO_START_BLOCK=18901381 for Sepolia; this exceeds 10143's 17932599 threshold and can abort startup even though Sepolia is configured correctly.

Recommend scoping checks to active chain(s) from the loaded config, or at minimum avoid mapping one generic env var to multiple unrelated chains.

pnpm --filter @mento-protocol/indexer-envio codegen --config config.multichain.mainnet.yaml
pnpm --filter @mento-protocol/indexer-envio codegen --config config.celo.sepolia.yaml
pnpm --filter @mento-protocol/indexer-envio codegen --config config.celo.mainnet.yaml
pnpm --filter @mento-protocol/indexer-envio codegen --config config.monad.testnet.yaml
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues in this block:

  1. config.multichain.testnet.yaml is not validated at all, despite the step comment saying all configs are validated.
  2. Since codegen output is overwritten on each command, downstream typecheck/tests run against the last config here (config.monad.testnet.yaml), not the default config (config.multichain.mainnet.yaml).

Please include all active configs and make the intended downstream target explicit (run it last).

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR switches indexer contract discovery to dynamic contractRegister for FPMM/VirtualPool and removes legacy per-chain mainnet/devnet configs in favor of multichain defaults. The direction is good, but there are several correctness and verification gaps that can lead to silent indexing failures and CI blind spots.

indexer-envio/config.celo.sepolia.yaml

  • 🟡 IMPORTANT (logic correctness): L19 and L32 set FPMM/VirtualPool addresses to [], making indexing depend entirely on historical FPMMDeployed/VirtualPoolDeployed events. If ENVIO_START_BLOCK is raised above those deploy blocks, pools are never registered and swaps/mints/burns are silently missed.
  • Suggested fix:
- name: FPMM
  address:
    - 0x... # known deployed pools (seed set)
- name: VirtualPool
  address:
    - 0x... # known deployed virtual pools (seed set)

indexer-envio/config.monad.testnet.yaml

  • 🟡 IMPORTANT (logic correctness): L17 has the same failure mode for FPMM on Monad testnet (address: [] + dynamic-only registration). A higher start block yields zero registered pools until a new deployment happens.

.github/workflows/indexer-envio.yml

  • 🟡 IMPORTANT (coverage gap): L56-L65 says “Validate all Envio configs” but does not validate config.multichain.testnet.yaml. That leaves one maintained config outside CI schema/codegen validation.
  • Suggested fix:
run: |
  pnpm --filter @mento-protocol/indexer-envio codegen --config config.multichain.mainnet.yaml
  pnpm --filter @mento-protocol/indexer-envio codegen --config config.multichain.testnet.yaml
  pnpm --filter @mento-protocol/indexer-envio codegen --config config.celo.sepolia.yaml
  pnpm --filter @mento-protocol/indexer-envio codegen --config config.monad.testnet.yaml
  • 🟢 SUGGESTION (readability/accuracy): L59 still says “Celo mainnet is run last,” but Celo-mainnet-specific config no longer exists.

indexer-envio/src/handlers/fpmm.ts

  • 🟡 IMPORTANT (test coverage): L105-L109 introduces critical registration behavior (context.addFPMM(...)) on a path that the current tests do not execute (as noted in L101-L104). This is new production logic with no automated verification.

indexer-envio/src/handlers/virtualPool.ts

  • 🟡 IMPORTANT (test coverage): L25-L29 adds critical dynamic registration (context.addVirtualPool(...)) on the same untested path.

bootstrap-worktree.sh

  • 🟡 IMPORTANT (error handling): L18-L19 suppresses stderr and swallows failing tests (|| echo ...), then prints “ready to code.” This can mask broken worktrees and false-green setup.
  • Suggested fix:
pnpm --filter @mento-protocol/ui-dashboard test -- --run
pnpm --filter @mento-protocol/indexer-envio test

AGENTS.md

  • 🟢 SUGGESTION (doc consistency): L47 and L196 still reference removed configs (config.celo.devnet.yaml, config.celo.mainnet.yaml, config.monad.mainnet.yaml), which now conflicts with other updated sections.

indexer-envio/AGENTS.md

  • 🟢 SUGGESTION (doc consistency): L5 says the indexer is “on Celo,” but this repo now uses multichain Celo + Monad configs.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

CI: add config.multichain.testnet.yaml to codegen validation step,
move multichain.mainnet last (output used by typecheck + tests),
fix stale comment (no longer 'Celo mainnet is run last').

bootstrap-worktree.sh: remove error-swallowing from test runs.
Tests now fail loudly if broken, so setup is never false-green.

Docs: fix stale config refs in AGENTS.md (root + indexer):
- root AGENTS.md L47: replace per-chain configs with multichain
- root AGENTS.md L196: same in 'Adding a new contract' guide
- indexer-envio/AGENTS.md: update 'on Celo' to 'Celo + Monad (multichain)'

Testnet configs: add comment explaining why address:[] is safe —
EventHandlers.ts startup invariant throws if ENVIO_START_BLOCK is
raised above the factory's first deploy block on any chain.
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR replaces hardcoded FPMM/VirtualPool address lists with dynamic contractRegister registration and adds startup guardrails intended to prevent missed deploy events when start blocks are set too high.

Findings

indexer-envio/src/EventHandlers.ts

  • 🔴 Critical (blocking)L31-L32 introduces a false-fatal regression for single-chain testnet runs by reusing the same ENVIO_START_BLOCK env var for both chain checks.
    • With config.celo.sepolia.yaml, ENVIO_START_BLOCK is intended for chain 11142220, but this code also validates it against chain 10143.
    • Example: ENVIO_START_BLOCK=18901381 (the current Celo Sepolia dev value) is valid for Sepolia but > 17932599, so startup throws for Monad testnet even though that chain is not in the active config.
    • This breaks legitimate Sepolia startup/configs and turns the safety check into a blocker.
    • Fix by removing cross-chain reuse of the generic env var: use chain-specific start block env vars per config (and validate those), or scope validation to only chains present in the active config.

Final Verdict

🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

const START_BLOCK_ENV: Record<number, string | undefined> = {
42220: process.env.ENVIO_START_BLOCK_CELO,
143: process.env.ENVIO_START_BLOCK_MONAD,
11142220: process.env.ENVIO_START_BLOCK,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issue: this maps ENVIO_START_BLOCK to chain 11142220, but line 32 maps the same env var to 10143 too. That creates false failures for single-chain configs.

For config.celo.sepolia.yaml, ENVIO_START_BLOCK is intended for Sepolia only, but startup will also validate it against Monad testnet and throw when the value is above 17932599 (even if chain 10143 is not active). This regresses valid Sepolia runs.

Please scope validation to active chains or switch testnet configs to chain-specific env vars and validate those one-to-one.

Per review: single-chain configs are now redundant since
config.multichain.testnet.yaml covers Celo Sepolia + Monad testnet.

- Delete config.celo.sepolia.yaml and config.monad.testnet.yaml
- Replace indexer:celo-sepolia:* and indexer:monad-testnet:* pnpm scripts
  with indexer:testnet:codegen / indexer:testnet:dev (multichain testnet)
- CI workflow: validate only the two multichain configs
- Fix EventHandlers.ts startup invariant: use chain-specific env vars
  (ENVIO_START_BLOCK_CELO_SEPOLIA, ENVIO_START_BLOCK_MONAD_TESTNET) instead
  of generic ENVIO_START_BLOCK — prevents false-fatal errors when running a
  single-chain run where the value is valid for one chain but exceeds first
  deploy block on another chain not in the active config
- Update all AGENTS.md and docs references
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR switches the indexer to dynamic contract registration for FPMM/VirtualPool and consolidates operational flows around the multichain configs/scripts. The direction is good, but there are a couple of correctness/operability gaps that should be fixed before merge.

indexer-envio/src/EventHandlers.ts

  • 🟡 Important (line 52): the fatal remediation message prints ENVIO_START_BLOCK for chain IDs 11142220 and 10143, but the code actually reads ENVIO_START_BLOCK_CELO_SEPOLIA and ENVIO_START_BLOCK_MONAD_TESTNET. This gives operators the wrong fix path in the exact failure mode this guard is meant to protect.
  • 🟡 Important (lines 21-57): this adds startup-critical branching logic (env parsing + chain-specific invariant checks) with no direct test coverage added. Given this logic now gates indexer startup, we should add focused tests for at least: valid override, invalid/non-numeric override, and high override per chain.

bootstrap-worktree.sh

  • 🟢 Suggestion (line 18): pnpm --filter @mento-protocol/ui-dashboard test -- --run forwards a redundant --run to a script that is already vitest run. Simplifying to pnpm --filter @mento-protocol/ui-dashboard test avoids confusing arg forwarding.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

`[EventHandlers] FATAL: start block for chain ${chainId} is ${startBlock}, ` +
`but FPMMFactory first deployed at block ${firstDeployBlock}. ` +
`All factory deploy events will be missed and no pools will be indexed. ` +
`Lower ENVIO_START_BLOCK${chainId === 42220 ? "_CELO" : chainId === 143 ? "_MONAD" : ""} ` +
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remediation string is incorrect for Sepolia/Monad-testnet. For chainId values 11142220 and 10143, this currently advises lowering ENVIO_START_BLOCK, but this file reads ENVIO_START_BLOCK_CELO_SEPOLIA and ENVIO_START_BLOCK_MONAD_TESTNET.

Please derive the env var name from an explicit map (same shape as START_BLOCK_ENV) so the error message always points to the actual variable consumed by the check.

pnpm --filter @mento-protocol/indexer-envio typecheck

echo "🧪 Running tests..."
pnpm --filter @mento-protocol/ui-dashboard test -- --run
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ui-dashboard's test script is already vitest run, so passing -- --run is redundant and potentially confusing in command output. Consider simplifying this to:

pnpm --filter @mento-protocol/ui-dashboard test

- Extract assertStartBlocksValid() as an exported function so it can be
  unit tested independently of module load side effects
- Fix error message: was printing ENVIO_START_BLOCK_CELO/MONAD for all
  chains; now uses START_BLOCK_ENV_NAME map to print the correct per-chain
  var (ENVIO_START_BLOCK_CELO_SEPOLIA, ENVIO_START_BLOCK_MONAD_TESTNET)
- Add test/startBlockInvariant.test.ts with 9 cases covering:
  - Valid overrides (below / equal to first deploy block)
  - Missing / empty overrides (no throw)
  - Non-numeric override (graceful skip)
  - Per-chain fatal throw with correct env var name in message
  - Regression: Celo Sepolia dev start block (18901381) > Monad testnet
    first deploy block must NOT trigger Monad guard (cross-chain false fatal)
- bootstrap-worktree.sh: remove redundant -- --run flag (vitest already
  runs in run mode via the npm script)
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR moves mainnet indexing to dynamic contractRegister-based pool discovery, removes legacy per-chain configs/scripts, and adds startup start-block guards with tests. The direction is good, but there are a few correctness/regression issues that should be fixed before merge.

indexer-envio/src/EventHandlers.ts

  • 🟡 IMPORTANT (49-57, 70-74): assertStartBlocksValid() validates all four ENVIO_START_BLOCK_* vars unconditionally, regardless of which chains are active in the selected config. A stale/high override for an inactive chain will hard-fail codegen/dev/start for unrelated configs. Scope this check to active chain IDs for the chosen config (or otherwise gate inactive-chain vars).

indexer-envio/test/startBlockInvariant.test.ts

  • 🟡 IMPORTANT (81): The negative assertion is ineffective: assert(!err.message.includes("ENVIO_START_BLOCK_CELO ")) only checks for the string with a trailing newline. If the wrong var name appears without , this still passes. Check against "ENVIO_START_BLOCK_CELO" directly.

package.json

  • 🟡 IMPORTANT (10-13): Script renames remove indexer:celo-sepolia:* and indexer:monad-testnet:* with no compatibility aliases, but the repo still documents the old commands (for example README.md and indexer-envio/README.md). This creates immediate command failures for documented workflows. Either add temporary aliases or update docs in the same PR.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

…docs

Startup guard: scope assertStartBlocksValid to mainnet chains only (42220,
143). Testnet env vars (ENVIO_START_BLOCK_CELO_SEPOLIA, MONAD_TESTNET)
are excluded — a leftover testnet value in .env must not block a mainnet
or unrelated testnet run. Testnet configs are dev-only and not deployed
to hosted Envio production.

Tests: update to reflect mainnet-only scope; replace the ineffective
newline-suffix check with a clear 'does not throw for testnet env vars'
regression test.

Docs: update README.md and indexer-envio/README.md to replace all old
single-chain pnpm commands (indexer:celo-sepolia:*, indexer:monad-testnet:*)
with the new multichain testnet commands (indexer:testnet:codegen/dev).
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Reviewed this PR end-to-end against all changed files.

This PR replaces static pool address lists with dynamic contractRegister registration for FPMM/VirtualPool, adds a startup guard for unsafe start blocks, and consolidates indexer commands/config usage around multichain mainnet/testnet.

Files reviewed:

  • .github/workflows/indexer-envio.yml
  • AGENTS.md
  • README.md
  • bootstrap-worktree.sh
  • indexer-envio/AGENTS.md
  • indexer-envio/README.md
  • indexer-envio/config.celo.devnet.yaml (deleted)
  • indexer-envio/config.celo.mainnet.yaml (deleted)
  • indexer-envio/config.celo.sepolia.yaml (deleted)
  • indexer-envio/config.monad.mainnet.yaml (deleted)
  • indexer-envio/config.monad.testnet.yaml (deleted)
  • indexer-envio/config.multichain.mainnet.yaml
  • indexer-envio/src/EventHandlers.ts
  • indexer-envio/src/handlers/fpmm.ts
  • indexer-envio/src/handlers/virtualPool.ts
  • indexer-envio/test/startBlockInvariant.test.ts
  • package.json

Findings

indexer-envio/src/EventHandlers.ts

  • 🟡 Important (66-69): assertStartBlocksValid(...) runs unconditionally at module load for every config, including testnet runs. If a developer keeps ENVIO_START_BLOCK_CELO/ENVIO_START_BLOCK_MONAD set high for local experiments, pnpm indexer:testnet:dev can fail even though testnet config does not use those values. This is a non-happy-path startup failure due to missing config-awareness.

indexer-envio/src/handlers/fpmm.ts

  • 🟡 Important (105-108): New dynamic registration path (contractRegister + context.addFPMM(...)) has no automated coverage.

indexer-envio/src/handlers/virtualPool.ts

  • 🟡 Important (25-29): New dynamic registration path (contractRegister + context.addVirtualPool(...)) has no automated coverage.

indexer-envio/test/startBlockInvariant.test.ts

  • 🟡 Important (1-86): Added tests validate only the start-block guard; there is still no test proving the core behavioral change in this PR (dynamic contract registration with empty config address arrays). The main risk introduced by this PR remains unverified.

indexer-envio/README.md

  • 🟢 Suggestion (50-51): Configuration table still references deleted files (config.celo.sepolia.yaml, config.monad.testnet.yaml). Docs are now inconsistent with package.json scripts and current config layout.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

Startup guard (EventHandlers.ts):
- Change to warn-by-default, throw only when ENVIO_STRICT_START_BLOCK=true
  (set in CI/hosted runs). A leftover mainnet ENVIO_START_BLOCK_CELO in
  .env no longer hard-fails an unrelated testnet pnpm indexer:testnet:dev.
- The strict path is still available for hosted Envio production to enforce
  correctness at deploy time.

Tests (startBlockInvariant.test.ts):
- Add test for warn-mode (does not throw, console.warn called with message)
- Add test for strict-mode (throws with correct env var name)
- Separate Monad strict-mode throw test

New test file (dynamicRegistration.test.ts):
- Documents contractRegister testing limitation (framework harness does not
  expose processContractRegister — this is an Envio limitation)
- Adds two handler tests: FPMMDeployed.handler and
  VirtualPoolDeployed.handler each create a Pool entity, proving the
  handler path works for dynamically discovered pools

Docs: fix indexer-envio/README.md config table (remove deleted
config.celo.sepolia.yaml and config.monad.testnet.yaml entries)
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR migrates mainnet pool indexing from hardcoded FPMM/VirtualPool address lists to dynamic contractRegister hooks and consolidates local commands/config usage around multichain mainnet/testnet flows. The direction is correct, but there are blocking correctness gaps in the new safety model.

indexer-envio/src/EventHandlers.ts

  • 🔴 CRITICAL (indexer-envio/src/EventHandlers.ts:49, indexer-envio/src/EventHandlers.ts:76-79): the new start-block invariant is non-enforcing by default (strict defaults to false unless ENVIO_STRICT_START_BLOCK=true). With this PR, mainnet FPMM and VirtualPool are now dynamically registered from deploy events, so a too-high start block means pools are never registered and subsequent pool events are silently dropped.
  • Suggested fix: make strict mode default outside explicit local-dev flows, or pass strict: true in startup for hosted/CI paths.

.github/workflows/indexer-envio.yml

  • 🔴 CRITICAL (.github/workflows/indexer-envio.yml:56-75): CI never sets ENVIO_STRICT_START_BLOCK=true, so the new invariant is never validated in the same strict mode expected for hosted runs. This leaves the exact failure mode this PR is trying to prevent unguarded in automation.
  • Suggested fix example:
env:
  ENVIO_STRICT_START_BLOCK: "true"

(At least for codegen/typecheck/test steps that load EventHandlers.ts.)

indexer-envio/src/handlers/fpmm.ts

  • 🟡 IMPORTANT (indexer-envio/src/handlers/fpmm.ts:105-109): core runtime behavior moved to contractRegister (context.addFPMM(...)) but there is still no executable regression test for the registration path; if this hook regresses, indexing can silently stop for newly deployed pools.

indexer-envio/src/handlers/virtualPool.ts

  • 🟡 IMPORTANT (indexer-envio/src/handlers/virtualPool.ts:25-29): same coverage gap for context.addVirtualPool(...); this is critical path logic but currently not enforced by tests.

indexer-envio/test/dynamicRegistration.test.ts

  • 🟡 IMPORTANT (indexer-envio/test/dynamicRegistration.test.ts:74-76): this file documents the limitation but still only asserts handler behavior; it does not fail if contractRegister(...) calls are removed. Add at least one integration/smoke test (fixture chain or local dev index run) that proves events from a newly deployed pool are indexed after deployment event registration.

Final verdict: 🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

Startup checks (EventHandlers.ts → src/startupChecks.ts):
- Extract assertStartBlocksValid + constants into dedicated module
- Gate module-level call with NODE_ENV=test skip to prevent shell env vars
  from interfering with unrelated test suites
- EventHandlers.ts re-exports all startup check symbols for test backwards compat

CI (.github/workflows/indexer-envio.yml):
- Add ENVIO_STRICT_START_BLOCK=true to codegen validation step so the guard
  throws (not warns) in CI, catching start-block misconfig before deploy

virtualPool.ts:
- Fix misleading 'handle defensively' comment on VirtualPool.Rebalanced —
  the event is real when emitted; comment now explains why handler is simpler
  than FPMM version (no oracle/RPC fetch)

AGENTS.md + indexer-envio/AGENTS.md:
- Remove blank line artifact breaking ASCII file tree

config.multichain.testnet.yaml:
- Document the mixed static+dynamic address model; note that testnet pools
  deployed before contractRegister support was added may be missing on fresh
  sync (same pattern as mainnet, same fix path: add to hardcoded list)

config.multichain.mainnet.yaml:
- Confirm OpenLiquidityStrategy is deployed on Monad mainnet (verified via
  eth_getCode — bytecode present), remove precautionary 'verify before going
  live' comment

dynamicRegistration.test.ts:
- Document why the generated module type cast is unavoidable (generated/index.d.ts
  does not export clean standalone interface types)
…n removal

The core risk of this PR is that someone removes context.addFPMM() or
context.addVirtualPool() from the contractRegister callbacks — which would
silently break pool discovery with no test failure.

Added registry introspection tests that directly query the Envio handler
registry after importing EventHandlers.ts. If either contractRegister()
call is removed from fpmm.ts or virtualPool.ts, EventRegister.getContractRegister()
returns undefined and the test fails with a clear message.

Uses envio/src/EventRegister.res.js (internal Envio module) and
generated/src/Types.res.js to read the handler registers directly.
These are stable CommonJS internals that have been present across
multiple Envio versions — if they change, the test fails loudly.

Also clarifies the distinction between the two test suites:
- Registry introspection: catches removed contractRegister() calls
- Handler smoke tests: verifies handler() creates correct DB entities
  (TestHelpers.processEvent does not exercise contractRegister)
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

This PR migrates mainnet pool discovery to dynamic contractRegister hooks (FPMM + VirtualPool), removes single-chain configs, and adds startup guards/tests for start-block safety. The direction is correct, but I found two correctness/operability issues that should be fixed before merge.

Changed Files Checklist (Issue / No issue)

Findings

🟡 IMPORTANT

  1. indexer-envio/src/EventHandlers.ts + indexer-envio/src/startupChecks.ts
  • runStartupChecks() is executed at module load in EventHandlers.ts line 13.
  • The skip guard in startupChecks.ts line 76 only skips when NODE_ENV === "test".
  • Your Mocha tests import EventHandlers directly, but the test runner is not guaranteed to set NODE_ENV=test, so these startup checks can run during tests and produce environment-dependent warnings/failures.
  • Impact: test flakiness and non-determinism based on shell env vars (ENVIO_START_BLOCK_*, ENVIO_STRICT_START_BLOCK).
  • Suggested fix: make test mode explicit in the test script (or avoid module-load side effects in tests).
{
  "scripts": {
    "test": "NODE_ENV=test pnpm mocha"
  }
}
  1. indexer-envio/config.multichain.testnet.yaml
  • Comment at line 5 says per-chain overrides are ENVIO_RPC_URL_11142220 and ENVIO_RPC_URL_10143.
  • Actual config at line 86 uses ENVIO_RPC_URL_CELO_SEPOLIA, and Monad testnet has no rpc_config.url override at all.
  • Impact: operators can set the documented vars and silently get defaults instead (wrong/misleading operational behavior).
  • Suggested fix: align comments and config to one naming scheme, e.g. chain-id vars:
rpc_config:
  url: ${ENVIO_RPC_URL_11142220:-https://forno.celo-sepolia.celo-testnet.org}

(and add the analogous ENVIO_RPC_URL_10143 entry if that is the intended contract)

Final Verdict

🚫 Needs changes

Open in Web View Automation 

Sent by Cursor Automation: PR Review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed all changed files in this PR end-to-end (workflow/docs/scripts/configs/handlers/startup guard/tests/package scripts) against correctness, runtime safety, and test coverage.

I did not find any blocking or important issues introduced by these changes.

Final verdict: ✅ LGTM

Residual risk/testing gap: dynamic contractRegister hooks are framework-level Envio callbacks that are not directly unit-testable via processEvent, so runtime behavior still depends on integration behavior in Envio itself.

Open in Web View Automation 

Sent by Cursor Automation: PR Review

@chapati23 chapati23 merged commit 9daee67 into main Mar 31, 2026
7 checks passed
@chapati23 chapati23 deleted the fix/dynamic-contract-registration branch March 31, 2026 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants