fix: watch the active watched chain in the live tailer, independent of backfill adapter-sync mode#141
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
tayy-i
left a comment
There was a problem hiding this comment.
Consolidated review-gate result (fable live-oracle + agy code-logic + adjudication):
The diagnosis is verified correct end-to-end — on main, Auto/RawOnly map to ManifestDeclaredOnly, the live tailer watches 6 base / 16 mainnet manifest-declared addresses, and logs from all 3,842,759 discovery-admitted Base contracts are dropped unless they share a transaction with a declared emitter. Runtime widening is impossible under Auto on main. The concern-split (bootstrap scope vs live scope) is architecturally right, bootstrap semantics and job identities are untouched, and the PR correctly avoids re-arming the #135 per-tick manifest-sync scans. This fix is needed before live following resumes post-recovery — thank you for finding and framing it precisely.
But at this deployment's scale the current form arms three stalls (live-measured against the production DB):
-
CRITICAL — per-tick full watch-plan reload.
discovery_refresh_enabled: trueunconditionally, on the 5s poll cadence, re-runsload_watched_contractsover 24,104,359 active discovery edges every tick. A strict subset of that query (single branch, no UNION dedup, no ORDER BY, no transfer) measures 20.65s / 28M buffer accesses on the live DB; the real reload is minutes per tick, forever — the poll loop degenerates into back-to-back plan reloads. Needs a change-detection sentinel (edge/address count or max-updated watermark) before any full reload, and/or a refresh cadence decoupled frompoll_interval_secs. -
CRITICAL — the widen arms a ~3.84M-address sequential eth_getCode baseline storm. Only 141 base addresses have any raw_code_hashes row, so the first canonical tick post-widen attempts ~3.84M sequential per-address getCode round-trips (3-11h inside one tick), upserts only after ALL succeed, and any single failure retries the entire storm from zero next tick — detonating exactly when checkpoint promotion first hands off to live following. Needs chunked, progress-persisting baseline via the batched provider path, a per-tick cap, or #133-style activity scoping for the live baseline.
-
HIGH — steady-state per-tick cost: three ~3.84M-element TEXT[] binds per chain per head-changed tick (~160MB parameter serialization each,
= ANYlinear probes, a no-op 3.84M-row UNNEST anti-join) plus a from-scratch lowercased BTreeSet rebuild of the watch set per tick. Hoist the set into the intake task at build time; join the plan tables server-side instead of binding the watch set back into Postgres.
Plus one MEDIUM: the widen double-loads the full plan (summary + plan are pure functions over one load_watched_contracts result).
agy's independent pass: APPROVE on the logic — no correctness defects found in the split itself; the divergence is entirely in live-scale dynamics, which is where the three findings above live.
Merge-order recommendation (also for the queue's awareness): #144 → #138 → the #125 promotion rework → this PR after remediation. Landing this as-is would convert the first post-promotion live-follow attempt into a multi-hour retry loop — the same silent-stall class this recovery exists to eliminate.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a7326bce8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c097590ca9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dd3309523
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Remediation of the three CHANGES_REQUESTED scale findings is pushed (previous head preserved at 0b13a37 for reference; the branch was rebased onto current main as a three-way replay because the old tip's merge commit carried resolution-only content a plain rebase would have dropped).
Gates: fmt/clippy clean, size ratchet OK, indexer 484/484, manifests 104/104 (an earlier 13-failure run was self-inflicted pool contention from four concurrent suites; serial rerun fully green). Review pipeline next: codex + independent fable + Gemini passes on the new head. @codex review |
…gated plan reload, capped code-hash baseline sweep, no whole-surface binds) Remediates the three scale findings from the CHANGES_REQUESTED review on #141, measured against ~3.84M watched Base targets / 24.1M active discovery edges. 1. CRITICAL - per-tick full watch-plan reload. The always-on discovery refresh re-ran the full watched-surface scan (load_watched_contracts, minutes per pass at Base scale) on every poll tick. The reload is now gated on the ratified per-chain discovery_admission_epochs sentinel: every transaction that mutates the watched surface already bumps the owning chain's epoch in the same transaction, so an unchanged epoch map proves the stored plan has not moved and a quiet tick costs one read of a tiny table (refresh_runtime_state_from_stored_discovery_when_epochs_move; sentinel state held by run_poll_loop; the one-shot replay-handoff refresh passes a None sentinel so it always reloads). This makes the refresh cadence question moot: the per-tick check is O(#chains). 2. CRITICAL - post-widen eth_getCode baseline storm. The baseline pass built the full watched-minus-stored set in one query, fetched code sequentially per address inside one tick, and upserted only after every fetch succeeded, so the first canonical tick after the widen armed ~3.84M serial provider round-trips that retried from zero on any failure. The baseline is now a capped cursor sweep: at most BIGNAME_INDEXER_RAW_CODE_BASELINE_MAX_ADDRESSES_PER_TICK addresses (default 2048) are verified per tick behind a process-lifetime per-chain frontier (ChainCoverageFrontiers::raw_code_baseline_frontier), only genuinely missing addresses are fetched via the batched provider path (fetch_code_observations_at_block_hashes), and observations are upserted per 256-address chunk so progress persists across failures and restarts. A finished sweep records the admission epoch it started under; an epoch move starts a fresh sweep so newly watched addresses are eventually baselined without ever re-arming a whole-surface fetch. 3. HIGH - steady-state per-tick cost. Removed all three whole-watch-set TEXT[] binds in the code-hash path: the emitter query no longer binds the watch set (it returns the candidate blocks' bounded distinct emitters plus a topic0-selected flag, filtered client-side); the stored-code query binds only the filtered emitters; the baseline anti-join binds at most one capped batch. The per-tick lowercased BTreeSet rebuild of the watch set is gone: SelectedAddressSet borrows the plan's already sorted, deduplicated, lowercase address vector and probes it with case-insensitive binary search (persist_reconciled_raw_payloads and the code-hash path share it). Also fixes the MEDIUM: summary and plan are pure functions over one load_watched_contracts result, so the widen, the stored-discovery refresh, and the ActiveWatchedChain state build now use a single-scan load_watched_contract_summary_and_chain_plan instead of two full scans. Tests: sentinel gating (stored_discovery_refresh_reloads_only_when_admission_ epochs_move), baseline progress persistence + epoch resweep (raw_code_baseline _sweep_persists_progress_and_resweeps_on_admission_epoch_move), cap parser, SelectedAddressSet probes; docs updated to describe the capped sweep and the sentinel-gated reload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
9dd3309 to
45617a5
Compare
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45617a570c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
tayy-i
left a comment
There was a problem hiding this comment.
Verdict: REQUEST_CHANGES — re-review of remediated head 45617a5 (578946f + 45617a5 on ae2e8b9). Consolidated: my own verification + fable adversarial pass + Gemini (agy) pass + codex bot (which did land on this head at 03:23Z; its 3 P2s are all valid and folded in). CI on this head is red (static + e2e).
The remediation's mechanisms are well built — the epoch sentinel, the capped cursor sweep, and the client-side filter are each individually correct — but the poll-loop wiring defeats the sentinel in the one mode this deployment runs, and CI is red.
Verified correct — do not re-litigate
- Sentinel mechanism (
runtime/refresh.rs:96-114): read ordering is race-safe in the safe direction (epochs read before the plan; sentinel advances only to the pre-read snapshot; a concurrent bump costs at worst one redundant reload, never a missed one).Ok(None)sentinel update is correct. Mutation-site enumeration re-done (#173-style) across the four plan-feeding tables: all production writers bump in-transaction (sync.rs:214,discovery/persistence.rs:158,reconciliation/full.rs:181,streamed.rs:432,scoped.rs:74,managed_edges/source_graph.rs:390,431;bulk.rswriters only reachable through those; no writers outsidecrates/manifests). Two latent invariant nicks that could not be made plan-visible are in the notes (N7). - Sweep cursor semantics (
code_hashes.rs): missing-observations SQLORDER BY+ sorted 256-chunks +partition_pointresume proves every missing address ≤ the stored cursor was fetched and everything above is re-examined; per-chunk upsert-then-advance survives mid-batch failure; errors contained per-chain (canonical/poll.rs:83-92, warn + retry). Frontier is process-lifetime in-memory by design; restart loses the cursor, not progress. Nothing gates live following, promotion, or the handoff on baseline completion (promotion's companion gate is per-promoted-path,stored_lineage/coverage/companions.rs; handoff latch is cursor-based). Full pass at defaults ≈ 3.84M/2048 per 5s tick ≈ 2.6h — acceptable since nothing blocks on it. - Filter equivalence (
code_hashes.rs:301-337+payload.rs):BOOL_OR(topic0)+ clienttopic0_selected || contains(address)selects exactly the old SQL OR-filter's (block, emitter) set, including NULLtopics[1](Option<bool>→unwrap_or(false), matching NULL-excludes semantics) and empty arrays.SelectedAddressSetpreconditions hold in release builds: plan addresses areLOWER()-ed in SQL, accumulated through aBTreeSet, copied verbatim intoIntakeChainTask.addresses; production tasks are built only bysync_intake_chain_tasks. Both call sites taketask.addresses. - Combined summary+plan loader trivially equivalent; dropped
resync_adapter_owned_stateflag lost nothing (inline-only re-derivation preserved;run_poll_loopargument order checked positionally); pre-catchup adapter sync is mechanically idempotent-safe vs the catch-up task (nonormalized_replay_cursorsinterference; conflict-safe upserts; strictly serialized before the spawn) — the B2 blocker below is about cost/scope, not corruption. - Local suites green (indexer 482 passed / 3 ignored; manifests 104/104; serial via scripts/test-db). e2e-workspace clippy clean; the strict-clippy failure is exactly one lint (B1).
Blockers
B0 (defect, high confidence — the headline finding): the per-tick full-reload stall is NOT fixed in auto+catchup, the mode this deployment runs. replay_handoff_required is computed from mode-constant flags (poll_loop.rs:381-384; in auto+catchup adapter_sync_on_live_poll=false, after_catchup=true for process lifetime), so renew_live_poll_adapter_sync_permit runs every tick forever — by design ("a one-poll permit… every later poll renews the full proof", replay_handoff.rs:191-194). Inside it, the refresh at replay_handoff.rs:247-259 passes &mut None, and a None sentinel always reloads (refresh.rs:106). Net: every steady-state tick performs the full watch-plan reload the PR itself prices at "minutes per pass" at Base scale (refresh.rs:90-93), plus the per-tick backlog sync. The comment calls this "a one-shot transition"; it is not. The epoch-gated path at poll_loop.rs:470-479 — the one carrying the real sentinel — is only reached after renew returns true, so in this mode the gated refresh runs in addition to the ungated one. Additionally, mid-catch-up with some-but-not-all chains complete, replay_handoff.rs:127 (poll_replay_ready_chains_raw_only) is a second un-gated per-tick full reload (pre-existing on main, but this deployment will sit in that window — mainnet's cursors complete long before Base's). Fix shape: thread the loop's watched_plan_admission_epochs sentinel through renew_live_poll_adapter_sync_permit into both sites (a forced reload is defensible only on the first latch attempt, if at all). The new docs/deployment.md claim ("the plan reload is gated on the sentinel… one tiny read per tick") is false for this mode until this is fixed. Credit: fable pass; hand-verified against poll_loop.rs:381-404 + replay_handoff.rs:243-262.
B1 (defect, high confidence): CI is red on this head.
static:fetch_code_observations_at_blockis dead code onRethDbProvider(apps/indexer/src/provider/reth_db/api.rs:159) — the#[allow(dead_code)]sweep covered the trait, JSON-RPC impl, andreth_db/unavailable.rsbut missed the feature-gatedreth_db/api.rsimpl (only compiled under--all-features). Reproduced locally; with-A dead_codethe strict step is otherwise clean, so this one allow clears it.test (e2e): 3 failures (rich_chain_live_reorg_converges_to_winning_branch,records_route_values_and_version_boundaries_follow_current_resolver,resolver_changes_follow_registry_and_zero_releases), one signature: the indexer goes silent mid-tick (last log is a completed handoff/discovery-refresh; the next per-chain head reconcile never logs) until the 600s readiness deadline SIGKILLs it. Main is green today on both ae2e8b9 and d2b6447. The branch base predates d2b6447 (#175, JSON-RPC pool recovery after transport timeouts), whose failure mode matches this hang, and this PR adds per-tick getCode traffic that raises exposure — but that is a hypothesis, not a verdict: rebase onto current main, re-run, and if e2e still fails, root-cause the hang as PR-caused. (Not locally reproducible: no foundry/anvil on this server.)
B2 (defect, high confidence on mechanics): unbounded synchronous startup derivation in auto+catchup (run_mode.rs:60-62 sync_adapter_after_startup_backfill = inline || normalized_replay_catchup_enabled; runs at run.rs:81-90 before the catch-up spawn). sync_adapter_owned_raw_log_state is one unbounded, unchunked pass per adapter family (7 families) over the whole retained corpus with in-memory accumulation. On a fresh chain or a mid-catch-up restart (the normal state mid-cutover), startup synchronously derives the entire remaining corpus — the work the catch-up exists to do in bounded, paced, recoverable chunks — before polling starts; even on a fully-derived DB, every restart pays 7 whole-corpus scans. Adjudication across lenses: fable verified this is idempotent-safe (no cursor interference, conflict-safe upserts, serialized) — so codex/Gemini's "ownership violation" framing overstates corruption risk — but the cost class alone blocks: the stated goal ("bootstrap discoveries enter the live plan before polling") needs only the discovery-materializing families, not all 7, and needs bounding. Narrow the scope and/or bound it. (codex P1 thread "Avoid full adapter sync before replay catch-up" resolves with this.)
B3 (defect, medium-high confidence): sweep epoch/plan coherence race (code_hashes.rs:217-229). The sweep pins a fresh DB epoch while walking the in-memory plan loaded under an older epoch (plan refresh runs at end-of-tick, after the poll phase that runs the sweep). A sweep that starts in the window after a mutation's bump but before the plan reload records completed_admission_epoch for a surface it never walked; the newly admitted address is skipped until the next epoch bump — permanent on a chain that goes quiet. Fix: pin the epoch snapshot the plan was loaded under (the poll loop already holds it: poll_loop.rs:64-66). A sweep already in progress at bump time is safe (pinned older epoch forces a re-sweep). Independently found by Gemini and fable; hand-verified.
B4 (defect, high confidence): sentinel advanced past an un-applied plan (codex P2, refresh.rs:111; hand-verified). The wrapper advances the sentinel on refreshed.is_ok() including Ok(Some(new plan)) (refresh.rs:110-112), but discovery_refresh.rs:64-72 then drops that plan when the resolver-profile drain fails (returns Ok(false) without installing). Later ticks see an unmoved epoch and skip the reload: stale intake tasks until an unrelated epoch move or restart — the exact stale-plan failure the sentinel exists to prevent, reachable by one transient DB error. Advance the sentinel only after drain + install succeed.
B5 (defect/perf, high confidence on the missing index): the sweep anti-join is O(table) per tick. NOT EXISTS (… LOWER(raw_code_hashes.contract_address) = watched.contract_address …) (code_hashes.rs:414-426) has no usable index — only (chain_id, contract_address, block_number DESC) and (chain_id, canonicality_state, …) exist (migrations/20260430060000_baseline.sql:1858,1865) and LOWER() defeats both. Realistic plan: hash anti-join over a full scan of the chain's raw_code_hashes (3.84M+ rows once baselined) — once per tick, ~1,875 consecutive ticks per pass, and a fresh pass after every epoch bump (frequent on live Base) and every restart. A per-tick table scan re-introduced by the mechanism built to avoid per-tick scans. Fix: (chain_id, LOWER(contract_address)) expression index (values are lowercase in practice but the schema doesn't guarantee it, so keep the LOWER()).
Blocker-or-fast-follow (user adjudication; recommendation: fix in this PR — all small)
F2 — runtime manifest reload now writes in non-inline modes (codex P2 thread, run.rs:244, tracking this head). run_poll_loop gets ActiveWatchedChain in all modes, so a manifest-repo change routes through sync_manifest_normalized_events (runtime/manifest.rs:99-110): normalized-event writes from a raw-only tailer (contract drift), plus a full plan scan and two normalized_events count-by-kind scans per repo change, landing mid-catch-up if an operator edits manifests. Fable verified the catch-up race is serialized by the epoch-fence FOR UPDATE rows (contention, not corruption) — but it contradicts the widen's own race rationale (refresh.rs:116-119). Give this path the widen's read-only reload, or gate the sync on broad_runtime_refresh_enabled.
F3 — raw-only now drains resolver-profile writes per tick (codex P2 on this head, run_mode.rs:66; hand-verified). discovery_refresh_enabled: true unconditionally means raw-only runs refresh_discovery_watch_state every tick, which always calls drain_resolver_profile_input_changes (discovery_refresh.rs:64-66) — resolver-profile normalized-event writes + projection invalidations in the mode documented as "no live adapter sync (a manual deferral)". On main, raw-only never reached this function. Split the plan reload from resolver-profile convergence, or gate the drain.
F4 — baseline completion is blind to reorg orphaning (codex P2 on this head, code_hashes.rs:220; premise verified — raw_children/orphaning.rs:36 lists raw_code_hashes). Reorgs orphan baseline rows without an epoch bump; a completed sweep keyed solely to the admission epoch never refetches. Shares a fix shape with B3 (key completion/invalidation to raw-code coverage, or invalidate the frontier on raw-code orphaning); recommend fixing together.
Non-blocking notes (ledger items)
- N1: First tick after boot pays one redundant full reload (loop sentinel starts
None); seed it from an epoch read taken before the widen's plan load. (The post-handoff duplicate folds into B0.) - N2: Baseline observations land at heterogeneous advancing canonical blocks — fine for baseline semantics; note only.
- N3: codex P2 (poll_loop.rs:476 thread) — intra-gap discovery loss is the pre-existing poll→refresh ordering on main, materially narrowed by this PR. Ledger item, disposition on thread.
- N4: codex P1 (run_mode.rs:66 thread) — during catch-up, live-observed discovery logs don't create edges (reload-only refresh; live adapter sync deferred), so a mid-catch-up live discovery stays unwatched until replay reaches it and its interim events are not raw-persisted. Real, inherited, bounded by the catch-up window; recommend accept-as-known-window + ledger, but the thread needs explicit disposition.
- N5: auto-WITHOUT-catchup never derives bootstrap-range discovery edges at all (
sync_adapter_after_startup_backfill=false; live sync is block-hash-scoped —reconciliation/adapter_sync/scope.rs:31-60), while the PR advertises the active-watched-chain scope in every mode; and the justifying comment intests/run_mode.rs("re-derives edges each poll") is factually wrong — fix the comment, and either document the mode's discovery gap or extend B2's narrowed seeding to it (codex P2 thread closes with this). - N6: per-tick O(surface) memory churn in all modes now that the plan is always wide:
refresh_intake_chain_tasksclones + compares the full 3.84M-address plan every tick (runtime/intake.rs:216-224,refresh.rs:40-51) andpoll_provider_heads_with_adapter_syncclones all tasks (canonical/poll.rs:54) — hundreds of MB of allocation churn per tick at Base scale. Fast-follow candidate (Arc the address vectors or compare cheap fingerprints). - N7: two latent epoch-invariant nicks (fable; could not be made plan-visible): (a)
discovery/persistence.rs:60-72can re-seed an activecontract_instance_addressesrow while the bump is gated oninserted_edge_count > 0(persistence.rs:156-159); (b)full.rs:176/streamed.rs:424discard the CIA reconcile's mutated chains (manifest sync correctly uses_with_mutations). Bump on seed-insert and use_with_mutationseverywhere. - N8: test gaps to close with the fix commit: multi-batch sweep (cap + cursor walk + empty-batch completion), sweep failure-resume with a failing provider, mid-sweep plan-swap (B3's interleaving), a loop-level assertion that a quiet tick issues no plan query (would have caught B0), renew-path gating, DB-level NULL-topics equivalence for the
BOOL_ORquery. - N9: docs to update with the fixes: deployment.md's sentinel-gating claim (false in auto+catchup until B0), and the B2 mode paragraph when its mechanism changes.
codex triage (head 45617a5)
The 03:10Z request errored, but a retry landed at 03:23:32Z on this exact head with 3 P2s — all hand-verified valid: refresh.rs:111 → B4; run_mode.rs:66 (raw-only drain) → F3; code_hashes.rs:220 (orphaned baselines) → F4. Prior-head threads still tracking this head unresolved: P1 run_mode.rs:66 → N4; P2 poll_loop.rs:476 → N3; P2 run.rs:244 → F2. Outdated-anchored but live in code: P1 "avoid full adapter sync before replay catch-up" → B2; P2 "post-bootstrap sync for auto without catch-up" → N5. The two 07-10 doc findings are addressed by the rebased docs. Gate: not met — multiple P1/P2 undispositioned; threads await author replies + reviewer resolution per lifecycle.
Panel attribution and false positives
fable pass: found B0 (the headline), B3 (independently), B5, N5-N8, and the Q5 idempotency adjudication that softened B2's corruption framing. Gemini pass: found B3 first, confirmed sweep-resume proof and sentinel race-safety; its "NULL decode crash" claim is a false positive (the code decodes Option<bool> with unwrap_or(false), code_hashes.rs:334-337) — an artifact of the inline packet abstraction, verified against the real code. codex: B4/F3/F4 on this head, B2/N3/N4/N5/F2 on prior heads. Every adopted finding was hand-verified against the worktree.
…f backfill adapter-sync mode Rebase of fix/prod-intake-frozen onto main (post-#151/#153/#173), squashing the branch's two commits and its upstream merge: - Split `runtime_watch_scope` into `bootstrap_watch_scope` (keeps the adapter-sync-mode mapping; bounds bootstrap provider cost) and `live_watch_scope` (always ActiveWatchedChain; live intake fetches whole blocks and filters client-side, so a wide scope adds no extra log fetches). - `discovery_refresh_enabled` is always on so targets admitted after startup enter the watch set without a restart; the whole-corpus re-derivation stays opt-in for broad runtime refresh (inline), now expressed through main's `sync_adapter_state_before_refresh` dispatch in refresh_discovery_watch_state. - Widen the plan to live scope after bootstrap drains and before the replay catch-up task spawns (both reconcile contract_instance_addresses). The widen replaces main's post-bootstrap stored-discovery refresh as its superset: it reloads unconditionally, for every adapter-sync mode including raw-only. - Seed bootstrap-discovered targets into the live plan: run the one-shot post-bootstrap adapter-owned sync when replay catch-up owns live sync (auto+catchup), merged with main's inline post-bootstrap sync as `inline || normalized_replay_catchup_enabled`. Conflict-resolution notes: main's #151 independently added a post-bootstrap stored-discovery refresh and the storage/stored refresh split; the PR's resync_adapter_owned_state flag was dropped in favor of main's two-function dispatch, and main's fresh-chain rule (no adapter sync before bootstrap) wins over the PR's pre-#151 inline-before-bootstrap flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
…gated plan reload, capped code-hash baseline sweep, no whole-surface binds) Remediates the three scale findings from the CHANGES_REQUESTED review on #141, measured against ~3.84M watched Base targets / 24.1M active discovery edges. 1. CRITICAL - per-tick full watch-plan reload. The always-on discovery refresh re-ran the full watched-surface scan (load_watched_contracts, minutes per pass at Base scale) on every poll tick. The reload is now gated on the ratified per-chain discovery_admission_epochs sentinel: every transaction that mutates the watched surface already bumps the owning chain's epoch in the same transaction, so an unchanged epoch map proves the stored plan has not moved and a quiet tick costs one read of a tiny table (refresh_runtime_state_from_stored_discovery_when_epochs_move; sentinel state held by run_poll_loop; the one-shot replay-handoff refresh passes a None sentinel so it always reloads). This makes the refresh cadence question moot: the per-tick check is O(#chains). 2. CRITICAL - post-widen eth_getCode baseline storm. The baseline pass built the full watched-minus-stored set in one query, fetched code sequentially per address inside one tick, and upserted only after every fetch succeeded, so the first canonical tick after the widen armed ~3.84M serial provider round-trips that retried from zero on any failure. The baseline is now a capped cursor sweep: at most BIGNAME_INDEXER_RAW_CODE_BASELINE_MAX_ADDRESSES_PER_TICK addresses (default 2048) are verified per tick behind a process-lifetime per-chain frontier (ChainCoverageFrontiers::raw_code_baseline_frontier), only genuinely missing addresses are fetched via the batched provider path (fetch_code_observations_at_block_hashes), and observations are upserted per 256-address chunk so progress persists across failures and restarts. A finished sweep records the admission epoch it started under; an epoch move starts a fresh sweep so newly watched addresses are eventually baselined without ever re-arming a whole-surface fetch. 3. HIGH - steady-state per-tick cost. Removed all three whole-watch-set TEXT[] binds in the code-hash path: the emitter query no longer binds the watch set (it returns the candidate blocks' bounded distinct emitters plus a topic0-selected flag, filtered client-side); the stored-code query binds only the filtered emitters; the baseline anti-join binds at most one capped batch. The per-tick lowercased BTreeSet rebuild of the watch set is gone: SelectedAddressSet borrows the plan's already sorted, deduplicated, lowercase address vector and probes it with case-insensitive binary search (persist_reconciled_raw_payloads and the code-hash path share it). Also fixes the MEDIUM: summary and plan are pure functions over one load_watched_contracts result, so the widen, the stored-discovery refresh, and the ActiveWatchedChain state build now use a single-scan load_watched_contract_summary_and_chain_plan instead of two full scans. Tests: sentinel gating (stored_discovery_refresh_reloads_only_when_admission_ epochs_move), baseline progress persistence + epoch resweep (raw_code_baseline _sweep_persists_progress_and_resweeps_on_admission_epoch_move), cap parser, SelectedAddressSet probes; docs updated to describe the capped sweep and the sentinel-gated reload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Keep the single-block Reth helper alongside the batched provider API without tripping the all-features dead-code sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Gate every catch-up and steady-state plan reload on the loaded admission epoch, force the handoff reload once, and commit sentinels only after state application. Scope auto startup and non-inline manifest work, keep raw-only convergence write-free, pin baselines to loaded plans, invalidate them after reorg orphaning, and batch code fetches across candidate blocks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Create the non-orphaned lower-address expression index concurrently and include it in migration-time invalid-index recovery and plan coverage tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Keep resolver-profile input work pending during partial replay and the pre-latch refresh, then let the normal live refresh drain it only after ownership is restored. Clarify the accepted process-local baseline cursor and clean up stale intake terminology. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Pass the documented per-tick baseline limit through the server Compose environment and show its default in the example configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
Define the raw-code baseline term, link its first uses, and state that the configured address budget applies independently to each chain on a poll tick. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
|
Round-2 remediation pushed (45617a5 → 6aba236, rebased onto current main incl. #175): B0 sentinel threading through both replay-handoff reload sites with a genuine one-shot transition; B1 dead-code allow + e2e green post-rebase; B2 startup sync narrowed and deferred to handoff; B3 sweep pins the plan's load epoch; B4 sentinel advances only on applied plans; B5 expression index (CONCURRENTLY); F2-F4 and the G-items addressed; run_mode test comment fixed; new 'Raw-code baseline' glossary entry for the coinage. All 11 codex threads have reply dispositions with fixing SHAs (left unresolved pending verification). Verification pass + CI next; threads resolve after both. @codex review |
45617a5 to
6aba236
Compare
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Round-2 remediation (head 6aba236) addressed all findings from this review; verified CLOSE and all threads dispositioned/resolved.
Superseded by round-2 (head 6aba236); all B0-B5/F2-F4 findings verified CLOSE and threads resolved.
tayy-i
left a comment
There was a problem hiding this comment.
Round-2 remediation verified: all B0-B5 blockers CLOSE (independent verification with named code-path evidence), all 4 latest-head codex findings adjudicated non-blocking (P1 is intended fenced-handoff design; two CLOSED by round-2; one pre-existing on main, narrowed here — follow-up filed), Gemini + fable lenses clean, CI 10/10. Live-following gate cleared.
#141 (raw-code baseline index) and #177 (journal normalization) both landed a 20260720120000 migration; sqlx keys _sqlx_migrations by version, so applying both fails with a duplicate-key violation — and the partial run records the INDEX migration's checksum at that version. Keep the index file at 20260720120000 (matching the recorded ledger row) and move the journal normalization to 20260720122000, updating the two include_str! test paths. Fresh databases apply index -> staging -> normalize in order; databases that hit the partial run resume cleanly at 121000. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p
#141 (raw-code baseline index) and #177 (journal normalization) both landed a 20260720120000 migration; sqlx keys _sqlx_migrations by version, so applying both fails with a duplicate-key violation — and the partial run records the INDEX migration's checksum at that version. Keep the index file at 20260720120000 (matching the recorded ledger row) and move the journal normalization to 20260720122000, updating the two include_str! test paths. Fresh databases apply index -> staging -> normalize in order; databases that hit the partial run resume cleanly at 121000. Claude-Session: https://claude.ai/code/session_01C9RTnbCyhqEUSPc7xCK76p Co-authored-by: TateB <yo@taytems.xyz> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
IndexerRunModederived a single watch scope fromadapter_sync_mode.auto(the default) andraw-onlymapped toManifestDeclaredOnlyand leftdiscovery_refresh_enabledoff, so the live tailer watched only manifest-declared contracts. A log from a discovery-admitted resolver or subregistry survived only if it shared a transaction with a manifest-declared emitter.This splits the one knob into the two questions it was conflating:
bootstrap_watch_scope— unchangedadapter_sync_modemapping. Bootstrap backfill resolves each selected target into an address-filtered range scan, so a narrow target set is a real provider-cost control.live_watch_scope— alwaysActiveWatchedChain. Live intake fetches every log in a block by block hash (provider/logs_receipts/exact.rs) and filters client-side (reconciliation/payload.rs), so a wide scope costs no additional provider calls. Narrowing it only drops facts.Discovery refresh is now always on, so targets admitted after startup enter the watch set without a restart. Only broad runtime refresh (
inline) still re-derives discovery edges from the whole stored raw-log corpus on a refresh tick; other modes reload the plan from edges already in storage, which live adapter sync writes per block.The widen is a stored-plan reload (
widen_runtime_state_to_live_watch_scope) rather than a manifest re-sync, and it runs before the replay catch-up task spawns. Bothsync_repositoryand catch-up's discovery persistence reconcilecontract_instance_addresses; doing the widen as a plain reload, before the spawn, keeps them off each other.Storage
Live code observation is activity-scoped and unchanged by this PR: a block admits
raw_code_hashesrows for the watched addresses that emitted in it, plus a one-time baseline for a watched address with no stored non-orphaned observation (load_missing_raw_code_baseline_addressesis aNOT EXISTSover all blocks, applied at the canonical head). Widening the live plan therefore adds one baseline row per previously-unwatched target, once — not one per block.Bootstrap scope is deliberately left narrow here. #133 scopes backfill code observations to a block's selected log emitters; a wide bootstrap scope should wait for it.
Tests
cargo test -p bigname-indexer --bin bigname-indexer— 308 pass.live_tailer_watches_the_active_watched_chain_in_every_adapter_sync_modebootstrap_watch_scope_stays_narrow_while_the_live_tailer_stays_wideonly_broad_runtime_refresh_resyncs_adapter_owned_state_on_discovery_refreshwiden_runtime_state_to_live_watch_scope_admits_discovered_targets_without_manifest_resynclive_watch_scope_refresh_does_not_rederive_discovery_edges_from_raw_logsDocs
docs/chain-intake.mdgains a "Live watch scope" section;docs/deployment.mdstates that adapter-sync mode scopes bootstrap backfill only. Both record that live coverage is never narrowed, and that a narrow bootstrap scope does not imply a narrow live scope.Intake coverage only. No public route, route-level coverage, manifest capability flag, ENSv2 profile support, or consumer-replacement change.