Run Go modernize tool to simplify code-base #844
Conversation
m-Peter
requested review from
janezpodhostnik,
peterargue and
zhangchiqing
as code owners
WalkthroughThis change replaces all uses of the empty interface type Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Blockchain
Client->>API: Call Syncing()
API->>Blockchain: Query sync status
Blockchain-->>API: Return status (any)
API-->>Client: Return status (any)
Client->>API: Call GetTransactionReceipt(hash)
API->>Blockchain: Query receipt for hash
Blockchain-->>API: Return receipt (map[string]any)
API-->>Client: Return receipt (map[string]any)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
api/api.go(4 hunks)api/server.go(1 hunks)bootstrap/bootstrap.go(1 hunks)bootstrap/create-multi-key-account.go(1 hunks)eth/types/types.go(2 hunks)models/errors/errors.go(1 hunks)models/events_test.go(7 hunks)models/stream_test.go(3 hunks)services/ingestion/engine_test.go(1 hunks)services/ingestion/event_subscriber.go(1 hunks)services/ingestion/event_subscriber_test.go(2 hunks)services/requester/requester.go(1 hunks)storage/index_test.go(6 hunks)storage/mocks/mocks.go(2 hunks)storage/pebble/storage_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
storage/pebble/storage_test.go (2)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: services/traces/engine.go:190-190
Timestamp: 2024-10-17T18:04:41.062Z
Learning: In `services/traces/engine.go`, the `e.indexBlockTraces` function limits the number of goroutines to a maximum of 5 and blocks until all are complete, so during backfill, concurrency is already managed to prevent resource exhaustion.
bootstrap/create-multi-key-account.go (4)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/pool.go:202-214
Timestamp: 2025-06-17T10:29:35.941Z
Learning: In Flow EVM Gateway's transaction pool (services/requester/pool.go), signing keys acquired with keystore.Take() are intentionally held until the transaction is sealed, not released immediately after sending. The key release happens asynchronously when sealed transactions are ingested via event_subscriber.go NotifyTransaction() -> keystore unsafeUnlockKey() -> key.Done(). This pattern ensures transaction integrity by preventing key reuse before transaction finalization.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/pool.go:202-214
Timestamp: 2025-06-17T10:29:35.941Z
Learning: In Flow EVM Gateway's transaction pool (services/requester/pool.go), signing keys acquired with keystore.Take() are intentionally held until the transaction is sealed, not released immediately after sending. The key release happens asynchronously when sealed transactions are ingested via event_subscriber.go NotifyTransaction() -> keystore unsafeUnlockKey() -> key.Done(). This pattern ensures transaction integrity by preventing key reuse before transaction finalization.
storage/index_test.go (4)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#617
File: api/stream.go:62-67
Timestamp: 2024-10-18T19:26:37.579Z
Learning: In the `flow-evm-gateway` project, within the Go file `api/stream.go`, the `prepareBlockResponse` method includes the Bloom filter as the field `LogsBloom` in the returned `Block` struct.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: services/traces/engine.go:190-190
Timestamp: 2024-10-17T18:04:41.062Z
Learning: In `services/traces/engine.go`, the `e.indexBlockTraces` function limits the number of goroutines to a maximum of 5 and blocks until all are complete, so during backfill, concurrency is already managed to prevent resource exhaustion.
services/ingestion/engine_test.go (3)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: services/traces/engine.go:190-190
Timestamp: 2024-10-17T18:04:41.062Z
Learning: In `services/traces/engine.go`, the `e.indexBlockTraces` function limits the number of goroutines to a maximum of 5 and blocks until all are complete, so during backfill, concurrency is already managed to prevent resource exhaustion.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
services/ingestion/event_subscriber_test.go (3)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: bootstrap/bootstrap.go:167-197
Timestamp: 2024-10-17T18:04:04.165Z
Learning: In the `flow-evm-gateway` Go project, the validation ensuring that `startHeight` is less than or equal to `endHeight` is performed before the `StartTraceDownloader` method in `bootstrap/bootstrap.go`, so additional checks in this method are unnecessary.
models/events_test.go (3)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: services/requester/batch_tx_pool.go:0-0
Timestamp: 2025-06-19T11:36:25.478Z
Learning: In Go, when copying maps that contain slices (like `map[gethCommon.Address][]pooledEvmTx`), perform deep copies by iterating over the map and copying each slice individually using `make()` and `copy()` to avoid shared references that could lead to race conditions and data corruption.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#617
File: api/stream.go:62-67
Timestamp: 2024-10-18T19:26:37.579Z
Learning: In the `flow-evm-gateway` project, within the Go file `api/stream.go`, the `prepareBlockResponse` method includes the Bloom filter as the field `LogsBloom` in the returned `Block` struct.
services/ingestion/event_subscriber.go (4)
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: bootstrap/bootstrap.go:167-197
Timestamp: 2024-10-17T18:04:04.165Z
Learning: In the `flow-evm-gateway` Go project, the validation ensuring that `startHeight` is less than or equal to `endHeight` is performed before the `StartTraceDownloader` method in `bootstrap/bootstrap.go`, so additional checks in this method are unnecessary.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#682
File: services/ingestion/event_subscriber.go:85-90
Timestamp: 2025-01-24T20:15:10.908Z
Learning: When calculating blocks to catch up in Flow, initialize the counter to 0 and only set it when the latest chain height is greater than the current height (latestOnChainHeight > currentHeight). There's no need to handle the else case as 0 blocks to catch up is the correct default when we're at the same height or ahead.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: services/traces/engine.go:175-178
Timestamp: 2024-10-17T18:05:27.084Z
Learning: In the trace backfill process, returning upon encountering an error when retrieving a block or its Cadence ID is intentional because future blocks are unlikely to exist if these blocks don't.
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: services/traces/engine.go:190-190
Timestamp: 2024-10-17T18:04:41.062Z
Learning: In `services/traces/engine.go`, the `e.indexBlockTraces` function limits the number of goroutines to a maximum of 5 and blocks until all are complete, so during backfill, concurrency is already managed to prevent resource exhaustion.
models/stream_test.go (1)
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#831
File: tests/tx_batching_test.go:95-104
Timestamp: 2025-06-17T10:22:19.395Z
Learning: Go 1.22 introduced the ability to use `range` directly on integer values. The syntax `for i := range n` where `n` is an integer will iterate from 0 to n-1, equivalent to `for i := 0; i < n; i++`. This is valid Go syntax and should not be flagged as an error.
services/requester/requester.go (1)
Learnt from: peterargue
PR: onflow/flow-evm-gateway#615
File: bootstrap/bootstrap.go:167-197
Timestamp: 2024-10-17T18:04:04.165Z
Learning: In the `flow-evm-gateway` Go project, the validation ensuring that `startHeight` is less than or equal to `endHeight` is performed before the `StartTraceDownloader` method in `bootstrap/bootstrap.go`, so additional checks in this method are unnecessary.
🧬 Code Graph Analysis (1)
api/api.go (1)
eth/types/types.go (1)
MarshalReceipt(504-560)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Lint
🔇 Additional comments (24)
storage/pebble/storage_test.go (1)
144-144: LGTM! Modern Go loop syntax.The range-over-integer syntax is a clean modernization that maintains the same iteration behavior while improving readability.
storage/index_test.go (1)
154-154: LGTM! Consistent loop modernization.All loop changes use Go 1.22's range-over-integer syntax correctly, improving code readability while maintaining identical iteration behavior across the test suite.
Also applies to: 178-178, 224-224, 430-430, 625-625, 673-673
bootstrap/create-multi-key-account.go (1)
84-84: LGTM! Valid Go 1.22+ range syntax.Using
rangeon integer variables is supported in Go 1.22+, as confirmed by the language specification. This modernization maintains the same iteration behavior while improving code readability.models/stream_test.go (1)
79-79: LGTM! Clean test modernization.The range-over-integer syntax simplifies the concurrent test loops while maintaining identical iteration behavior and test logic.
Also applies to: 83-83, 93-93, 102-102
services/ingestion/engine_test.go (1)
499-499: LGTM! Consistent modernization pattern.The range-over-integer syntax with the
eventCountvariable is valid Go 1.22+ syntax and maintains the same iteration behavior for the transaction creation test logic.services/ingestion/event_subscriber_test.go (2)
210-210: LGTM! Modern Go range syntax over slice.The change to use
for i := range txHashesis correct and more idiomatic than the traditional index-based loop.
355-355: LGTM! Modern Go 1.22+ range syntax over integer.The change to use
for i := range txCountis correct and takes advantage of Go 1.22's ability to range directly over integer values, which is equivalent tofor i := 0; i < txCount; i++.models/events_test.go (7)
72-72: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and leverages Go 1.22's modern range syntax.
249-249: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and more concise than the traditional index-based loop.
318-318: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and follows Go 1.22+ idioms.
341-341: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and improves code readability.
391-391: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and consistent with the modernization effort.
511-511: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and follows modern Go conventions.
538-538: LGTM! Modern Go range syntax over integer.The change to use
for i := range txCountis correct and completes the consistent modernization of loop syntax throughout the test file.models/errors/errors.go (1)
95-95: LGTM: Modern Go type alias usageGood modernization change replacing
interface{}withanyfor improved readability. This aligns with Go 1.18+ best practices.bootstrap/bootstrap.go (1)
534-534: LGTM: Consistent type modernizationProper use of
anytype alias for gRPC interceptor parameters, maintaining interface compliance while improving readability.services/ingestion/event_subscriber.go (1)
275-275: LGTM: Simplified boundary calculationExcellent use of the
minfunction to simplify the endHeight calculation. This is more concise and readable than the previous conditional approach while maintaining the same logic.storage/mocks/mocks.go (1)
51-51: LGTM: Efficient byte slice creationGood modernization using
fmt.Appendf(nil, ...)which directly returns a byte slice, avoiding the intermediate string allocation fromfmt.Sprintf().Also applies to: 61-61
services/requester/requester.go (1)
403-407: LGTM: Streamlined binary search logicExcellent simplification of the binary search midpoint calculation using
min(). This maintains the same gas estimation logic while improving code clarity and consistency with other modernization changes in the codebase.eth/types/types.go (2)
471-472: Idiomatic alias looks goodReplacing
interface{}with theanyalias is fully idiomatic since Go 1.18 and keeps the existing polymorphic semantics of theTransactionsfield unchanged. 👍
507-515: Consistent return-type modernisationThe function signature and composite literals were updated to
map[string]any. All call-sites shown in this PR already compile against the new type, so no further action is required.api/api.go (2)
137-160: Return-type switch is harmless
Syncingnow returnsany, matching the RPC expectation offalse | object. All internal callers already treat the value opaquely, so behaviour is preserved.
325-350: Receipt path compiles afteranyswitch
GetTransactionReceiptwas adapted to the newmap[string]anyreturn type and allhandleErrorsites were updated accordingly—good consistency.api/server.go (1)
439-442: Struct field modernised correctlySwitching
jsonError.Datatoanymirrors the Go standard and aligns with other changes in the PR. No behavioural change introduced.
Description
Run the modernize analyzer to
simplify code by using modern constructs.For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
Refactor
anyalias throughout the codebase for improved clarity and consistency.Tests