[Access] Add contracts extended index and endpoints#8471
Open
peterargue wants to merge 49 commits intomasterfrom
Open
[Access] Add contracts extended index and endpoints#8471peterargue wants to merge 49 commits intomasterfrom
peterargue wants to merge 49 commits intomasterfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds full contract-deployments support: domain models, cursor types, storage index + bootstrapper and implementation, extended indexer and event decoders, backend APIs and REST experimental endpoints (parsing, models, link generator), script/execution helpers to read account code, metrics, mocks, and extensive unit & integration tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST as REST Handler
participant Backend as ContractsBackend
participant Storage as ContractDeploymentsIndex
participant ScriptExec as ScriptExecutor
Client->>REST: GET /contracts/{identifier}?expand=...
REST->>REST: parse request (limit,cursor,filter,expand)
REST->>Backend: GetContract(ctx, id, filter, expand, encoding)
Backend->>Storage: ByContract(address,name)
Storage-->>Backend: ContractDeployment (maybe placeholder)
alt expand includes code
Backend->>ScriptExec: GetAccountCode(address, name, height)
ScriptExec-->>Backend: code bytes
Backend->>Backend: attach code to deployment
end
Backend-->>REST: ContractDeployment
REST-->>Client: JSON response
sequenceDiagram
participant Indexer
participant Events as Event Decoder
participant ScriptExec as Snapshot ScriptExecutor
participant Storage as ContractDeploymentsIndex
participant Metrics
Indexer->>Events: decode AccountContractAdded/Updated events
Events-->>Indexer: parsed event (addr,name,codeHash)
Indexer->>ScriptExec: GetAccountCode(addr,name,height)
ScriptExec-->>Indexer: code bytes (or nil)
Indexer->>Indexer: validate code hash vs event
Indexer->>Storage: Store(lctx, rw, height, deployments)
Storage-->>Indexer: success
Indexer->>Metrics: ContractDeploymentIndexed(created, updated)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
zhangchiqing
approved these changes
Mar 5, 2026
| // executionParametersAddressMainnet is the address of the Execution Parameters contract on Mainnet | ||
| executionParametersAddressMainnet = flow.HexToAddress("f426ff57ee8f6110") | ||
|
|
||
| // flowFeesReceiversAddressTestnet is the address of the Flow Fees Receivers contract on Testnet |
Member
There was a problem hiding this comment.
maybe also link the tx that created them
be210889dd26a320f530595bd369093e866e26c3941bf7a3d01f861db3eeda81
fxamacker
approved these changes
Mar 7, 2026
Member
fxamacker
left a comment
fxamacker
left a comment
There was a problem hiding this comment.
Nice! I left some suggestions and comments.
Comment on lines
+66
to
+70
| req.ID = r.GetVar("identifier") | ||
|
|
||
| if err := parseContractFilter(r, &req.Filter); err != nil { | ||
| return req, err | ||
| } |
Member
There was a problem hiding this comment.
Do we want to check if req.ID has the same contract name as filter.ContractName if filter.ContractName is provided?
Contributor
Author
There was a problem hiding this comment.
I'll add the check into backend so it works for all api protocols
| if err != nil { | ||
| return req, err | ||
| } | ||
| req.Cursor = c |
Member
There was a problem hiding this comment.
Same as above. Do we want to check if req.ID has the same contract name as cursor.ContractName if cursor.ContractName is provided?
Contributor
Author
There was a problem hiding this comment.
this is handled explicitly in the backend by ignoring these fields. The cursor is documented as opaque, so I think it's OK to just ignore anything that's not valid.
Comment on lines
+111
to
+113
| if err := parseContractFilter(r, &req.Filter); err != nil { | ||
| return req, err | ||
| } |
Member
There was a problem hiding this comment.
Same as above. Do we want to check if req.ID has the same contract name as filter.ContractName if filter.ContractName is provided?
Contributor
Author
There was a problem hiding this comment.
I'll add the check into backend so it works for all api protocols
| } | ||
|
|
||
| if raw := r.GetQueryParam("cursor"); raw != "" { | ||
| c, err := DecodeContractDeploymentsCursor(raw) |
Member
There was a problem hiding this comment.
Do we want to check if address is the same as cursor.Address if cursor.Address is provided?
Contributor
Author
There was a problem hiding this comment.
this is handled explicitly in the backend by ignoring these fields. The cursor is documented as opaque, so I think it's OK to just ignore anything that's not valid.
| cursor.ContractName = contractName | ||
| } | ||
|
|
||
| iter, err := b.store.DeploymentsByContract(account, contractName, cursor) |
Member
There was a problem hiding this comment.
Should we incorporate filter.StartBlock and filter.EndBlock in the storage iteration keys?
If filter.EndBlock < cursor.BlockHeight, it can be more efficient to iterate from filter's EndBlock.
| BlockHeight string `json:"block_height,omitempty"` | ||
| TransactionId string `json:"transaction_id,omitempty"` | ||
| // Position of the deploying transaction within its block. | ||
| TxIndex string `json:"tx_index,omitempty"` |
Member
There was a problem hiding this comment.
Maybe replace "transaction" with "tx" in all the fields to be more efficient.
Or maybe replace "tx" with "transaction" here to be consistent with other fields.
Suggested change
| TxIndex string `json:"tx_index,omitempty"` | |
| TransactionIndex string `json:"transaction_index,omitempty"` |
| // contractDeploymentsCursorJSON is the JSON shape for a [accessmodel.ContractDeploymentsCursor]. | ||
| // Address and Name are omitted when empty (DeploymentsByContract cursors; the contract is | ||
| // identified by the URL parameter). | ||
| type contractDeploymentsCursorJSON struct { |
Member
There was a problem hiding this comment.
Maybe remove JSON suffix from struct name.
Suggested change
| type contractDeploymentsCursorJSON struct { | |
| type contractDeploymentsCursor struct { |
| } | ||
| } | ||
|
|
||
| flowFeesReceiversAddressesFunc := func(chainID flow.ChainID) []SystemAccount { |
Member
There was a problem hiding this comment.
Maybe remove Func suffix to be consistent with other code.
Suggested change
| flowFeesReceiversAddressesFunc := func(chainID flow.ChainID) []SystemAccount { | |
| flowFeesReceiversAddresses := func(chainID flow.ChainID) []SystemAccount { |
Contributor
Author
There was a problem hiding this comment.
this is consistent with the naming for other similar closures in the file. I'll leave it this way for consistency
| func (idx *ContractDeploymentsIndex) rangeKeysAll(cursor *access.ContractDeploymentsCursor) (startKey, endKey []byte, err error) { | ||
| prefix := []byte{codeContractDeployment} | ||
|
|
||
| if cursor == nil || cursor.ContractName == "" { |
Member
There was a problem hiding this comment.
Should we check cursor.Address as well?
Suggested change
| if cursor == nil || cursor.ContractName == "" { | |
| if cursor == nil || (cursor.Address == EmptyAddress && cursor.ContractName == "") { |
storage/indexes/contracts.go
Outdated
| // Used as the keyPrefix argument to [iterator.BuildPrefixIterator] so that all deployments of | ||
| // the same contract are grouped together and only the first (most recent) is yielded. | ||
| func contractDeploymentKeyPrefix(key []byte) []byte { | ||
| return key[:len(key)-heightLen-txIndexLen-eventIndexLen] |
Member
There was a problem hiding this comment.
Should we check key length before slicing key?
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/observer/node_builder/observer_builder.go (1)
2196-2217:⚠️ Potential issue | 🟠 MajorValidate extended-indexing prerequisites before wiring the experimental backend.
Line 2196 only checks
extendedIndexingEnabled, but the storage passed here is initialized insideBuildExecutionSyncComponents()only when execution-data sync and execution-data indexing are also enabled. That means--extended-indexing-enabled=truestill allows the contracts backend/routes to be wired with nil bootstrappers on observers that did not enable the underlying index pipeline.Suggested guard
}).ValidateFlags(func() error { + if builder.extendedIndexingEnabled { + if !builder.executionDataSyncEnabled { + return errors.New("extended-indexing-enabled requires execution-data-sync-enabled") + } + if !builder.executionDataIndexingEnabled { + return errors.New("extended-indexing-enabled requires execution-data-indexing-enabled") + } + } + if builder.executionDataSyncEnabled { if builder.executionDataConfig.FetchTimeout <= 0 { return errors.New("execution-data-fetch-timeout must be greater than 0")As per coding guidelines, treat all inputs as potentially byzantine and explicitly reject invalid configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/observer/node_builder/observer_builder.go` around lines 2196 - 2217, The code currently wires builder.ExtendedBackend when builder.extendedIndexingEnabled is true even though required execution-data components in BuildExecutionSyncComponents() (e.g., builder.ExtendedStorage bootstrappers) may be nil; update the guard that creates extendedbackend.New(...) to also verify the execution-data prerequisites (the flags/configs that enable execution-data sync and execution-data indexing and that builder.ExtendedStorage bootstrappers are non-nil) and reject or skip wiring when they are not satisfied: explicitly check those conditions before calling extendedbackend.New (referencing builder.extendedIndexingEnabled, BuildExecutionSyncComponents, builder.ExtendedStorage, and ExtendedBackend) and return an error or disable extended indexing instead of passing nil bootstrappers into New.
🧹 Nitpick comments (10)
fvm/systemcontracts/system_contracts.go (1)
130-137: Splitflow.Mainnetout of the fallback branch.Right now every non-testnet chain silently reuses the
FlowFeescontract address, even though the real mainnet receiver list still has a TODO. That makes placeholder data look authoritative and easy to forget. An explicit mainnet branch would let this fail fast, or return a clearly unsupported value, until the curated list exists.Also applies to: 484-502
module/execution/scripts.go (1)
255-258: Variablesnapshotshadows the imported package name.The variable
snapshoton line 256 shadows thesnapshotpackage import. While this compiles correctly, it can cause confusion. Consider renaming tosnapfor consistency with other methods in this file.Suggested fix
func (s *Scripts) GetStorageSnapshot(height uint64) (snapshot.StorageSnapshot, error) { - snapshot, _, err := s.snapshotWithBlock(height) - return snapshot, err + snap, _, err := s.snapshotWithBlock(height) + return snap, err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/execution/scripts.go` around lines 255 - 258, The local variable named snapshot in Scripts.GetStorageSnapshot shadows the imported snapshot package; rename that local variable (e.g., to snap) to avoid confusion and match other methods in this file, updating the assignment from snapshotWithBlock(height) and the return to use the new name; reference: Scripts.GetStorageSnapshot and snapshotWithBlock.module/state_synchronization/indexer/extended/contracts_loader.go (1)
161-167: Misleading log field nameduration_ms.The field is named
duration_msbut the value istime.Since(start).String(), which returns a string like"1.234567s"or"123.456ms", not a numeric milliseconds value. Consider renaming to"duration"or converting to actual milliseconds.Suggested fix
c.log.Info(). Uint64("height", height). Int("contracts", len(deployments)). Int("skipped_updated_in_block", len(seenContracts)). Int("deleted", deletedCount). - Str("duration_ms", time.Since(start).String()). + Dur("duration", time.Since(start)). Msg("loaded contracts during bootstrap")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/contracts_loader.go` around lines 161 - 167, The log field name duration_ms is misleading because c.log.Info() is logging time.Since(start).String() (a human-readable string) rather than a numeric millisecond value; update the call in contracts_loader.go (the c.log.Info() block that uses start and height/deployments/seenContracts/deletedCount) to either rename the field to "duration" (to keep the string output) or convert time.Since(start) to an integer milliseconds value (e.g., int64(time.Since(start)/time.Millisecond)) and keep the name "duration_ms" so the field type matches its name.module/state_synchronization/indexer/extended/events/contract.go (1)
122-139: Consider validating codeHash length.The comment states this is a "32-byte hash," but
decodeCodeHashValueaccepts arrays of any length. While Flow protocol events should be well-formed, defensive validation could catch corrupted data early.Optional: Add length validation
func decodeCodeHashValue(v cadence.Value) ([]byte, error) { arr, ok := v.(cadence.Array) if !ok { return nil, fmt.Errorf("expected cadence.Array, got %T", v) } + if len(arr.Values) != 32 { + return nil, fmt.Errorf("expected 32-byte hash, got %d bytes", len(arr.Values)) + } result := make([]byte, len(arr.Values)) for i, elem := range arr.Values {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/events/contract.go` around lines 122 - 139, The decodeCodeHashValue function currently accepts cadence.Array of any length; add defensive validation to ensure the array length equals the expected 32-byte hash length (e.g., check len(arr.Values) == 32) and return an error if it doesn't, keeping the existing element-type checks (use the function name decodeCodeHashValue and the arr.Values/UInt8 checks to locate where to insert the length validation).access/backends/extended/backend_contracts_test.go (1)
652-660: Add a same-name/out-of-range case here.This subtest only covers
(matching name, in range)and(different name, out of range), so it still passes if the combined predicate accidentally behaves likename || height. Please add a deployment whose name matches but whose height falls outsideEndBlockto pin down theContractName+ block-range semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@access/backends/extended/backend_contracts_test.go` around lines 652 - 660, Test is missing a case where ContractName matches but the deployment height is outside EndBlock; add a deployment (e.g., clone of foo) whose ContractName is "FungibleToken" but whose Height is > EndBlock (150) and assert that filter(deployment) is false. Update the subtest that constructs f := ContractDeploymentFilter{ContractName: "FungibleToken", EndBlock: &end} and uses filter := f.Filter() to include this out‑of‑range same‑name deployment (alongside existing foo and bar) to ensure the combined predicate (ContractName + EndBlock) enforces both constraints.engine/access/rest/experimental/routes/contracts.go (1)
111-167: Consider consolidating duplicate response builder logic.
buildContractDeploymentsResponseandbuildContractsResponseshare nearly identical logic for building deployments and encoding cursors, differing only in the response struct field names. A generic helper or shared function could reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@engine/access/rest/experimental/routes/contracts.go` around lines 111 - 167, Both buildContractDeploymentsResponse and buildContractsResponse duplicate deployment-building and cursor-encoding logic; extract that shared work into a helper (e.g., buildDeploymentsAndCursor or similar) that accepts (*accessmodel.ContractDeploymentPage, models.LinkGenerator, map[string]bool) and returns ([]models.ContractDeployment, string, error), move the loop that calls ContractDeployment.Build and the request.EncodeContractDeploymentsCursor logic into that helper, and then update buildContractDeploymentsResponse and buildContractsResponse to call the helper and simply wrap the returned deployments and nextCursor into their respective response structs.module/state_synchronization/indexer/extended/contracts.go (2)
173-175: Include address in code hash mismatch error messages.The error message includes the event type and contract name but not the account address, which would help debugging.
♻️ Suggested improvement
- return nil, 0, 0, fmt.Errorf("code hash mismatch for %s event: %s", event.Type, e.ContractName) + return nil, 0, 0, fmt.Errorf("code hash mismatch for %s event: %s on %s", event.Type, e.ContractName, e.Address)Also applies to: 205-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/contracts.go` around lines 173 - 175, The code hash mismatch error messages (the fmt.Errorf calls comparing e.CodeHash with access.CadenceCodeHash(code)) omit the account address; update those error strings to include the address (use the event/contract holder field, e.g. e.Address or the struct field that holds the account) so they read like "code hash mismatch for %s event: %s (address: %s)". Modify both occurrences (the one using event.Type and e.ContractName around the e.CodeHash check and the similar block at the later location) to pass the address value as an additional fmt.Errorf argument.
156-188: Consider extracting common deployment creation logic.The
flowAccountContractAddedandflowAccountContractUpdatedcases share nearly identical code for decoding events, fetching code, validating hash, and creating deployments. While the current approach is readable, extracting a helper could reduce duplication.Also applies to: 189-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/contracts.go` around lines 156 - 188, Extract the duplicated logic in the flowAccountContractAdded and flowAccountContractUpdated cases into a small helper (e.g., buildContractDeployment or createDeploymentFromEvent) that accepts the decoded event payload/result (or a function to decode it), the event metadata, and data.Header.Height; inside the helper call retriever.contractCode, verify the hash with access.CadenceCodeHash and bytes.Equal, construct and return an access.ContractDeployment (with ContractName, Address, BlockHeight, TransactionID, TransactionIndex, EventIndex, Code, CodeHash), and any error; replace the duplicated blocks to call this helper after using events.DecodeAccountContractAdded / events.DecodeAccountContractUpdated to keep decoding specific but deployment creation shared.access/backends/extended/backend_contracts.go (2)
90-121:filterparameter is accepted but not applied inGetContract.The
filterparameter is part of the method signature but is never used.ByContractretrieves the most recent deployment directly, and the filter'sStartBlock/EndBlock/ContractNameconstraints are not applied. If filtering is not intended for single-contract lookup, consider removing the parameter or documenting why it's ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@access/backends/extended/backend_contracts.go` around lines 90 - 121, GetContract currently ignores the incoming filter parameter; fix by either applying the filter or rejecting it: if your storage supports ranged lookups use the range-aware store call (e.g., replace b.store.ByContract(...) with the store method that accepts block range / ContractDeploymentFilter such as b.store.ByContractRange/ByContractWithFilter) and pass filter.StartBlock/EndBlock and ensure filter.ContractName (if present) matches parsed contractName, otherwise if the backend only ever returns the latest deployment then explicitly validate and return InvalidArgument when filter contains non-empty constraints (StartBlock/EndBlock/ContractName) or remove the unused filter parameter from the GetContract signature; keep the existing expand logic (expand) and code-clearing behavior unchanged.
149-153: Cursor mutation may have unintended side effects.The caller's cursor pointer is modified directly (
cursor.Address = account). If the caller reuses the cursor object after this call, they'll observe the modified values. This pattern also appears at lines 270-273. Consider documenting this behavior or working with a local copy if mutation is undesirable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@access/backends/extended/backend_contracts.go` around lines 149 - 153, The code mutates the caller's cursor pointer (cursor.Address and cursor.ContractName), causing surprising side effects if the caller reuses the object; change the logic to avoid mutating the passed-in pointer by working on a local copy (e.g., copy := *cursor or newCursor := cursor.Clone()), update the copy's Address and ContractName and use that copy for further processing, and apply the same change to the other occurrence that sets cursor.Address/cursor.ContractName so callers’ cursors remain unchanged (alternatively document the mutation clearly if mutation is intended).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@access/backends/extended/backend_contracts_test.go`:
- Around line 104-120: The callback installed by
contractSignalerCtxExpectingThrow currently does a blocking send into the size-1
channel `thrown`, which can deadlock if `irrecoverable.Throw` is invoked more
than once; change the callback passed to
`irrecoverable.NewMockSignalerContextWithCallback` to perform a non-blocking
send (e.g., use a select with a default) into `thrown` so the callback never
blocks, and keep `verify` unchanged to assert that at least one error was
received from `thrown`.
In `@engine/access/rest/experimental/models/contract_deployment.go`:
- Around line 14-15: The Build method on ContractDeployment lacks a nil guard
for its input and will panic when accessing d.Address; update
ContractDeployment.Build to check if d == nil at the start (input type
accessmodel.ContractDeployment) and return a descriptive error (e.g., fmt.Errorf
or errors.New) instead of proceeding, then only assign m.ContractId =
accessmodel.ContractID(d.Address, d.ContractName) when d is non-nil.
In `@engine/access/rest/experimental/models/link.go`:
- Around line 30-50: The route lookup can return nil and is dereferenced in
ContractCodeLink and link; update both to guard the router.Get(route) call (and
router.Get("getContract") in ContractCodeLink) by assigning its result to a
variable, checking for nil, and returning a descriptive error (e.g.,
fmt.Errorf("route %q not found", route or "getContract")) before calling
URLPath; ensure fmt is imported if needed and keep the existing behavior of
returning URLPath errors unchanged.
In `@integration/tests/access/cohort3/extended_indexing_contracts_test.go`:
- Around line 260-262: The call to s.Require().Fail uses a format string with
one %s but passes three arguments, producing a malformed message; replace this
with s.Require().Failf and a matching format string, e.g. use
s.Require().Failf("contract %s not found in /accounts/%s/contracts list",
accessmodel.ContractID(expected.Address, expected.ContractName), address) so the
format placeholders match the supplied values (references: s.Require().Fail /
s.Require().Failf, accessmodel.ContractID, expected, address).
In `@model/access/scheduled_transaction.go`:
- Around line 135-147: HandlerContractID() currently returns only
"A.<address>.<contract>" which lets lookups (e.g., backend call
b.contracts.ByContract) return the latest deployment and thus lose historical
context; update the code to include a deployment disambiguator when computing
the handler identifier (use CreatedAt or CreatedTransactionID from
ScheduledTransaction) so lookups can request the specific ContractDeployment, or
change the HandlerContract field back to *Contract if API intends to expose only
current metadata; specifically, modify HandlerContractID (and any callers) to
append or encode the deployment discriminator (e.g., timestamp or tx id) and
ensure backend lookups (ByContract/ByContractDeployment) accept and use that
discriminator to fetch the exact ContractDeployment rather than the newest
deployment.
In `@module/state_synchronization/indexer/extended/mock/snapshot_provider.go`:
- Around line 13-24: The mock types are unexported so other packages can't use
them; rename and export the constructor and type to match package conventions
(e.g., change snapshotProvider -> SnapshotProvider and newSnapshotProvider ->
NewSnapshotProvider) or regenerate the mock with mockery export settings; ensure
the exported NewSnapshotProvider retains the same signature (accepts interface {
mock.TestingT; Cleanup(func()) }), still calls mock.Mock.Test(t), registers
t.Cleanup(func() { mock.AssertExpectations(t) }) and returns *SnapshotProvider
so callers in other packages can construct and use the mock.
In `@storage/inmemory/registers_reader.go`:
- Around line 58-61: The ByKeyPrefix stub in RegistersReader should return a
clearer, descriptive error explaining that range scans are not supported by the
in-memory implementation; update RegistersReader.ByKeyPrefix to call yield(nil,
fmt.Errorf(...)) with a message referencing the method name (ByKeyPrefix), the
unsupported operation (range/key-prefix scans), and include the keyPrefix and
height parameters for debugging (keep behavior matching that it fails
immediately rather than validating height like Get). Mention
storage.IndexIterator, flow.RegisterID and flow.RegisterValue so the change is
applied to the existing iterator-returning stub.
In `@storage/pebble/registers_test.go`:
- Around line 563-572: In collectRegistersByKey, guard against silently
overwriting duplicate cursors by checking whether results already contains
entry.Cursor() before assigning; if a cursor already exists, call require.Failf
(or require.Fatalf/require.False with a clear message) to fail the test and
include the duplicate cursor and both values, otherwise assign
results[entry.Cursor()] = val—this ensures the helper asserts uniqueness as well
as value correctness for the ByKey iterator.
---
Outside diff comments:
In `@cmd/observer/node_builder/observer_builder.go`:
- Around line 2196-2217: The code currently wires builder.ExtendedBackend when
builder.extendedIndexingEnabled is true even though required execution-data
components in BuildExecutionSyncComponents() (e.g., builder.ExtendedStorage
bootstrappers) may be nil; update the guard that creates
extendedbackend.New(...) to also verify the execution-data prerequisites (the
flags/configs that enable execution-data sync and execution-data indexing and
that builder.ExtendedStorage bootstrappers are non-nil) and reject or skip
wiring when they are not satisfied: explicitly check those conditions before
calling extendedbackend.New (referencing builder.extendedIndexingEnabled,
BuildExecutionSyncComponents, builder.ExtendedStorage, and ExtendedBackend) and
return an error or disable extended indexing instead of passing nil
bootstrappers into New.
---
Nitpick comments:
In `@access/backends/extended/backend_contracts_test.go`:
- Around line 652-660: Test is missing a case where ContractName matches but the
deployment height is outside EndBlock; add a deployment (e.g., clone of foo)
whose ContractName is "FungibleToken" but whose Height is > EndBlock (150) and
assert that filter(deployment) is false. Update the subtest that constructs f :=
ContractDeploymentFilter{ContractName: "FungibleToken", EndBlock: &end} and uses
filter := f.Filter() to include this out‑of‑range same‑name deployment
(alongside existing foo and bar) to ensure the combined predicate (ContractName
+ EndBlock) enforces both constraints.
In `@access/backends/extended/backend_contracts.go`:
- Around line 90-121: GetContract currently ignores the incoming filter
parameter; fix by either applying the filter or rejecting it: if your storage
supports ranged lookups use the range-aware store call (e.g., replace
b.store.ByContract(...) with the store method that accepts block range /
ContractDeploymentFilter such as b.store.ByContractRange/ByContractWithFilter)
and pass filter.StartBlock/EndBlock and ensure filter.ContractName (if present)
matches parsed contractName, otherwise if the backend only ever returns the
latest deployment then explicitly validate and return InvalidArgument when
filter contains non-empty constraints (StartBlock/EndBlock/ContractName) or
remove the unused filter parameter from the GetContract signature; keep the
existing expand logic (expand) and code-clearing behavior unchanged.
- Around line 149-153: The code mutates the caller's cursor pointer
(cursor.Address and cursor.ContractName), causing surprising side effects if the
caller reuses the object; change the logic to avoid mutating the passed-in
pointer by working on a local copy (e.g., copy := *cursor or newCursor :=
cursor.Clone()), update the copy's Address and ContractName and use that copy
for further processing, and apply the same change to the other occurrence that
sets cursor.Address/cursor.ContractName so callers’ cursors remain unchanged
(alternatively document the mutation clearly if mutation is intended).
In `@engine/access/rest/experimental/routes/contracts.go`:
- Around line 111-167: Both buildContractDeploymentsResponse and
buildContractsResponse duplicate deployment-building and cursor-encoding logic;
extract that shared work into a helper (e.g., buildDeploymentsAndCursor or
similar) that accepts (*accessmodel.ContractDeploymentPage,
models.LinkGenerator, map[string]bool) and returns ([]models.ContractDeployment,
string, error), move the loop that calls ContractDeployment.Build and the
request.EncodeContractDeploymentsCursor logic into that helper, and then update
buildContractDeploymentsResponse and buildContractsResponse to call the helper
and simply wrap the returned deployments and nextCursor into their respective
response structs.
In `@module/execution/scripts.go`:
- Around line 255-258: The local variable named snapshot in
Scripts.GetStorageSnapshot shadows the imported snapshot package; rename that
local variable (e.g., to snap) to avoid confusion and match other methods in
this file, updating the assignment from snapshotWithBlock(height) and the return
to use the new name; reference: Scripts.GetStorageSnapshot and
snapshotWithBlock.
In `@module/state_synchronization/indexer/extended/contracts_loader.go`:
- Around line 161-167: The log field name duration_ms is misleading because
c.log.Info() is logging time.Since(start).String() (a human-readable string)
rather than a numeric millisecond value; update the call in contracts_loader.go
(the c.log.Info() block that uses start and
height/deployments/seenContracts/deletedCount) to either rename the field to
"duration" (to keep the string output) or convert time.Since(start) to an
integer milliseconds value (e.g., int64(time.Since(start)/time.Millisecond)) and
keep the name "duration_ms" so the field type matches its name.
In `@module/state_synchronization/indexer/extended/contracts.go`:
- Around line 173-175: The code hash mismatch error messages (the fmt.Errorf
calls comparing e.CodeHash with access.CadenceCodeHash(code)) omit the account
address; update those error strings to include the address (use the
event/contract holder field, e.g. e.Address or the struct field that holds the
account) so they read like "code hash mismatch for %s event: %s (address: %s)".
Modify both occurrences (the one using event.Type and e.ContractName around the
e.CodeHash check and the similar block at the later location) to pass the
address value as an additional fmt.Errorf argument.
- Around line 156-188: Extract the duplicated logic in the
flowAccountContractAdded and flowAccountContractUpdated cases into a small
helper (e.g., buildContractDeployment or createDeploymentFromEvent) that accepts
the decoded event payload/result (or a function to decode it), the event
metadata, and data.Header.Height; inside the helper call retriever.contractCode,
verify the hash with access.CadenceCodeHash and bytes.Equal, construct and
return an access.ContractDeployment (with ContractName, Address, BlockHeight,
TransactionID, TransactionIndex, EventIndex, Code, CodeHash), and any error;
replace the duplicated blocks to call this helper after using
events.DecodeAccountContractAdded / events.DecodeAccountContractUpdated to keep
decoding specific but deployment creation shared.
In `@module/state_synchronization/indexer/extended/events/contract.go`:
- Around line 122-139: The decodeCodeHashValue function currently accepts
cadence.Array of any length; add defensive validation to ensure the array length
equals the expected 32-byte hash length (e.g., check len(arr.Values) == 32) and
return an error if it doesn't, keeping the existing element-type checks (use the
function name decodeCodeHashValue and the arr.Values/UInt8 checks to locate
where to insert the length validation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a6e9e22-ad1b-4abd-ac0b-35ff11fc54f6
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumintegration/go.sumis excluded by!**/*.sum
📒 Files selected for processing (98)
access/backends/extended/api.goaccess/backends/extended/backend.goaccess/backends/extended/backend_account_transfers_test.goaccess/backends/extended/backend_contracts.goaccess/backends/extended/backend_contracts_test.goaccess/backends/extended/backend_scheduled_transactions.goaccess/backends/extended/backend_scheduled_transactions_test.goaccess/backends/extended/mock/api.gocmd/access/node_builder/access_node_builder.gocmd/observer/node_builder/observer_builder.goengine/access/rest/experimental/handler.goengine/access/rest/experimental/models/account_transaction.goengine/access/rest/experimental/models/contract.goengine/access/rest/experimental/models/contract_deployment.goengine/access/rest/experimental/models/fungible_token_transfer.goengine/access/rest/experimental/models/link.goengine/access/rest/experimental/models/model_contract_deployment.goengine/access/rest/experimental/models/model_contract_deployment__expandable.goengine/access/rest/experimental/models/model_contract_deployments_response.goengine/access/rest/experimental/models/model_contracts_response.goengine/access/rest/experimental/models/model_scheduled_transaction.goengine/access/rest/experimental/models/non_fungible_token_transfer.goengine/access/rest/experimental/models/scheduled_transaction.goengine/access/rest/experimental/request/cursor_contracts.goengine/access/rest/experimental/request/cursor_transfer.goengine/access/rest/experimental/request/get_contracts.goengine/access/rest/experimental/routes/account_ft_transfers.goengine/access/rest/experimental/routes/account_nft_transfers.goengine/access/rest/experimental/routes/account_transactions.goengine/access/rest/experimental/routes/contracts.goengine/access/rest/experimental/routes/contracts_test.goengine/access/rest/experimental/routes/scheduled_transactions.goengine/access/rest/experimental/routes/scheduled_transactions_test.goengine/access/rest/router/router.goengine/access/rest/router/routes_experimental.goengine/access/rpc/backend/script_executor.goengine/execution/computation/query/executor.goengine/execution/mock/on_disk_register_store.gofvm/fvm.gofvm/systemcontracts/system_contracts.gofvm/systemcontracts/system_contracts_test.gogo.modintegration/go.modintegration/testnet/experimental_client.gointegration/tests/access/cohort3/extended_indexing_contracts_test.gointegration/tests/access/cohort3/extended_indexing_test.gomodel/access/account_transaction.gomodel/access/account_transfer.gomodel/access/contract.gomodel/access/contract_deployment.gomodel/access/contract_deployment_test.gomodel/access/scheduled_transaction.gomodule/execution/mock/script_executor.gomodule/execution/scripts.gomodule/metrics.gomodule/metrics/extended_indexing.gomodule/metrics/noop.gomodule/mock/extended_indexing_metrics.gomodule/state_synchronization/indexer/extended/account_ft_transfers.gomodule/state_synchronization/indexer/extended/account_nft_transfers.gomodule/state_synchronization/indexer/extended/account_transactions.gomodule/state_synchronization/indexer/extended/bootstrap/bootstrap.gomodule/state_synchronization/indexer/extended/contracts.gomodule/state_synchronization/indexer/extended/contracts_loader.gomodule/state_synchronization/indexer/extended/contracts_test.gomodule/state_synchronization/indexer/extended/events/contract.gomodule/state_synchronization/indexer/extended/events/helpers_test.gomodule/state_synchronization/indexer/extended/indexer.gomodule/state_synchronization/indexer/extended/mock/contract_script_executor.gomodule/state_synchronization/indexer/extended/mock/register_scanner.gomodule/state_synchronization/indexer/extended/mock/snapshot_provider.gomodule/state_synchronization/indexer/extended/scheduled_transactions.gomodule/state_synchronization/indexer/extended/transfers/ft_group.gomodule/state_synchronization/indexer/extended/transfers/ft_parser.gomodule/state_synchronization/indexer/extended/transfers/ft_parser_test.gomodule/state_synchronization/indexer/indexer_core.gostorage/contract_deployments.gostorage/indexes/contracts.gostorage/indexes/contracts_bootstrapper.gostorage/indexes/contracts_bootstrapper_test.gostorage/indexes/contracts_test.gostorage/indexes/iterator/iterator.gostorage/indexes/prefix.gostorage/inmemory/registers_reader.gostorage/locks.gostorage/mock/contract_deployments_index.gostorage/mock/contract_deployments_index_bootstrapper.gostorage/mock/contract_deployments_index_range_reader.gostorage/mock/contract_deployments_index_reader.gostorage/mock/contract_deployments_index_writer.gostorage/mock/register_index.gostorage/mock/register_index_reader.gostorage/pebble/lookup.gostorage/pebble/lookup_test.gostorage/pebble/registers.gostorage/pebble/registers_test.gostorage/registers.goutils/slices/slices.go
💤 Files with no reviewable changes (3)
- engine/access/rest/experimental/models/contract.go
- access/backends/extended/backend_account_transfers_test.go
- model/access/contract.go
Comment on lines
+104
to
+120
| func contractSignalerCtxExpectingThrow(t *testing.T) (context.Context, func()) { | ||
| t.Helper() | ||
| thrown := make(chan error, 1) | ||
| m := irrecoverable.NewMockSignalerContextWithCallback(t, context.Background(), func(err error) { | ||
| thrown <- err | ||
| }) | ||
| ctx := irrecoverable.WithSignalerContext(context.Background(), m) | ||
| verify := func() { | ||
| t.Helper() | ||
| select { | ||
| case err := <-thrown: | ||
| require.Error(t, err) | ||
| default: | ||
| t.Fatal("expected irrecoverable.Throw to be called but it was not") | ||
| } | ||
| } | ||
| return ctx, verify |
Contributor
There was a problem hiding this comment.
Make the signaler callback non-blocking.
The callback writes straight into a size-1 channel. If a regression causes irrecoverable.Throw to fire twice, the second send blocks and the test hangs instead of producing a failure. Use a non-blocking send or track the throw count separately in verify.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@access/backends/extended/backend_contracts_test.go` around lines 104 - 120,
The callback installed by contractSignalerCtxExpectingThrow currently does a
blocking send into the size-1 channel `thrown`, which can deadlock if
`irrecoverable.Throw` is invoked more than once; change the callback passed to
`irrecoverable.NewMockSignalerContextWithCallback` to perform a non-blocking
send (e.g., use a select with a default) into `thrown` so the callback never
blocks, and keep `verify` unchanged to assert that at least one error was
received from `thrown`.
Comment on lines
+14
to
+15
| func (m *ContractDeployment) Build(d *accessmodel.ContractDeployment, link LinkGenerator, expand map[string]bool) error { | ||
| m.ContractId = accessmodel.ContractID(d.Address, d.ContractName) |
Contributor
There was a problem hiding this comment.
Add nil check for input parameter d.
If nil is passed for d, the method will panic on line 15 when accessing d.Address. Consider adding a guard.
🛡️ Proposed fix
func (m *ContractDeployment) Build(d *accessmodel.ContractDeployment, link LinkGenerator, expand map[string]bool) error {
+ if d == nil {
+ return fmt.Errorf("contract deployment cannot be nil")
+ }
m.ContractId = accessmodel.ContractID(d.Address, d.ContractName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/access/rest/experimental/models/contract_deployment.go` around lines
14 - 15, The Build method on ContractDeployment lacks a nil guard for its input
and will panic when accessing d.Address; update ContractDeployment.Build to
check if d == nil at the start (input type accessmodel.ContractDeployment) and
return a descriptive error (e.g., fmt.Errorf or errors.New) instead of
proceeding, then only assign m.ContractId = accessmodel.ContractID(d.Address,
d.ContractName) when d is non-nil.
Comment on lines
+30
to
+50
| func (generator *LinkGeneratorImpl) ContractLink(identifier string) (string, error) { | ||
| return generator.link("getContract", "identifier", identifier) | ||
| } | ||
|
|
||
| func (generator *LinkGeneratorImpl) ContractCodeLink(identifier string) (string, error) { | ||
| u, err := generator.router.Get("getContract").URLPath("identifier", identifier) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| q := u.Query() | ||
| q.Set("expand", "code") | ||
| u.RawQuery = q.Encode() | ||
| return u.String(), nil | ||
| } | ||
|
|
||
| func (generator *LinkGeneratorImpl) link(route string, key string, value string) (string, error) { | ||
| url, err := generator.router.Get(route).URLPath(key, value) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return url.String(), nil |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect at least one registration for the named route used here.
# No match means the current code path can panic immediately.
rg -n --type=go 'Name\("getContract"\)|Name\(`getContract`\)'Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
cat -n engine/access/rest/experimental/models/link.goRepository: onflow/flow-go
Length of output: 1829
🏁 Script executed:
rg -n --type=go 'import|irrecoverable|fmt\.Errorf' engine/access/rest/experimental/models/link.goRepository: onflow/flow-go
Length of output: 68
🏁 Script executed:
# Find where the router field is declared in LinkGeneratorImpl
ast-grep --pattern $'type LinkGeneratorImpl struct {
$$$
router $_
$$$
}'Repository: onflow/flow-go
Length of output: 525
🏁 Script executed:
fd -t f 'irrecoverable' --type f | head -20Repository: onflow/flow-go
Length of output: 227
🏁 Script executed:
cat -n engine/access/rest/common/models/link.goRepository: onflow/flow-go
Length of output: 3529
🏁 Script executed:
# Search for where routes are registered (Name("getContract"))
rg -n --type=go 'HandleFunc|Methods\(' engine/access/rest/ | grep -i contract | head -20Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
head -50 module/irrecoverable/irrecoverable.goRepository: onflow/flow-go
Length of output: 1425
🏁 Script executed:
# Search for router setup/registration in the REST handlers
rg -n --type=go 'NewRouter|router\s*:=|mux.NewRouter' engine/access/rest/ -A 3 | head -40Repository: onflow/flow-go
Length of output: 2840
🏁 Script executed:
# Check if there are tests or setup code that registers these routes
rg -n --type=go 'Name\(|HandleFunc|Methods' engine/access/rest/routes/ | head -30Repository: onflow/flow-go
Length of output: 124
🏁 Script executed:
rg -n --type=go 'getContract|AddExperimentalRoutes' engine/access/rest/ -A 5 | head -60Repository: onflow/flow-go
Length of output: 3714
🏁 Script executed:
# Look for all route Name(...) registrations in the REST code
rg -n --type=go 'Name\(' engine/access/rest/ -B 2 -A 2 | head -80Repository: onflow/flow-go
Length of output: 1659
🏁 Script executed:
rg -n --type=go 'errors\.New|fmt\.Errorf|irrecoverable' engine/access/rest/ -B 1 -A 1 | head -50Repository: onflow/flow-go
Length of output: 4073
🏁 Script executed:
# Check what's imported in routes_experimental.go
head -30 engine/access/rest/router/routes_experimental.goRepository: onflow/flow-go
Length of output: 837
Guard named-route lookup before calling URLPath.
router.Get(...) can return nil when the route name is missing. Both ContractCodeLink and the link helper dereference that result immediately, causing a panic during response rendering instead of returning an error.
🛠️ Suggested fix
func (generator *LinkGeneratorImpl) ContractCodeLink(identifier string) (string, error) {
- u, err := generator.router.Get("getContract").URLPath("identifier", identifier)
+ route := generator.router.Get("getContract")
+ if route == nil {
+ return "", fmt.Errorf("route %q is not registered", "getContract")
+ }
+ u, err := route.URLPath("identifier", identifier)
if err != nil {
return "", err
}
q := u.Query()
q.Set("expand", "code")
u.RawQuery = q.Encode()
return u.String(), nil
}
func (generator *LinkGeneratorImpl) link(route string, key string, value string) (string, error) {
- url, err := generator.router.Get(route).URLPath(key, value)
+ r := generator.router.Get(route)
+ if r == nil {
+ return "", fmt.Errorf("route %q is not registered", route)
+ }
+ url, err := r.URLPath(key, value)
if err != nil {
return "", err
}
return url.String(), nil
}(Ensure fmt is imported if not already present.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (generator *LinkGeneratorImpl) ContractLink(identifier string) (string, error) { | |
| return generator.link("getContract", "identifier", identifier) | |
| } | |
| func (generator *LinkGeneratorImpl) ContractCodeLink(identifier string) (string, error) { | |
| u, err := generator.router.Get("getContract").URLPath("identifier", identifier) | |
| if err != nil { | |
| return "", err | |
| } | |
| q := u.Query() | |
| q.Set("expand", "code") | |
| u.RawQuery = q.Encode() | |
| return u.String(), nil | |
| } | |
| func (generator *LinkGeneratorImpl) link(route string, key string, value string) (string, error) { | |
| url, err := generator.router.Get(route).URLPath(key, value) | |
| if err != nil { | |
| return "", err | |
| } | |
| return url.String(), nil | |
| func (generator *LinkGeneratorImpl) ContractLink(identifier string) (string, error) { | |
| return generator.link("getContract", "identifier", identifier) | |
| } | |
| func (generator *LinkGeneratorImpl) ContractCodeLink(identifier string) (string, error) { | |
| route := generator.router.Get("getContract") | |
| if route == nil { | |
| return "", irrecoverable.NewExceptionf("route %q is not registered", "getContract") | |
| } | |
| u, err := route.URLPath("identifier", identifier) | |
| if err != nil { | |
| return "", err | |
| } | |
| q := u.Query() | |
| q.Set("expand", "code") | |
| u.RawQuery = q.Encode() | |
| return u.String(), nil | |
| } | |
| func (generator *LinkGeneratorImpl) link(route string, key string, value string) (string, error) { | |
| r := generator.router.Get(route) | |
| if r == nil { | |
| return "", irrecoverable.NewExceptionf("route %q is not registered", route) | |
| } | |
| url, err := r.URLPath(key, value) | |
| if err != nil { | |
| return "", err | |
| } | |
| return url.String(), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/access/rest/experimental/models/link.go` around lines 30 - 50, The
route lookup can return nil and is dereferenced in ContractCodeLink and link;
update both to guard the router.Get(route) call (and router.Get("getContract")
in ContractCodeLink) by assigning its result to a variable, checking for nil,
and returning a descriptive error (e.g., fmt.Errorf("route %q not found", route
or "getContract")) before calling URLPath; ensure fmt is imported if needed and
keep the existing behavior of returning URLPath errors unchanged.
Comment on lines
+260
to
+262
| s.Require().Fail("contract should appear in /accounts/%s/contracts list", | ||
| "contract %s not found in /accounts/%s/contracts list", address, accessmodel.ContractID(expected.Address, expected.ContractName), address) | ||
| } |
Contributor
There was a problem hiding this comment.
Fix mismatched format arguments in Require.Fail.
The format string contains one %s placeholder but three arguments are passed. This will produce a malformed error message.
🐛 Proposed fix
- s.Require().Fail("contract should appear in /accounts/%s/contracts list",
- "contract %s not found in /accounts/%s/contracts list", address, accessmodel.ContractID(expected.Address, expected.ContractName), address)
+ s.Require().Fail(fmt.Sprintf("contract %s not found in /accounts/%s/contracts list",
+ accessmodel.ContractID(expected.Address, expected.ContractName), address))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/tests/access/cohort3/extended_indexing_contracts_test.go` around
lines 260 - 262, The call to s.Require().Fail uses a format string with one %s
but passes three arguments, producing a malformed message; replace this with
s.Require().Failf and a matching format string, e.g. use
s.Require().Failf("contract %s not found in /accounts/%s/contracts list",
accessmodel.ContractID(expected.Address, expected.ContractName), address) so the
format placeholders match the supplied values (references: s.Require().Fail /
s.Require().Failf, accessmodel.ContractID, expected, address).
Comment on lines
+135
to
+147
| HandlerContract *ContractDeployment `msgpack:"-"` // Handler contract (nil unless expanded) | ||
|
|
||
| // Timestamp fields are populated by the backend. Never persisted. Zero when not applicable. | ||
| CreatedAt uint64 `msgpack:"-"` // Unix ms timestamp of block in which the scheduled transaction was created | ||
| CompletedAt uint64 `msgpack:"-"` // Unix ms timestamp of block in which the scheduled transaction was executed or cancelled | ||
| } | ||
|
|
||
| func (tx *ScheduledTransaction) HandlerContractID() (string, error) { | ||
| parts := strings.Split(tx.TransactionHandlerTypeIdentifier, ".") | ||
| if len(parts) < 3 { | ||
| return "", fmt.Errorf("invalid handler type identifier: %s", tx.TransactionHandlerTypeIdentifier) | ||
| } | ||
| return strings.Join(parts[:3], "."), nil |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites of ScheduledTransaction.HandlerContractID():"
rg -n -C3 '\.HandlerContractID\s*\(' --type go
echo
echo "Population sites for ScheduledTransaction.HandlerContract:"
rg -n -C4 'HandlerContract\s*=|HandlerContract:' --type go
echo
echo "Contract-deployment lookups that appear to carry historical context:"
rg -n -C4 'ContractDeployment.*(Height|Block|Transaction|Created)|GetContractDeployment.*(Height|Block|Transaction|Created)' --type goRepository: onflow/flow-go
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full expandHandlerContract function ==="
sed -n '430,480p' access/backends/extended/backend_scheduled_transactions.go
echo
echo "=== ByContract method signature ==="
rg -A10 'func.*ByContract' access/backends/extended/ --type go | head -30
echo
echo "=== Check if height/block context is available in scheduled transaction ==="
rg -B5 -A15 'expandHandlerContract' access/backends/extended/backend_scheduled_transactions.go | head -50Repository: onflow/flow-go
Length of output: 1867
HandlerContractID() extracts a contract identifier insufficient for deployment disambiguation.
The method returns A.<address>.<contract>, which identifies the current contract but not a specific deployment. When ScheduledTransaction.HandlerContract is populated via b.contracts.ByContract(address, contractName) (line 445 in access/backends/extended/backend_scheduled_transactions.go), the lookup has no historical context. Any contract upgrade causes older scheduled transactions to point at the newest deployment instead of the one active when the handler was created.
Thread a deployment disambiguator into the lookup (for example CreatedAt or CreatedTransactionID, both available in the struct), or revert HandlerContract to *Contract if the API only intends to expose current metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@model/access/scheduled_transaction.go` around lines 135 - 147,
HandlerContractID() currently returns only "A.<address>.<contract>" which lets
lookups (e.g., backend call b.contracts.ByContract) return the latest deployment
and thus lose historical context; update the code to include a deployment
disambiguator when computing the handler identifier (use CreatedAt or
CreatedTransactionID from ScheduledTransaction) so lookups can request the
specific ContractDeployment, or change the HandlerContract field back to
*Contract if API intends to expose only current metadata; specifically, modify
HandlerContractID (and any callers) to append or encode the deployment
discriminator (e.g., timestamp or tx id) and ensure backend lookups
(ByContract/ByContractDeployment) accept and use that discriminator to fetch the
exact ContractDeployment rather than the newest deployment.
Comment on lines
+13
to
+24
| // newSnapshotProvider creates a new instance of snapshotProvider. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | ||
| // The first argument is typically a *testing.T value. | ||
| func newSnapshotProvider(t interface { | ||
| mock.TestingT | ||
| Cleanup(func()) | ||
| }) *snapshotProvider { | ||
| mock := &snapshotProvider{} | ||
| mock.Mock.Test(t) | ||
|
|
||
| t.Cleanup(func() { mock.AssertExpectations(t) }) | ||
|
|
||
| return mock |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether this mock package exposes any exported top-level constructor/type.
sed -n '13,40p' module/state_synchronization/indexer/extended/mock/snapshot_provider.go
printf '\nExported top-level declarations in module/state_synchronization/indexer/extended/mock:\n'
rg -n --type=go '^(func|type) [A-Z]' module/state_synchronization/indexer/extended/mockRepository: onflow/flow-go
Length of output: 2130
Export a usable entry point for this mock to match the pattern used by other mocks in this package.
This file only exports package-private symbols (newSnapshotProvider, snapshotProvider), while peer mocks in the same package (indexer.go, indexer_manager.go) all export their constructors and types with uppercase names (NewIndexer, Indexer, NewIndexerManager, IndexerManager). Callers in other packages cannot construct this mock, making it unusable from tests that depend on it. Regenerate with mockery configured to export the constructor and type, matching the pattern of existing mocks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@module/state_synchronization/indexer/extended/mock/snapshot_provider.go`
around lines 13 - 24, The mock types are unexported so other packages can't use
them; rename and export the constructor and type to match package conventions
(e.g., change snapshotProvider -> SnapshotProvider and newSnapshotProvider ->
NewSnapshotProvider) or regenerate the mock with mockery export settings; ensure
the exported NewSnapshotProvider retains the same signature (accepts interface {
mock.TestingT; Cleanup(func()) }), still calls mock.Mock.Test(t), registers
t.Cleanup(func() { mock.AssertExpectations(t) }) and returns *SnapshotProvider
so callers in other packages can construct and use the mock.
Comment on lines
+58
to
+61
| func (r *RegistersReader) ByKeyPrefix(keyPrefix string, height uint64, cursor *flow.RegisterID) storage.IndexIterator[flow.RegisterValue, flow.RegisterID] { | ||
| return func(yield func(storage.IteratorEntry[flow.RegisterValue, flow.RegisterID], error) bool) { | ||
| yield(nil, fmt.Errorf("not implemented")) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other RegisterIndexReader implementations have ByKeyPrefix implemented
ast-grep --pattern $'func ($_ $_) ByKeyPrefix($$$) storage.IndexIterator[$$$] {
$$$
}'Repository: onflow/flow-go
Length of output: 12598
🏁 Script executed:
sed -n '30,45p' storage/inmemory/registers_reader.goRepository: onflow/flow-go
Length of output: 679
Improve error message for clarity on unsupported operation.
The stub implementation should provide a more descriptive error message. Unlike the Get method which validates height upfront (appropriate for single-value lookups), production implementations like Pebble handle height validation during iteration for range scans. For this stub, focus on clarifying why the operation is not supported.
♻️ Proposed improvement
func (r *RegistersReader) ByKeyPrefix(keyPrefix string, height uint64, cursor *flow.RegisterID) storage.IndexIterator[flow.RegisterValue, flow.RegisterID] {
return func(yield func(storage.IteratorEntry[flow.RegisterValue, flow.RegisterID], error) bool) {
- yield(nil, fmt.Errorf("not implemented"))
+ yield(nil, fmt.Errorf("ByKeyPrefix is not supported by in-memory RegistersReader"))
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (r *RegistersReader) ByKeyPrefix(keyPrefix string, height uint64, cursor *flow.RegisterID) storage.IndexIterator[flow.RegisterValue, flow.RegisterID] { | |
| return func(yield func(storage.IteratorEntry[flow.RegisterValue, flow.RegisterID], error) bool) { | |
| yield(nil, fmt.Errorf("not implemented")) | |
| } | |
| func (r *RegistersReader) ByKeyPrefix(keyPrefix string, height uint64, cursor *flow.RegisterID) storage.IndexIterator[flow.RegisterValue, flow.RegisterID] { | |
| return func(yield func(storage.IteratorEntry[flow.RegisterValue, flow.RegisterID], error) bool) { | |
| yield(nil, fmt.Errorf("ByKeyPrefix is not supported by in-memory RegistersReader")) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@storage/inmemory/registers_reader.go` around lines 58 - 61, The ByKeyPrefix
stub in RegistersReader should return a clearer, descriptive error explaining
that range scans are not supported by the in-memory implementation; update
RegistersReader.ByKeyPrefix to call yield(nil, fmt.Errorf(...)) with a message
referencing the method name (ByKeyPrefix), the unsupported operation
(range/key-prefix scans), and include the keyPrefix and height parameters for
debugging (keep behavior matching that it fails immediately rather than
validating height like Get). Mention storage.IndexIterator, flow.RegisterID and
flow.RegisterValue so the change is applied to the existing iterator-returning
stub.
Comment on lines
+563
to
+572
| // collectRegistersByKey drains a ByKey iterator into a map keyed by register ID. | ||
| func collectRegistersByKey(t *testing.T, iter storage.IndexIterator[flow.RegisterValue, flow.RegisterID]) map[flow.RegisterID]flow.RegisterValue { | ||
| t.Helper() | ||
| results := make(map[flow.RegisterID]flow.RegisterValue) | ||
| for entry, err := range iter { | ||
| require.NoError(t, err) | ||
| val, err := entry.Value() | ||
| require.NoError(t, err) | ||
| results[entry.Cursor()] = val | ||
| } |
Contributor
There was a problem hiding this comment.
Assert on duplicate cursors in collectRegistersByKey.
results[entry.Cursor()] = val silently overwrites repeated keys, so the new “one entry per owner/register” tests still pass if ByKeyPrefix emits the same register twice. Add a duplicate check before assignment so the helper verifies uniqueness as well as values.
🔍 Suggested tightening
func collectRegistersByKey(t *testing.T, iter storage.IndexIterator[flow.RegisterValue, flow.RegisterID]) map[flow.RegisterID]flow.RegisterValue {
t.Helper()
results := make(map[flow.RegisterID]flow.RegisterValue)
for entry, err := range iter {
require.NoError(t, err)
val, err := entry.Value()
require.NoError(t, err)
- results[entry.Cursor()] = val
+ cursor := entry.Cursor()
+ _, duplicate := results[cursor]
+ require.Falsef(t, duplicate, "duplicate register yielded: %v", cursor)
+ results[cursor] = val
}
return results
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@storage/pebble/registers_test.go` around lines 563 - 572, In
collectRegistersByKey, guard against silently overwriting duplicate cursors by
checking whether results already contains entry.Cursor() before assigning; if a
cursor already exists, call require.Failf (or require.Fatalf/require.False with
a clear message) to fail the test and include the duplicate cursor and both
values, otherwise assign results[entry.Cursor()] = val—this ensures the helper
asserts uniqueness as well as value correctness for the ByKey iterator.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
module/state_synchronization/indexer/extended/contracts_loader.go (1)
189-191: Consider validating thatcontractNameis non-empty.
flow.KeyContractNamereturns an empty string if the key doesn't start withCodeKeyPrefix. While the iterator filters byCodeKeyPrefix, an explicit check would provide defense-in-depth and a clearer error message. The downstreamstoreAllContractDeploymentswill reject empty names, but failing earlier aids debugging.Proposed fix
address := flow.BytesToAddress([]byte(reg.Owner)) contractName := flow.KeyContractName(reg.Key) + if contractName == "" { + return access.ContractDeployment{}, false, fmt.Errorf("register key %q yielded empty contract name", reg.Key) + } contractID := access.ContractID(address, contractName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/contracts_loader.go` around lines 189 - 191, Validate that contractName (result of flow.KeyContractName(reg.Key)) is non-empty before constructing contractID and calling storeAllContractDeployments; if empty, return or skip with a clear error/log mentioning the offending key/owner (e.g., include reg.Key or reg.Owner) so callers get an early, descriptive failure instead of relying on downstream rejection. Ensure you check contractName right after computing it (before using address, contractID, or calling storeAllContractDeployments) and handle the empty-case consistently with surrounding error handling conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@module/state_synchronization/indexer/extended/contracts_loader.go`:
- Line 62: The log field name is misleading: replace Str("duration_ms",
time.Since(start).String()) with either a correctly typed millisecond value or a
matching name; for a numeric ms field use Int64("duration_ms",
time.Since(start).Milliseconds()) (or the logger's numeric helper) so the value
is an integer milliseconds, or rename the field to "duration" (or
"duration_str") if you keep the human-readable time.Since(start).String().
In `@storage/indexes/contracts.go`:
- Around line 408-416: The function contractDeploymentKeyPrefix is computing the
slice end incorrectly due to operator precedence—change the suffix subtraction
so the returned prefix strips the 16-byte suffix (heightLen + txIndexLen +
eventIndexLen) rather than effectively adding; update the return in
contractDeploymentKeyPrefix to subtract the sum of heightLen, txIndexLen and
eventIndexLen (e.g., use len(key) - (heightLen + txIndexLen + eventIndexLen)) so
BuildPrefixIterator gets the correct contract-only prefix; keep the existing
checks using minValidKeyLen and codeContractDeployment unchanged.
---
Nitpick comments:
In `@module/state_synchronization/indexer/extended/contracts_loader.go`:
- Around line 189-191: Validate that contractName (result of
flow.KeyContractName(reg.Key)) is non-empty before constructing contractID and
calling storeAllContractDeployments; if empty, return or skip with a clear
error/log mentioning the offending key/owner (e.g., include reg.Key or
reg.Owner) so callers get an early, descriptive failure instead of relying on
downstream rejection. Ensure you check contractName right after computing it
(before using address, contractID, or calling storeAllContractDeployments) and
handle the empty-case consistently with surrounding error handling conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6c0845e-c356-40f6-9967-e64049af2941
📒 Files selected for processing (10)
access/backends/extended/backend_contracts.goaccess/backends/extended/backend_contracts_test.goaccess/backends/extended/backend_scheduled_transactions_test.goengine/access/rest/experimental/request/cursor_contracts.gofvm/systemcontracts/system_contracts.gomodule/state_synchronization/indexer/extended/contracts_loader.gostorage/indexes/contracts.gostorage/indexes/contracts_test.gostorage/indexes/iterator/iterator.gostorage/indexes/iterator/iterator_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- engine/access/rest/experimental/request/cursor_contracts.go
| Int("contracts", len(deployments)). | ||
| Int("skipped_updated_in_block", len(seenContracts)). | ||
| Int("deleted", deletedCount). | ||
| Str("duration_ms", time.Since(start).String()). |
Contributor
There was a problem hiding this comment.
Misleading log field name: duration_ms contains a formatted string, not milliseconds.
time.Since(start).String() returns a human-readable string (e.g., "1.234567s"), but the field name suggests a numeric milliseconds value. This can cause confusion when parsing logs or building dashboards.
Proposed fix
- Str("duration_ms", time.Since(start).String()).
+ Dur("duration", time.Since(start)).Or if you specifically want milliseconds:
- Str("duration_ms", time.Since(start).String()).
+ Int64("duration_ms", time.Since(start).Milliseconds()).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Str("duration_ms", time.Since(start).String()). | |
| Dur("duration", time.Since(start)). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@module/state_synchronization/indexer/extended/contracts_loader.go` at line
62, The log field name is misleading: replace Str("duration_ms",
time.Since(start).String()) with either a correctly typed millisecond value or a
matching name; for a numeric ms field use Int64("duration_ms",
time.Since(start).Milliseconds()) (or the logger's numeric helper) so the value
is an integer milliseconds, or rename the field to "duration" (or
"duration_str") if you keep the human-readable time.Since(start).String().
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
storage/indexes/contracts.go (1)
276-285:⚠️ Potential issue | 🟠 MajorReject cursors for a different account in
ByAddress.Line 284 rebuilds the start key from
cursor.Addressinstead of the method'saccount. A malformed cursor can therefore steer pagination outside the requested address filter instead of failing fast.Possible fix
func (idx *ContractDeploymentsIndex) rangeKeysByAddress(account flow.Address, cursor *access.ContractDeploymentsCursor) (startKey, endKey []byte, err error) { prefix := makeContractDeploymentAddressPrefix(account) if cursor == nil || cursor.ContractName == "" { // by default, iterate over all contracts for the address return prefix, prefix, nil } - startKey = makeContractDeploymentContractPrefix(cursor.Address, cursor.ContractName) + if cursor.Address != account { + return nil, nil, fmt.Errorf("cursor address %s does not match requested account %s", cursor.Address.Hex(), account.Hex()) + } + startKey = makeContractDeploymentContractPrefix(account, cursor.ContractName) endKey = storage.PrefixInclusiveEnd(prefix, startKey) return startKey, endKey, nil }As per coding guidelines, "Treat all inputs as potentially byzantine and classify errors in a context-dependent manner; no code path is safe unless explicitly proven and documented."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/indexes/contracts.go` around lines 276 - 285, In rangeKeysByAddress (type ContractDeploymentsIndex) ensure we reject cursors that reference a different account: if cursor != nil and cursor.Address is non-empty and does not equal the method's account, return an error; then build startKey using the provided account (not cursor.Address) via makeContractDeploymentContractPrefix(account, cursor.ContractName) before computing endKey with storage.PrefixInclusiveEnd(prefix, startKey). This prevents a malformed cursor from steering pagination outside the requested address.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@module/state_synchronization/indexer/extended/contracts_loader.go`:
- Around line 105-112: The current loop only records addresses in
loadedContracts when isNew is true, so addresses that were scanned but skipped
due to seenContracts never get their name registers validated; add a separate
touchedAddresses set (e.g., map[string]struct{}) inside the same scan loop and
mark touchedAddresses[deployment.Address] for every iteration (both when isNew
and when skipped), then change the post-pass validation that iterates over
loadedContracts (the validation logic around loadedContracts and seenContracts
at lines ~135-167) to iterate over the union represented by touchedAddresses so
every touched address gets its names-register check regardless of whether a new
placeholder was created; update references to loadedContracts lookups to handle
addresses that may not exist in loadedContracts (treat missing entries as empty)
during that validation pass.
- Around line 185-191: Validate the owner string is a properly formatted Flow
address and ensure the derived contract name is non-empty before calling
access.ContractID: decode reg.Owner (e.g., hex.DecodeString) and confirm the
decoded byte slice length matches the expected Flow address length (reject and
return an error if not), then call flow.BytesToAddress only on the validated
bytes; after that call flow.KeyContractName(reg.Key) and if the returned
contractName is empty, return an error instead of constructing the ContractID.
Use the existing symbols reg.Owner, flow.BytesToAddress, flow.KeyContractName,
and access.ContractID to locate and update the logic.
- Around line 96-103: Replace generic fmt.Errorf wrapping in contracts_loader.go
(including the call sites around parseContractRegister and other storage/state
checks) with explicit irrecoverable error types per the repo pattern:
create/return well-named errors (e.g., ErrStorageRead, ErrMalformedState,
ErrConsistencyViolation) or use the package's factory (irrecoverable.New* or
similar) and include contextual details (registerID, height, underlying err)
when constructing them; update calls in parseContractRegister and its callers to
return these explicit error values instead of fmt.Errorf so the bootstrap caller
can classify and handle irrecoverable failures.
In `@storage/indexes/contracts.go`:
- Around line 363-369: The txIndex and eventIndex need to be encoded in
descending order like height is: instead of writing txIndex and eventIndex
directly, write their one's-complement (bitwise NOT) so lexicographic ordering
makes later tx/event in the same block come first; update the
binary.BigEndian.PutUint32 calls that write txIndex and eventIndex (and the
identical occurrences around the second block at the other location) to pass
^txIndex and ^eventIndex (i.e., the uint32 bitwise inverse) before advancing
offsets, keeping the same offsets/lengths (txIndexLen, eventIndexLen).
---
Duplicate comments:
In `@storage/indexes/contracts.go`:
- Around line 276-285: In rangeKeysByAddress (type ContractDeploymentsIndex)
ensure we reject cursors that reference a different account: if cursor != nil
and cursor.Address is non-empty and does not equal the method's account, return
an error; then build startKey using the provided account (not cursor.Address)
via makeContractDeploymentContractPrefix(account, cursor.ContractName) before
computing endKey with storage.PrefixInclusiveEnd(prefix, startKey). This
prevents a malformed cursor from steering pagination outside the requested
address.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3dd8d9fa-42ff-4302-b4af-8398cd1ba2ae
📒 Files selected for processing (3)
module/state_synchronization/indexer/extended/contracts_loader.gostorage/indexes/contracts.gostorage/indexes/iterator/iterator_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- storage/indexes/iterator/iterator_test.go
Comment on lines
+96
to
+103
| return nil, fmt.Errorf("error scanning contract code registers: %w", err) | ||
| } | ||
|
|
||
| registerID := item.Cursor() | ||
| deployment, isNew, err := c.parseContractRegister(registerID, item.Value, seenContracts, height) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error processing contract: %w", err) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "contracts_loader.go" -type f 2>/dev/null | head -5Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Search for the file in common locations
find . -name "contracts_loader.go" -type f 2>/dev/null | head -20Repository: onflow/flow-go
Length of output: 125
🏁 Script executed:
# List files in the repository to understand structure
git ls-files | grep -E "(contracts_loader|irrecoverable)" | head -20Repository: onflow/flow-go
Length of output: 399
🏁 Script executed:
wc -l ./module/state_synchronization/indexer/extended/contracts_loader.goRepository: onflow/flow-go
Length of output: 129
🏁 Script executed:
sed -n '90,210p' ./module/state_synchronization/indexer/extended/contracts_loader.goRepository: onflow/flow-go
Length of output: 4165
🏁 Script executed:
cat -n ./module/irrecoverable/irrecoverable.goRepository: onflow/flow-go
Length of output: 4201
🏁 Script executed:
# Search for where deployedContractsLoader is instantiated
rg "deployedContractsLoader|ContractsLoader" --type go -A 3 -B 1 | head -60Repository: onflow/flow-go
Length of output: 3771
🏁 Script executed:
# Check for examples of irrecoverable.Throw usage
rg "irrecoverable\.Throw|Throw\(" --type go -B 2 -A 1 | head -80Repository: onflow/flow-go
Length of output: 5456
🏁 Script executed:
# Check if this component implements the Component interface
grep -n "interface\|type deployedContractsLoader\|func.*Component" ./module/state_synchronization/indexer/extended/contracts_loader.goRepository: onflow/flow-go
Length of output: 98
🏁 Script executed:
# Check the full contracts.go to see how errors are handled
sed -n '1,100p' ./module/state_synchronization/indexer/extended/contracts.go | head -80Repository: onflow/flow-go
Length of output: 3327
🏁 Script executed:
# Look at the extended_indexer to see how contracts loading is called
grep -n "deployedContractsLoader\|Load.*bootstrapHeight" ./module/state_synchronization/indexer/extended/extended_indexer.go -B 5 -A 5Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Check if deployedContractsLoader should be part of a component
wc -l ./module/state_synchronization/indexer/extended/contracts.go
sed -n '1,150p' ./module/state_synchronization/indexer/extended/contracts.goRepository: onflow/flow-go
Length of output: 6397
🏁 Script executed:
# Check if Contracts implements Component interface
grep -n "func.*Contracts.*Start\|func.*Contracts.*Stop\|func.*ReadyFunc\|func.*DoneFunc" ./module/state_synchronization/indexer/extended/contracts.goRepository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Check extended_indexer to see component structure
sed -n '1,100p' ./module/state_synchronization/indexer/extended/extended_indexer.goRepository: onflow/flow-go
Length of output: 3103
🏁 Script executed:
# Check the full contracts.go file for component-related methods
tail -150 ./module/state_synchronization/indexer/extended/contracts.goRepository: onflow/flow-go
Length of output: 5528
🏁 Script executed:
# Check the Indexer interface definition
grep -n "type Indexer\|type IndexerManager" ./module/state_synchronization/indexer/extended/extended_indexer.go -A 15Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Check how extended_indexer processes indexer errors
sed -n '80,200p' ./module/state_synchronization/indexer/extended/extended_indexer.goRepository: onflow/flow-go
Length of output: 3851
🏁 Script executed:
# Check if there are structured error types defined
rg "type.*Error\|var Err" ./module/state_synchronization/indexer/extended/ --type go -B 1Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Find Indexer interface definition
rg "type Indexer interface" ./module/state_synchronization/indexer/extended/ --type go -A 10Repository: onflow/flow-go
Length of output: 1118
🏁 Script executed:
# Find where IndexBlockData is called in ingestLoop
sed -n '200,400p' ./module/state_synchronization/indexer/extended/extended_indexer.goRepository: onflow/flow-go
Length of output: 7711
🏁 Script executed:
# Get full ingestLoop implementation
grep -n "func.*ingestLoop\|IndexBlockData" ./module/state_synchronization/indexer/extended/extended_indexer.go -A 50 | head -100Repository: onflow/flow-go
Length of output: 3669
🏁 Script executed:
# Look for custom error types in the indexer package
rg "var Err|type.*Error" ./module/state_synchronization/indexer/ --type go -B 1 -A 2Repository: onflow/flow-go
Length of output: 404
🏁 Script executed:
# Check if Contracts component stores or has access to SignalerContext
sed -n '1,80p' ./module/state_synchronization/indexer/extended/contracts.goRepository: onflow/flow-go
Length of output: 3327
🏁 Script executed:
# Check what component.Component provides and if SignalerContext is available
grep -n "component.Component\|Start\|Stop" ./module/component/component.go | head -30Repository: onflow/flow-go
Length of output: 1052
🏁 Script executed:
# Check how other utility helpers in the codebase handle errors during bootstrap
find ./module/state_synchronization/indexer/extended -name "*.go" -type f -exec grep -l "helper\|retriever\|scanner" {} \;Repository: onflow/flow-go
Length of output: 839
🏁 Script executed:
# Look at contractRetriever errors handling
sed -n '240,280p' ./module/state_synchronization/indexer/extended/contracts.goRepository: onflow/flow-go
Length of output: 1283
🏁 Script executed:
# Check if there are error wrapping patterns with context in utility functions
rg "func.*\(.*context\|.*ctx.*\)" ./module/state_synchronization/indexer/extended/ --type go -A 8 | head -80Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Look at how errors flow from IndexBlockData up through runIndexers
sed -n '290,330p' ./module/state_synchronization/indexer/extended/extended_indexer.goRepository: onflow/flow-go
Length of output: 1480
🏁 Script executed:
# Check if there are structured error types in the contracts module
rg "type.*Error|errors.New" ./module/state_synchronization/indexer/extended/ --type go -B 1 -A 1Repository: onflow/flow-go
Length of output: 993
🏁 Script executed:
# Look for example error classification patterns in the codebase
rg "irrecoverable\|Throw" ./module/ --type go -B 5 | grep -A 5 "type.*Error\|errors.New\|fmt.Errorf" | head -60Repository: onflow/flow-go
Length of output: 40
🏁 Script executed:
# Check how errors are handled at module/component level
grep -r "IndexBlockData" ./module/state_synchronization/indexer/extended/ --include="*.go" -B 2 -A 5 | head -80Repository: onflow/flow-go
Length of output: 7777
Use explicit error classification for bootstrap failures instead of wrapping with fmt.Errorf.
Lines 96–103, 138–144, 163, and 186–199 wrap storage-read failures, malformed state, and consistency violations into generic errors. These failures occur during bootstrap (when loading deployed contracts for the first time) and represent irrecoverable state problems that the caller must distinguish. Per the coding guidelines, use the repository's explicit error-classification pattern instead of fmt.Errorf: "Use the irrecoverable package for exception handling instead of fmt.Errorf; always explicitly handle errors and never log and continue on a best-effort basis."
Define structured error types for different failure classes (e.g., storage read failures, malformed state, consistency violations) and return these from the loader, allowing the caller to classify them appropriately when signaling irrecoverable state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@module/state_synchronization/indexer/extended/contracts_loader.go` around
lines 96 - 103, Replace generic fmt.Errorf wrapping in contracts_loader.go
(including the call sites around parseContractRegister and other storage/state
checks) with explicit irrecoverable error types per the repo pattern:
create/return well-named errors (e.g., ErrStorageRead, ErrMalformedState,
ErrConsistencyViolation) or use the package's factory (irrecoverable.New* or
similar) and include contextual details (registerID, height, underlying err)
when constructing them; update calls in parseContractRegister and its callers to
return these explicit error values instead of fmt.Errorf so the bootstrap caller
can classify and handle irrecoverable failures.
Comment on lines
+105
to
+112
| if isNew { | ||
| deployments = append(deployments, deployment) | ||
|
|
||
| if _, ok := loadedContracts[deployment.Address]; !ok { | ||
| loadedContracts[deployment.Address] = make(map[string]int) | ||
| } | ||
| loadedContracts[deployment.Address][deployment.ContractName] = len(deployments) - 1 | ||
| } |
Contributor
There was a problem hiding this comment.
Validate names registers for every touched address, not just newly loaded ones.
Lines 105-112 only record addresses that produced a new placeholder, and Line 135 verifies only that map. If an account’s contract registers are all skipped because they are already present in seenContracts, its names register is never checked. That leaves a gap where an extra registered name with no corresponding code register is silently missed during bootstrap. Track touched addresses separately during the scan and run the post-pass validation over that union. As per coding guidelines, "Treat all inputs as potentially byzantine and classify errors in a context-dependent manner; no code path is safe unless explicitly proven and documented."
Also applies to: 135-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@module/state_synchronization/indexer/extended/contracts_loader.go` around
lines 105 - 112, The current loop only records addresses in loadedContracts when
isNew is true, so addresses that were scanned but skipped due to seenContracts
never get their name registers validated; add a separate touchedAddresses set
(e.g., map[string]struct{}) inside the same scan loop and mark
touchedAddresses[deployment.Address] for every iteration (both when isNew and
when skipped), then change the post-pass validation that iterates over
loadedContracts (the validation logic around loadedContracts and seenContracts
at lines ~135-167) to iterate over the union represented by touchedAddresses so
every touched address gets its names-register check regardless of whether a new
placeholder was created; update references to loadedContracts lookups to handle
addresses that may not exist in loadedContracts (treat missing entries as empty)
during that validation pass.
Comment on lines
+185
to
+191
| if reg.Owner == "" { | ||
| return access.ContractDeployment{}, false, fmt.Errorf("found contract with empty owner: %q", reg) | ||
| } | ||
|
|
||
| address := flow.BytesToAddress([]byte(reg.Owner)) | ||
| contractName := flow.KeyContractName(reg.Key) | ||
| contractID := access.ContractID(address, contractName) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 200 module/state_synchronization/indexer/extended/contracts_loader.go | tail -n 40Repository: onflow/flow-go
Length of output: 1390
🏁 Script executed:
cat -n module/state_synchronization/indexer/extended/contracts_loader.go | sed -n '175,210p'Repository: onflow/flow-go
Length of output: 1490
🏁 Script executed:
fd -t f -e go | xargs rg "func (|func )BytesToAddress" -A 5Repository: onflow/flow-go
Length of output: 344
🏁 Script executed:
fd -t f -e go | xargs rg "func (|func )KeyContractName" -A 5Repository: onflow/flow-go
Length of output: 303
🏁 Script executed:
cat -n module/state_synchronization/indexer/extended/contracts_loader.go | sed -n '1,50p'Repository: onflow/flow-go
Length of output: 2000
🏁 Script executed:
cat -n module/state_synchronization/indexer/extended/contracts_loader.go | sed -n '150,190p'Repository: onflow/flow-go
Length of output: 1800
🏁 Script executed:
cat -n model/flow/ledger.go | sed -n '1,100p'Repository: onflow/flow-go
Length of output: 3114
🏁 Script executed:
rg "IsContractKey" -A 3 model/flow/Repository: onflow/flow-go
Length of output: 389
🏁 Script executed:
rg "func ContractID" -A 5Repository: onflow/flow-go
Length of output: 504
🏁 Script executed:
cat -n model/access/contract_id.go 2>/dev/null || rg "func ContractID" -B 2 -A 10Repository: onflow/flow-go
Length of output: 1156
Validate owner format and require non-empty contract names before deriving contract identity.
Lines 189-191 derive address, contractName, and contractID with insufficient input validation. The owner is only checked for emptiness, but BytesToAddress silently truncates or pads malformed values without format validation. Additionally, KeyContractName can return an empty string if the key is not a valid contract key, and this is not validated before constructing the ContractID. Empty or malformed contract names will produce corrupted contract IDs (e.g., "A.{address}."), poisoning the bootstrap index with synthetic deployments. Validate the owner format, ensure the contract name is non-empty, and fail fast on invalid inputs, consistent with the requirement to "treat all inputs as potentially byzantine."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@module/state_synchronization/indexer/extended/contracts_loader.go` around
lines 185 - 191, Validate the owner string is a properly formatted Flow address
and ensure the derived contract name is non-empty before calling
access.ContractID: decode reg.Owner (e.g., hex.DecodeString) and confirm the
decoded byte slice length matches the expected Flow address length (reject and
return an error if not), then call flow.BytesToAddress only on the validated
bytes; after that call flow.KeyContractName(reg.Key) and if the returned
contractName is empty, return an error instead of constructing the ContractID.
Use the existing symbols reg.Owner, flow.BytesToAddress, flow.KeyContractName,
and access.ContractID to locate and update the logic.
Comment on lines
+363
to
+369
| binary.BigEndian.PutUint64(key[offset:], ^height) // one's complement for descending height order | ||
| offset += heightLen | ||
|
|
||
| binary.BigEndian.PutUint32(key[offset:], txIndex) | ||
| offset += txIndexLen | ||
|
|
||
| binary.BigEndian.PutUint32(key[offset:], eventIndex) |
Contributor
There was a problem hiding this comment.
Encode txIndex and eventIndex in descending order too.
These iterators treat the first key under a contract prefix as the “latest” deployment, but the current encoding only reverses height. If the same contract is updated multiple times in one block, lexicographic order will pick the earliest transaction/event in that block instead of the newest one.
Possible fix
func makeContractDeploymentKey(addr flow.Address, name string, height uint64, txIndex, eventIndex uint32) []byte {
@@
binary.BigEndian.PutUint64(key[offset:], ^height) // one's complement for descending height order
offset += heightLen
- binary.BigEndian.PutUint32(key[offset:], txIndex)
+ binary.BigEndian.PutUint32(key[offset:], ^txIndex)
offset += txIndexLen
- binary.BigEndian.PutUint32(key[offset:], eventIndex)
+ binary.BigEndian.PutUint32(key[offset:], ^eventIndex)
return key
}- txIndex := binary.BigEndian.Uint32(key[offset:])
+ txIndex := ^binary.BigEndian.Uint32(key[offset:])
offset += txIndexLen
- eventIndex := binary.BigEndian.Uint32(key[offset:])
+ eventIndex := ^binary.BigEndian.Uint32(key[offset:])Also applies to: 443-446
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@storage/indexes/contracts.go` around lines 363 - 369, The txIndex and
eventIndex need to be encoded in descending order like height is: instead of
writing txIndex and eventIndex directly, write their one's-complement (bitwise
NOT) so lexicographic ordering makes later tx/event in the same block come
first; update the binary.BigEndian.PutUint32 calls that write txIndex and
eventIndex (and the identical occurrences around the second block at the other
location) to pass ^txIndex and ^eventIndex (i.e., the uint32 bitwise inverse)
before advancing offsets, keeping the same offsets/lengths (txIndexLen,
eventIndexLen).
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
module/state_synchronization/indexer/extended/scheduled_transaction_requester.go (1)
57-89:⚠️ Potential issue | 🟠 MajorReject duplicate or unexpected metadata IDs explicitly.
After the metadata refactor, the final
len(updatedTxs) == len(missingTxs)check is not strong enough. Ifmetacontains two updates for one scheduled transaction and omits another, or includes an ID that was never requested, this code can still return the wrong result set without surfacing the inconsistency. Track updated IDs in a set and fail on duplicates/unexpected IDs instead of validating cardinality only.As per coding guidelines, "Treat all inputs as potentially byzantine and classify errors in a context-dependent manner; no code path is safe unless explicitly proven and documented."Suggested hardening
updatedTxs := make([]access.ScheduledTransaction, 0, len(missingTxs)) + updatedIDs := make(map[uint64]struct{}, len(missingTxs)) + appendUpdated := func(id uint64, tx access.ScheduledTransaction) error { + if _, ok := updatedIDs[id]; ok { + return fmt.Errorf("duplicate scheduled transaction update for id %d", id) + } + updatedIDs[id] = struct{}{} + updatedTxs = append(updatedTxs, tx) + return nil + } for _, entry := range meta.ExecutedEntries { - if missing, ok := missingTxs[entry.event.ID]; ok { + missing, ok := missingTxs[entry.event.ID] + if !ok { + return nil, fmt.Errorf("unexpected executed update for scheduled transaction %d", entry.event.ID) + } // set IsPlaceholder = true to signal that some information is missing because we don't know the original transaction. missing.IsPlaceholder = true missing.Status = access.ScheduledTxStatusExecuted missing.ExecutedTransactionID = entry.transactionID - updatedTxs = append(updatedTxs, missing) - } + if err := appendUpdated(entry.event.ID, missing); err != nil { + return nil, err + } } + // Apply the same unexpected-ID / duplicate-ID check to CanceledEntries and FailedEntries. - if len(updatedTxs) != len(missingTxs) { + if len(updatedIDs) != len(missingTxs) { return nil, fmt.Errorf("expected %d updated scheduled transactions, got %d", len(missingTxs), len(updatedTxs)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/scheduled_transaction_requester.go` around lines 57 - 89, The loops over meta.ExecutedEntries, meta.CanceledEntries, and meta.FailedEntries must explicitly detect duplicate or unexpected IDs instead of only comparing len(updatedTxs) to len(missingTxs); add a seenIDs map[string]bool (or appropriate key type) and when you look up missingTxs (e.g. in the three loops that set missing.IsPlaceholder and append to updatedTxs) check if the id is already in seenIDs and return an error for duplicate metadata; also if an id from meta is not present in missingTxs return an error for unexpected metadata; finally ensure you still verify seenIDs covers exactly the missingTxs keys (or compare lengths) before returning updatedTxs so duplicates/unexpected IDs fail fast and deterministically.
🧹 Nitpick comments (4)
module/state_synchronization/indexer/extended/contracts.go (1)
176-240: Consider extracting common event processing logic.The
AccountContractAdded(lines 176-207) andAccountContractUpdated(lines 209-240) cases share nearly identical logic:
- Decode event payload
- Decode specific event type
- Fetch contract code via retriever
- Validate code hash
- Append deployment with metadata
A helper function could reduce duplication while maintaining clarity.
♻️ Suggested refactor to reduce duplication
func (c *Contracts) processContractEvent( event flow.Event, retriever *contractRetriever, data BlockData, isUpdate bool, ) (*access.ContractDeployment, error) { cadenceEvent, err := events.DecodePayload(event) if err != nil { return nil, fmt.Errorf("failed to decode %s event payload: %w", event.Type, err) } var address flow.Address var contractName string var codeHash []byte if isUpdate { e, err := events.DecodeAccountContractUpdated(cadenceEvent) if err != nil { return nil, fmt.Errorf("failed to decode %s event: %w", event.Type, err) } address, contractName, codeHash = e.Address, e.ContractName, e.CodeHash } else { e, err := events.DecodeAccountContractAdded(cadenceEvent) if err != nil { return nil, fmt.Errorf("failed to decode %s event: %w", event.Type, err) } address, contractName, codeHash = e.Address, e.ContractName, e.CodeHash } code, err := retriever.contractCode(address, contractName, data.Header.Height) if err != nil { return nil, fmt.Errorf("failed to get contract code: %w", err) } if !bytes.Equal(codeHash, access.CadenceCodeHash(code)) { return nil, fmt.Errorf("code hash mismatch for %s event: %s", event.Type, contractName) } return &access.ContractDeployment{ ContractName: contractName, Address: address, BlockHeight: data.Header.Height, TransactionID: event.TransactionID, TransactionIndex: event.TransactionIndex, EventIndex: event.EventIndex, Code: code, CodeHash: codeHash, }, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/contracts.go` around lines 176 - 240, The two switch cases for flowAccountContractAdded and flowAccountContractUpdated duplicate decoding, code retrieval, hash validation, and deployment construction; extract this into a helper like processContractEvent (callable from Contracts) which takes the flow.Event, retriever *contractRetriever, data BlockData and a flag or decoder selector, performs events.DecodePayload + events.DecodeAccountContractAdded/Updated, calls retriever.contractCode, verifies access.CadenceCodeHash, returns *access.ContractDeployment (or error); replace both case bodies to call processContractEvent and then append the returned deployment and increment created/updated respectively.module/state_synchronization/indexer/extended/account_ft_transfers.go (1)
91-116: Make the newFilteredCountobservable.
ProcessBlockDatanow computes how many transfers were dropped, butIndexBlockDatathrows that metadata away and only records the kept count. If filtering suddenly spikes because of a parser regression or a chain-side change, operators will only see indexed volume fall. Consider surfacingFilteredCountvia metrics or structured logs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/account_ft_transfers.go` around lines 91 - 116, IndexBlockData currently discards the FungibleTokenTransfersMetadata returned by ProcessBlockData so the FilteredCount is not observable; update IndexBlockData in the FungibleTokenTransfers flow to capture the metadata (the second return value of ProcessBlockData) and export FilteredCount via your observability path—e.g., increment a new metric like metrics.FTTransferFiltered(metadata.FilteredCount) and/or emit a structured log including metadata.FilteredCount alongside the existing metrics.FTTransferIndexed(len(ftEntries)) after ftStore.Store succeeds; make sure to reference ProcessBlockData, FungibleTokenTransfersMetadata.FilteredCount, IndexBlockData, and metrics.FTTransferIndexed when adding the metric/log so the dropped-count becomes visible.module/state_synchronization/indexer/extended/account_transactions_test.go (1)
643-663: Assert the fullAccountTransactionpayload in this happy-path case.This only pins
AddressandTransactionID. A regression inRoles,BlockHeight, orTransactionIndexwould still pass even thoughProcessBlockDatanow exposes those fields directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/account_transactions_test.go` around lines 643 - 663, The test only checks Address and TransactionID but should assert the full AccountTransaction payload; update the test in the t.Run block to build the expected AccountTransaction (using payer, tx.ID(), testHeight, transaction index 0 and the expected Roles for the created tx) and replace the partial asserts with an equality/assert deep-equal between entries[0] and that expected struct (locate the test in account_transactions_test.go, the t.Run "returns correct entries for single transaction", and the call to indexer.ProcessBlockData / entries[0] to make the change).module/state_synchronization/indexer/extended/account_transactions.go (1)
125-204: MakeProcessBlockDatareturn a deterministic order.The returned slice is assembled from a map iteration, so callers can observe a different entry order across runs. That was mostly hidden when this logic only wrote to storage, but the new processor API is now directly consumed and tested. Sorting by
TransactionIndexand address before returning would make the contract stable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@module/state_synchronization/indexer/extended/account_transactions.go` around lines 125 - 204, ProcessBlockData builds entries via map iteration which yields nondeterministic order; before the final return in AccountTransactions.ProcessBlockData, sort the entries slice deterministically by TransactionIndex (ascending) and then by Address (ascending) so callers always get a stable order. Locate the entries variable in ProcessBlockData and apply a stable sort (e.g., sort.Slice or slices.SortFunc) comparing entry.TransactionIndex first and, when equal, comparing entry.Address (string/bytes) to break ties, then return the sorted entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@module/state_synchronization/indexer/extended/scheduled_transaction_requester.go`:
- Around line 57-89: The loops over meta.ExecutedEntries, meta.CanceledEntries,
and meta.FailedEntries must explicitly detect duplicate or unexpected IDs
instead of only comparing len(updatedTxs) to len(missingTxs); add a seenIDs
map[string]bool (or appropriate key type) and when you look up missingTxs (e.g.
in the three loops that set missing.IsPlaceholder and append to updatedTxs)
check if the id is already in seenIDs and return an error for duplicate
metadata; also if an id from meta is not present in missingTxs return an error
for unexpected metadata; finally ensure you still verify seenIDs covers exactly
the missingTxs keys (or compare lengths) before returning updatedTxs so
duplicates/unexpected IDs fail fast and deterministically.
---
Nitpick comments:
In `@module/state_synchronization/indexer/extended/account_ft_transfers.go`:
- Around line 91-116: IndexBlockData currently discards the
FungibleTokenTransfersMetadata returned by ProcessBlockData so the FilteredCount
is not observable; update IndexBlockData in the FungibleTokenTransfers flow to
capture the metadata (the second return value of ProcessBlockData) and export
FilteredCount via your observability path—e.g., increment a new metric like
metrics.FTTransferFiltered(metadata.FilteredCount) and/or emit a structured log
including metadata.FilteredCount alongside the existing
metrics.FTTransferIndexed(len(ftEntries)) after ftStore.Store succeeds; make
sure to reference ProcessBlockData,
FungibleTokenTransfersMetadata.FilteredCount, IndexBlockData, and
metrics.FTTransferIndexed when adding the metric/log so the dropped-count
becomes visible.
In `@module/state_synchronization/indexer/extended/account_transactions_test.go`:
- Around line 643-663: The test only checks Address and TransactionID but should
assert the full AccountTransaction payload; update the test in the t.Run block
to build the expected AccountTransaction (using payer, tx.ID(), testHeight,
transaction index 0 and the expected Roles for the created tx) and replace the
partial asserts with an equality/assert deep-equal between entries[0] and that
expected struct (locate the test in account_transactions_test.go, the t.Run
"returns correct entries for single transaction", and the call to
indexer.ProcessBlockData / entries[0] to make the change).
In `@module/state_synchronization/indexer/extended/account_transactions.go`:
- Around line 125-204: ProcessBlockData builds entries via map iteration which
yields nondeterministic order; before the final return in
AccountTransactions.ProcessBlockData, sort the entries slice deterministically
by TransactionIndex (ascending) and then by Address (ascending) so callers
always get a stable order. Locate the entries variable in ProcessBlockData and
apply a stable sort (e.g., sort.Slice or slices.SortFunc) comparing
entry.TransactionIndex first and, when equal, comparing entry.Address
(string/bytes) to break ties, then return the sorted entries.
In `@module/state_synchronization/indexer/extended/contracts.go`:
- Around line 176-240: The two switch cases for flowAccountContractAdded and
flowAccountContractUpdated duplicate decoding, code retrieval, hash validation,
and deployment construction; extract this into a helper like
processContractEvent (callable from Contracts) which takes the flow.Event,
retriever *contractRetriever, data BlockData and a flag or decoder selector,
performs events.DecodePayload + events.DecodeAccountContractAdded/Updated, calls
retriever.contractCode, verifies access.CadenceCodeHash, returns
*access.ContractDeployment (or error); replace both case bodies to call
processContractEvent and then append the returned deployment and increment
created/updated respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d55c3cc5-1a4e-44bb-b188-3e6f0332a687
📒 Files selected for processing (13)
module/state_synchronization/indexer/extended/account_ft_transfers.gomodule/state_synchronization/indexer/extended/account_ft_transfers_test.gomodule/state_synchronization/indexer/extended/account_nft_transfers.gomodule/state_synchronization/indexer/extended/account_nft_transfers_test.gomodule/state_synchronization/indexer/extended/account_transactions.gomodule/state_synchronization/indexer/extended/account_transactions_test.gomodule/state_synchronization/indexer/extended/contracts.gomodule/state_synchronization/indexer/extended/contracts_test.gomodule/state_synchronization/indexer/extended/indexer.gomodule/state_synchronization/indexer/extended/scheduled_transaction_requester.gomodule/state_synchronization/indexer/extended/scheduled_transaction_requester_test.gomodule/state_synchronization/indexer/extended/scheduled_transactions.gomodule/state_synchronization/indexer/extended/scheduled_transactions_test.go
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
4 participants
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.
Summary by CodeRabbit
New Features
Improvements