feat(protocols): Add code to parse and process ContractData entires for Blend processors#657
feat(protocols): Add code to parse and process ContractData entires for Blend processors#657aditya1702 wants to merge 10 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4233173ee
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Introduces a new optional “framework seam” for protocol processors to consume ContractData ledger-entry changes, gated behind a RequiresContractData() capability so event-only protocols avoid the heavier extraction path. This supports upcoming Blend Capital v2 lending ingestion by making ContractData deltas available to processors without forcing extraction for all protocols.
Changes:
- Extends
ProtocolProcessorwithRequiresContractData() booland extendsProtocolProcessorInputwithContractDataChanges map[string][]ingest.Change. - Adds
indexer.ExtractContractDataChangesForLedgerand wires it into both migration (processAllProtocols) and live ingestion (lazy/memoized per ledger). - Adds/updates mocks and tests to validate the capability gate and fixture-based correctness of the extractor.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/services/sep41/processor.go | Implements RequiresContractData() as false for SEP-41 (event-only). |
| internal/services/protocol_processor.go | Adds the capability gate method and ContractDataChanges to processor input. |
| internal/services/protocol_migrate.go | Extracts ContractData changes (conditionally) and passes them into processors during migration. |
| internal/services/protocol_migrate_test.go | Updates test processors/mocks and adds migration tests for ContractDataChanges gating. |
| internal/services/processor_registry_test.go | Adds a mock-based test covering the new interface method. |
| internal/services/mocks.go | Extends ProtocolProcessorMock with RequiresContractData(). |
| internal/services/ingest_test.go | Adds live-ingestion tests for nil/non-nil ContractDataChanges based on the gate. |
| internal/services/ingest_live.go | Lazily extracts and memoizes ContractData changes per ledger when required by a CAS-winning processor. |
| internal/indexer/indexer.go | Adds ExtractContractDataChangesForLedger grouped by owning contract C-address. |
| internal/indexer/indexer_test.go | Adds fixture-corpus test validating extraction correctness and grouping invariants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… behind the capability gate
…d the capability gate
…sors Live ingestion derived ProtocolContracts from this ledger's event emitters only. A ContractData-requiring processor needs the protocol's complete committed membership: entries can change on contracts that emitted no event this ledger, and event decoding disambiguates shared symbols against the full tracked set (e.g. backstop vs pool withdraw).
…data extract metric phase
A processor enriching protocol_contracts (contract names decoded from instance storage) inserts rows FK-filtered against protocol_wasms; a contract deployed in the same ledger as its wasm upload was silently dropped because the wasm rows persisted after the processor block.
…ta processors The migration engine loaded each tracker's classified-contract membership once before the unbounded ledger loop. A contract classified while the run is in flight (live ingestion's validator classifies newly deployed contracts concurrently) never reached membership-driven processors: for BLEND, a pool deployed mid-run lost its history rows permanently and mis-resolved the pool-vs-backstop withdraw disambiguation for the rest of the run. Trackers whose processor requires ContractData now re-read membership via GetByProtocolID after every committed window (both in-loop and at-tip flushes), mirroring the live path's per-ledger full-membership resolution. Event-only processors keep the run-start snapshot.
325123d to
a5acea9
Compare
…extraction Live ingestion materialized every ledger's transactions twice: once in the main indexing pass and again inside ExtractContractDataChangesForLedger, whose reader constructor re-hashes every transaction envelope. ProcessLedger now returns the transactions it already built, and the live path hands them to a new ExtractContractDataChangesFromTransactions, so a ledger is read exactly once. The ledger-based extractor remains as a thin wrapper for protocol-migrate, which has no prior transaction pass to share.
protocol-migrate spent ~7ms/ledger in extract_contract_data — a LedgerTransactionReader build (SHA-256 of every envelope) plus a full GetChanges walk of every transaction — on every ledger of the range, though almost none touch a tracked contract. Soroban guarantees writes ⊆ the declared read-write footprint (host storage is footprint-seeded and writes outside it trap; RestoreFootprint restores exactly the read-write keys; protocol-23 auto-restores are indices into it), so a skim of the already-decoded envelopes' footprints decides ledger relevance exactly. Ledgers whose footprints touch no tracked contract skip extraction outright; the rare hit extracts via transaction meta directly — GetChanges reads only meta/result/ledger-version, so no reader is built even then. The fixture-corpus test now pins both properties on real ledgers: meta-based output equals the reader-based reference, and every changed contract, tracked alone, triggers the gate. Live ingestion keeps the ungated slice-based path over its already-materialized transactions.
aditya1702
changed the title
| if processor.RequiresContractData() { | ||
| if !contractDataExtracted { | ||
| var cdErr error | ||
| contractDataChanges, cdErr = indexer.ExtractContractDataChangesFromTransactions(transactions, ledgerSeq) |
There was a problem hiding this comment.
Should we hoist this to ingestProcessedDataWithRetry (or ingestLiveLedgers) and cache the result across retry attempts, instead of recomputing it inside each one?
The extraction only depends on transactions, which doesn't change between attempts, so retries redo the same work inside an open transaction, and only when the DB is already struggling.
| // only in another contract's topics. Protocols requiring contract | ||
| // data have bounded membership, so the per-ledger query stays cheap. | ||
| var fullErr error | ||
| committed, fullErr = m.models.ProtocolContracts.GetByProtocolID(ctx, protocolID) |
There was a problem hiding this comment.
Should GetByProtocolID run on dbTx instead of the pool? Seems like a cheap swap since the connection is already in hand.
Right now it's called inside the open persist transaction but executes on m.DB, so it grabs a second pool connection while already holding one. Fine today because live ingestion is single-threaded, but it's the classic pool-saturation deadlock pattern if this path ever runs concurrently.
|
Race at the migration/live frontier: a contract classified in the ledger being folded can permanently lose its deploy-ledger ContractData This is latent today (the only in-tree processor is event-only — The asymmetry Both producers of protocol state need contract membership, because ContractData extraction is footprint-gated:
The failure timeline Say a Blend-style processor tracks pools P1, P2, and migration has caught up to the frontier (cursor = 1999, window size 1 via
End state: P3 is a tracked contract, its post-2001 changes flow normally, but its deploy-ledger entries exist on chain and nowhere in the DB. Never-rewritten keys (immutable pool config) never heal; keys a later ledger rewrites heal in current-state but stay missing from history. No error is raised anywhere. The race is confined to the frontier — below the tip, a contract deployed at tip T has no footprint in older windows, and membership refreshes pick it up long before migration reaches T. But "migration catches up to tip while a new pool deploys via the permissionless factory" is exactly the window this handoff design exists for. Possible fixes Refreshing harder can't close this — committed-row reads can't see an uncommitted classification. The options change who owns the deploy ledger:
Related: |
Framework:
ContractDataChanges+ capability gateFirst of 5 stacked PRs adding Blend Capital v2 lending support.
Adds
RequiresContractData() booltoProtocolProcessorand aContractDataChanges map[string][]ingest.Changefield onProtocolProcessorInput, populated by the newindexer.ExtractContractDataChangesForLedger:GetChanges()and groups every ContractData ledger-entry change by owning contract C-address, preserving tx application order (last-write-wins folding per entry key stays deterministic). Per-tx entry removals surface asPost == nil; ledger-level archival evictions are not surfaced.processAllProtocols) extracts once per ledger, only when a tracker that will fold the ledger has a processor requiring it; all trackers share the map. Timed into the existingextractphase metric.false— event-only protocols are unaffected.Fixture-based extractor test covers 169 grouped ContractData changes across 5 real pubnet ledgers, with an independent oracle (re-walk of successful txs) rather than self-referential assertions. One fixture contains a pre-CAP-46-11 ContractData entry owned by a classic account address; it is skipped (no owning contract) and the test documents this.
Deviations register (reviewer sign-off):
processor,ProtocolProcessorMock,testRecordingProcessor, plustestProtocolProcessor(ingest_test.go, discovered viago vet)🤖 Generated with Claude Code
Review fixes:
GetByProtocolID, matching the migration engine) forRequiresContractData()processors — the event-derived subset missed contracts whose entries change without events and broke cross-contract topic disambiguation (Codex). Covered by a new CAS-gating subtest.extract_contract_datametric phase —extractkeeps one observation per ledger (Copilot).ExtractContractDataChangesForLedgerfails fast on contract-id encode errors instead of silently dropping changes (Copilot).Post-review fixes:
a5acea97— the migration engine refreshes contract membership per committed window forRequiresContractData()processors. With a run-start snapshot, a contract deployed mid-run (e.g. a new Blend pool) lost all history for the rest of the run and broke withdraw disambiguation, which depends on the tracked-contract set being current.3675a34e— live ingestion no longer materializes a ledger's transactions twice:ProcessLedgerreturns the transactions it already built and ContractData extraction reuses them (ExtractContractDataChangesFromTransactions), instead of constructing a secondLedgerTransactionReader— whose constructor re-hashes every transaction envelope — for the same ledger. The ledger-based extractor stays as a wrapper forprotocol-migrate, which has no prior transaction pass to share.c3f279bb— footprint-gated, reader-free ContractData extraction:protocol-migratewas spending ~7ms/ledger building aLedgerTransactionReader(SHA-256 of every envelope) and walkingGetChangeson every transaction, on every ledger of the range. Soroban guarantees writes ⊆ the declared read-write footprint, so a skim of the already-decoded envelopes' footprints now skips ledgers that touch no tracked contract outright, and the rare hit extracts directly from transaction meta (GetChangesreads only meta/result/ledger-version — no reader even then). The fixture-corpus test pins both properties on real ledgers: meta-based output ≡ the reader-based reference, and every changed contract, tracked alone, triggers the gate.