Skip to content

Conversation

@m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Oct 23, 2025

Description

This is causing syncing from the EVM Genesis block to fail, because the previous value of MainnetInitCadenceHeight, actually belonged to mainnet24.

Also updated the README with proper configuration for testnet/mainnet, for anyone interested in syncing from the EVM genesis block.

Removed some references of init-cadence-height, as we want to deprecate this config flag.


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Summary by CodeRabbit

  • Documentation

    • Reorganized and clarified headings; expanded Run-from-CLI and Run-with-Docker sections. Replaced concrete COA and height values with placeholders, added a mainnet example, and clarified API and unsupported API lists (including debug_traceCall).
  • Chores

    • Enabled local emulator access and COA transaction lookup; normalized COA flags. Removed explicit init-cadence-height usage and bumped the mainnet initialization height.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

Removed INIT_CADENCE_HEIGHT usage, added emulator gRPC host and --coa-tx-lookup-enabled=true to emulator args, normalized COA flag spacing, bumped MainnetInitCadenceHeight by 1, and reorganized README with expanded CLI/Docker examples and COA placeholders.

Changes

Cohort / File(s) Summary
Makefile / Emulator args
Makefile
Added --access-node-grpc-host=localhost:3569 to EMULATOR_ARGS, appended --coa-tx-lookup-enabled=true, normalized spacing around --coa-address/--coa-key, and removed INIT_CADENCE_HEIGHT from docker-run assembly, checks, and help text.
Documentation
README.md
Converted bold sections to structured headings, reorganized Run/Verify sections, replaced explicit cadence/COA values with placeholders, updated CLI and Docker examples for Testnet/Mainnet, and added/clarified API lists (including debug_traceCall) and unsupported API notes.
Configuration
config/config.go
Incremented MainnetInitCadenceHeight from 85981134 to 85981135 and added a reference comment.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer / Makefile
  participant EM as Local Emulator
  participant GW as EVM Gateway

  rect rgb(240,248,255)
    Dev->>EM: start emulator with EMULATOR_ARGS\n(--access-node-grpc-host=localhost:3569,\n--coa-address <addr>, --coa-key <key>,\n--coa-tx-lookup-enabled=true)
    EM-->>GW: emulator exposes configured gRPC host
  end

  rect rgb(245,255,240)
    GW->>EM: request chain data or COA lookup
    alt COA tx lookup enabled
      GW->>EM: call COA tx lookup endpoint
      EM-->>GW: COA lookup result
    else
      EM-->>GW: standard chain responses
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify Makefile argument assembly, spacing normalization, and removal of INIT_CADENCE_HEIGHT references.
  • Confirm README examples and placeholders accurately reflect runtime flags and env vars.
  • Check the comment and rationale for the MainnetInitCadenceHeight bump in config/config.go.

Possibly related PRs

Suggested labels

Documentation

Suggested reviewers

  • peterargue
  • janezpodhostnik

Poem

🐰 I nudged a flag, I bumped a height,
I straightened flags to tidy right;
The emulator hears a new small port,
Docs now guide each run and sort,
Hop, build, and ship into the night.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Update MainnetInitCadenceHeight to the first height of mainnet25" is directly related to the core technical change in this PR. According to the PR objectives, the primary purpose is to fix the MainnetInitCadenceHeight constant, which was set to a mainnet24 value and caused syncing from the EVM genesis block to fail. This title clearly and specifically identifies that key fix. While the PR also includes supporting changes such as README documentation updates and Makefile modifications to remove deprecated references, the title appropriately focuses on the most important technical change rather than trying to cover every detail of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mpeter/update-mainnet-evm-genesis-block

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
README.md (1)

156-156: Use proper heading syntax instead of bold emphasis.

The bold text is being used as a section heading, which breaks document structure and accessibility. Markdown linting flags this as improper usage.

Apply this diff:

-**Run EVM Gateway connected to Testnet**
+### Run EVM Gateway connected to Testnet
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05cfde8 and da285fe.

