feat(protocols): ContractDataChanges framework seam behind a RequiresContractData capability gate#657
Open
aditya1702 wants to merge 10 commits into
Open
feat(protocols): ContractDataChanges framework seam behind a RequiresContractData capability gate#657aditya1702 wants to merge 10 commits into
aditya1702 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".
Contributor
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.
aditya1702
force-pushed
the
blend/pr1-framework-seam
branch
from
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Framework seam:
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.