📒 Files selected for processing (3)
  • Makefile (1 hunks)
  • README.md (3 hunks)
  • config/config.go (1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
Makefile

[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 markdownlint-cli2 (0.18.1)
README.md

156-156: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (5)
Makefile (1)

21-33: LGTM! Emulator configuration improvements.

The changes improve the emulator setup by:

  • Explicitly configuring the access node gRPC host
  • Normalizing COA field formatting for consistency
  • Enabling WebSocket support and COA transaction lookup feature

These align well with the PR's goal of improving configuration and enabling new features.

README.md (3)

161-170: LGTM! Improved testnet configuration.

The configuration improvements include:

  • Explicit spork host configuration for proper multi-spork support
  • Security improvement by replacing actual credentials with xxx placeholders
  • Removal of the deprecated init-cadence-height flag

These changes make the example more production-ready and secure.


201-202: LGTM! Security improvement with credential placeholders.

Replacing the COA credentials with xxx placeholders prevents accidental credential exposure and makes it clear that users must provide their own credentials.


215-227: LGTM! Valuable mainnet configuration example.

This new mainnet configuration example provides:

  • Proper mainnet25/mainnet26 spork host configuration
  • Appropriate mainnet gas price (100000000 vs testnet's 100)
  • Consistent structure with the testnet example
  • Secure credential placeholders

This addresses the PR objective of providing proper mainnet configuration guidance and aligns with the MainnetInitCadenceHeight update.

config/config.go (1)

33-33: Mainnet25 genesis height verified as correct.

The official Flow EVM Gateway setup documentation confirms INIT_CADENCE_HEIGHT="85981135" for mainnet, validating the constant update from 85981134 to 85981135.

@m-Peter m-Peter force-pushed the mpeter/update-mainnet-evm-genesis-block branch from da285fe to 60f9282 Compare October 29, 2025 11:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
README.md (1)

194-194: Consider consistency in heading markup.

Line 194 still uses bold emphasis (Run local EVM GW docker container connected to Testnet) instead of a proper markdown heading, similar to the issue flagged on line 156. While "local" may be intentional to distinguish this docker section, consistency with proper heading syntax would improve document structure.

If you wish to align with markdown best practices, apply this diff:

-**Run local EVM GW docker container connected to Testnet**
+### Run local EVM GW docker container connected to Testnet
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da285fe and 60f9282.

📒 Files selected for processing (3)
  • Makefile (1 hunks)
  • README.md (3 hunks)
  • config/config.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • config/config.go
  • Makefile
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

156-156: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (3)
README.md (3)

162-162: Testnet configuration example looks good.

The testnet CLI example correctly specifies the testnet access node hosts, uses placeholder xxx values for COA credentials, and includes appropriate configuration flags. The removal of explicit init-cadence-height aligns with the PR objective to deprecate that flag.

Also applies to: 167-169


201-202: Environment variables properly updated with placeholders.

The Docker environment variables section correctly uses xxx placeholders for sensitive COA credentials, making the documentation safer for public display.


215-227: Mainnet configuration example is well-structured and complete.

The new mainnet example follows the same pattern as the testnet example above, properly configures mainnet access nodes, includes appropriate spork hosts for mainnet25/26, and uses placeholder values for sensitive credentials. The gas-price adjustment for mainnet (100000000 vs 100 for testnet) is correct.

README.md Outdated
--coinbase=FACF71692421039876a5BB4F10EF7A439D8ef61E \
--coa-address=62631c28c9fc5a91 \
--coa-key=2892fba444f1d5787739708874e3b01160671924610411ac787ac1379d420f49 \
--coa-address=xxx \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why removing it? can we name it more meaningful?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's not a good idea to have publicly available a Flow address and its private key. Although this is for testnet, it's still not something that should be in public documentation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about mentioning about its format like <16-character hexadecimal address>, and <64-character hexadecimal private key>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 8043f61 .

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91868cf and 52beaf8.

📒 Files selected for processing (1)
  • README.md (10 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md

[grammar] ~196-~196: Use a hyphen to join words.
Context: ...the make target to connect a container based gateway instance to testnet requir...

(QB_NEW_EN_HYPHEN)

🪛 markdownlint-cli2 (0.18.1)
README.md

299-299: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


300-300: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


301-301: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (3)
README.md (3)

156-156: Markdown heading syntax previously fixed.

The section heading at line 156 now correctly uses ### Run EVM Gateway connected to Testnet rather than bold emphasis, which resolves the previous MD036 linting warning. Good catch addressing this in the revision.


215-227: New mainnet example configuration is well-structured.

The mainnet configuration block at lines 215–227 follows the same pattern as the testnet example, includes appropriate spork hosts for mainnet25/26, and correctly uses xxx placeholders for COA credentials (consistent with the security improvement from prior review feedback). The gas price and other settings align with production expectations.


160-170: Testnet configuration example improvements are sound.

The testnet configuration at lines 160–170 now includes --access-node-spork-hosts for historical access, properly uses xxx placeholders for sensitive COA values, and removes concrete credentials from public documentation. This aligns well with the PR objective to support syncing from the EVM genesis block while maintaining security.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52beaf8 and 6f98213.

📒 Files selected for processing (1)
  • README.md (10 hunks)
⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (1)
README.md (1)

215-227: Verify mainnet configuration aligns with PR objectives.

The new mainnet example section correctly references mainnet25 and mainnet26 spork hosts, which aligns with the PR objective to update to mainnet25's first height. However, ensure that the placeholder format is corrected (see separate comment on line 225).

Can you confirm that access-001.mainnet25.nodes.onflow.org and access-001.mainnet26.nodes.onflow.org are the correct spork endpoints for mainnet configuration?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
README.md (1)

215-227: Mainnet configuration example is well-structured; fix placeholder consistency.

The new mainnet configuration block (lines 215–227) properly reflects the PR objective to update for mainnet25 support, with correct access node hosts and configuration parameters. However, line 225 also exhibits the same placeholder formatting issue as the testnet example: the --coa-key placeholder is missing the closing > bracket.

--coa-address=<16-character hexadecimal address> \
---coa-key=<64-character hexadecimal private key> \
+--coa-key=<64-character hexadecimal private key> \
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f98213 and 697aac2.

📒 Files selected for processing (1)
  • README.md (10 hunks)
⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (3)
README.md (3)

26-52: Section structure improvements approved.

The conversion from bold emphasis to proper markdown heading syntax (##/### hierarchy) improves document structure and resolves linter compliance. Heading organization now clearly separates build, running, and configuration contexts.


297-313: API documentation additions approved.

The reorganized Additional/Unsupported APIs sections correctly:

  • Introduce debug_traceCall as a newly supported method
  • Maintain proper 2-space nested list indentation (MD007 compliance)
  • Clearly document Wallet, Proof, and Access List limitations with rationale

134-208: Testnet configuration sections well-organized with proper placeholders.

Sections for COA account creation, CLI execution, and Docker container setup are logically structured with clear environment variable and placeholder guidance. The placeholder format <16-character hexadecimal address> and guidance in documentation (lines 201–202) aligns with the security improvement from prior reviews (removing concrete credentials).

@m-Peter m-Peter force-pushed the mpeter/update-mainnet-evm-genesis-block branch from 697aac2 to dfc216b Compare October 30, 2025 10:21
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
Makefile (1)

26-26: Use variable reference instead of hardcoded value for consistency.

Line 26 should reference $(EMULATOR_COA_KEY) (defined on line 14) rather than hardcoding the value. This improves maintainability and ensures a single source of truth if the key ever needs updating.

-		--wallet-api-key=2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21 \
+		--wallet-api-key=$(EMULATOR_COA_KEY) \
README.md (1)

215-227: Mainnet configuration example is a valuable addition.

Lines 215–227 properly document the mainnet setup with appropriate spork hosts, gas price (100000000 vs. 100 for testnet), and configuration flags. This aligns well with the PR objective to support mainnet25. However, the gas-price difference warrants a brief explanatory comment to help operators understand why mainnet pricing differs.

--gas-price=100000000

Consider adding a note above the mainnet example:

+Note: The significantly higher gas-price for mainnet reflects actual Flow network conditions; adjust based on current network fees.
+
 ```bash
 ./flow-evm-gateway run \
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 697aac2 and dfc216b.

📒 Files selected for processing (3)
  • Makefile (1 hunks)
  • README.md (10 hunks)
  • config/config.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • config/config.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-07T01:35:09.751Z
Learnt from: peterargue
PR: onflow/flow-evm-gateway#772
File: services/requester/keystore/key_store.go:50-62
Timestamp: 2025-03-07T01:35:09.751Z
Learning: In the flow-evm-gateway codebase, panics are acceptable in scenarios where immediate detection of critical bugs is desired during development and testing, particularly for invariant violations that should never occur in a correctly functioning system (e.g., when a key is available but locked in the keystore implementation).

Applied to files:

  • README.md
📚 Learning: 2024-10-18T19:26:37.579Z
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.

Applied to files:

  • README.md
🪛 Gitleaks (8.28.0)
Makefile

[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ 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). (1)
  • GitHub Check: Test
🔇 Additional comments (6)
Makefile (2)

21-33: Good alignment of emulator configuration with new features.

The additions of --access-node-grpc-host, --ws-enabled=true, and --coa-tx-lookup-enabled=true properly extend the emulator setup for improved testing capabilities. The normalized COA flag spacing (lines 24-25) improves readability.


14-14: Development key hardcoded; ensure this is development-only.

Gitleaks detected a generic API key on this line. While this is marked for local/testing use only, consider adding an inline comment to explicitly document that this is a test/emulator-only key and should never be used in production.

-EMULATOR_COA_KEY := 2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21
+# NOTE: This is a development/emulator key for local testing only
+EMULATOR_COA_KEY := 2619878f0e2ff438d17835c2a4561cb87b4d24d72d12ec34569acd0dd4af7c21
README.md (4)

5-5: Clear, improved heading that better describes the purpose of EVM Gateway.

The updated description provides a more direct articulation of the gateway's role in facilitating EVM interaction on Flow.


79-79: Proper markdown heading structure and improved document organization.

The reorganization into explicit subsections (CLI, Docker, Verify, Testnet, Mainnet) using proper heading syntax (###) improves navigability and resolves markdown linting issues (MD036). The structure now clearly separates local development, testnet, and mainnet configuration paths.

Also applies to: 99-99, 111-111, 156-156


162-162: Excellent security improvement: placeholders replace hardcoded keys throughout.

Removing actual private keys and addresses in favor of placeholders (<16-character hexadecimal address>, <64-character hexadecimal private key>) significantly improves security posture. This prevents accidental key leakage via documentation copying. The format descriptors are clear and helpful.

Also applies to: 167-169, 201-202, 224-225


298-302: Tracing APIs section properly organized and expanded.

The addition of debug_traceCall (line 302) extends tracing capabilities and maintains correct 2-space list indentation per markdownlint (MD007).

@m-Peter m-Peter merged commit 638fe2e into main Oct 30, 2025
2 checks passed
@m-Peter m-Peter deleted the mpeter/update-mainnet-evm-genesis-block branch October 30, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants