Skip to content

Run E2E tests with EVM VM bridge enabled#894

Merged
m-Peter merged 2 commits into
mainfrom
mpeter/fix-evm-vm-bridge-bootstrapping
Oct 10, 2025
Merged

Run E2E tests with EVM VM bridge enabled#894
m-Peter merged 2 commits into
mainfrom
mpeter/fix-evm-vm-bridge-bootstrapping

Conversation

@m-Peter
Copy link
Copy Markdown
Collaborator

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

Depends on: onflow/flow-go#7999
Depends on: onflow/flow-emulator#870

Closes: #893

Description

This will make sure that the EVM Gateway works correctly when the EVM VM bridge is enabled on Flow Emulator.


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

  • Chores
    • Bumped multiple Go module dependencies (Flow, Cadence, flow-go-sdk, gRPC, lz4, protobuf) for compatibility and alignment.
  • Tests
    • Enabled VM bridge in emulator test setup and switched test account storage/capability handling to the updated storage path.
    • Updated fee-history test expectation to reflect a non-zero first gasUsedRatio value.
  • Bug Fixes
    • Adjusted EVM event ingestion start logic to use the next cadence block height and handle emulator init edge cases.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 3, 2025

Walkthrough

Enable VM bridge in tests, move Cadence-owned account storage/capability paths from /storage/evm to /storage/evm_coa, bump multiple Flow/Cadence-related module versions in root and tests go.mod files, update a Web3.js getFeeHistory test expectation, and change EVM event ingestion start to use latestCadenceHeight + 1 with emulator edge-case handling.

Changes

Cohort / File(s) Summary
Dependency updates
go.mod, tests/go.mod
Bumped Flow/Cadence-related dependencies (e.g., github.com/onflow/cadence, github.com/onflow/flow-go, github.com/onflow/flow-go-sdk, google.golang.org/grpc, github.com/pierrec/lz4, and flow protobuf) to newer versions in both root and tests modules.
Test helper: VM bridge & COA storage
tests/helpers.go
Enabled SetupVMBridgeEnabled = true; migrated checks, saves, issuance, publication, and borrows for CadenceOwnedAccount from /storage/evm/storage/evm_coa and adjusted related public capability usage.
Web3.js test expectation
tests/web3js/eth_non_interactive_test.js
Updated expected gasUsedRatio[0] in getFeeHistory test from 00.066314225.
Bootstrap: EVM event ingestion start
bootstrap/bootstrap.go
Start height for EVM event subscriber changed to nextCadenceHeight = latestCadenceHeight + 1 with special-case handling for local Emulator initialization to avoid non-existent-height errors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Tester
  participant Emulator
  participant CadenceStorage as "Cadence /storage (evm_coa)"
  participant PublicCap as "Cadence /public/evm"

  Note over Tester,Emulator: Start emulator with VM bridge enabled
  Tester->>Emulator: Start (SetupVMBridgeEnabled = true)
  Emulator-->>Tester: Ready

  Note over Tester,CadenceStorage: Ensure CadenceOwnedAccount exists at /storage/evm_coa
  Tester->>CadenceStorage: Check `/storage/evm_coa`
  alt not present
    Tester->>CadenceStorage: Save COA at `/storage/evm_coa`
    Tester->>PublicCap: Publish capability from `/storage/evm_coa` -> `/public/evm`
  else present
    Tester->>CadenceStorage: Borrow COA from `/storage/evm_coa`
  end
Loading
sequenceDiagram
  autonumber
  participant Bootstrap
  participant AccessNode as "Cadence Access Node"
  participant Subscriber as "EVM Event Subscriber"

  Bootstrap->>AccessNode: Query latestCadenceHeight
  Note right of Bootstrap: compute nextCadenceHeight = latestCadenceHeight + 1\n(handle Emulator init edge case)
  Bootstrap->>Subscriber: Create subscriber(startHeight = nextCadenceHeight)
  Subscriber->>AccessNode: Subscribe from nextCadenceHeight...
  AccessNode-->>Subscriber: Events stream
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Testing

Suggested reviewers

  • zhangchiqing
  • peterargue
  • janezpodhostnik

Poem

I hopped the bridge and moved the COA with care,
From /storage/evm to /storage/evm_coa I wear a whiskered air.
Modules updated, tests now sing in tune,
Off-by-one fixed — I danced by the moon. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning While most modifications are targeted at enabling the VM bridge and fixing indexing under SetupVMBridgeEnabled=true, the broad dependency bumps for unrelated modules such as google.golang.org/grpc, pierrec/lz4, and cadence may fall outside the scope of issue #893 since they are not required to address the EVM VM bridge indexing issue. Please isolate or justify general dependency upgrades in a separate pull request or clearly document their necessity to avoid conflating unrelated changes with the fix for issue #893.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the main objective of this changeset by highlighting that E2E tests will be run with the EVM VM bridge enabled, which is the core purpose of the PR. It is concise, clear, and directly reflects the primary change without extraneous details.
Linked Issues Check ✅ Passed The changes enable the VM bridge in test helpers, adjust the bootstrap logic to handle the nextCadenceHeight, update tests to reflect the expected gasUsedRatio, and bump dependent modules including flow-emulator to v1.8.0, which collectively address the indexing breakage when SetupVMBridgeEnabled=true as described in issue #893.
✨ 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/fix-evm-vm-bridge-bootstrapping

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22aaa3e and cb843cf.

⛔ Files ignored due to path filters (2)
  • go.sum is excluded by !**/*.sum
  • tests/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • bootstrap/bootstrap.go (1 hunks)
  • go.mod (3 hunks)
  • tests/go.mod (2 hunks)
  • tests/helpers.go (2 hunks)
  • tests/web3js/eth_non_interactive_test.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-06T10:14:49.676Z
Learnt from: m-Peter
PR: onflow/flow-evm-gateway#890
File: services/ingestion/event_subscriber.go:235-239
Timestamp: 2025-10-06T10:14:49.676Z
Learning: In services/ingestion/event_subscriber.go, when reconnecting after disconnect errors (DeadlineExceeded, Internal, Unavailable), the subscription should reconnect at lastReceivedHeight rather than lastReceivedHeight+1. This avoids errors when the next height doesn't exist yet, and duplicate event processing is safe because the ingestion engine is explicitly designed to be idempotent (storage uses batch.Set() which overwrites existing entries).

Applied to files:

  • bootstrap/bootstrap.go
🧬 Code graph analysis (1)
bootstrap/bootstrap.go (2)
config/config.go (1)
  • EmulatorInitCadenceHeight (21-21)
services/ingestion/event_subscriber.go (1)
  • NewRPCEventSubscriber (49-65)
⏰ 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 (8)
tests/web3js/eth_non_interactive_test.js (1)

392-392: Test expectation correctly updated for VM bridge bootstrap.

The updated gasUsedRatio value of 0.066314225 accurately reflects the 18 EVM transactions (7,957,707 gas) from the VM bridge bootstrap that are now attributed to the first committed EVM block.

bootstrap/bootstrap.go (1)

146-162: Event subscription height logic correctly handles emulator edge case.

The logic properly computes nextCadenceHeight = latestCadenceHeight + 1 for normal operation and handles the emulator genesis edge case where block 1 doesn't exist yet. Since the ingestion engine is idempotent (storage uses batch.Set() which overwrites entries), subscribing at height 0 when already at height 0 is safe.

Consider enhancing the comment to explicitly mention that duplicate event processing is safe due to idempotent storage, which makes this edge case handling robust.

Based on learnings

tests/go.mod (4)

8-8: LGTM: Cadence dependency updated.

The cadence module update from v1.7.0 to v1.7.1 supports the VM bridge functionality.


10-10: LGTM: Flow Emulator updated to v1.8.0.

This update aligns with the PR dependencies (flow-emulator@v1.8.0) and includes the VM bridge support mentioned in the PR objectives.


12-13: LGTM: Flow Go and SDK dependencies updated.

These updates provide the necessary VM bridge functionality and align with the coordinated changes across onflow/flow-go#7999 and onflow/flow-emulator#870.


167-174: LGTM: Indirect dependencies updated consistently.

The indirect dependency updates (flow protobuf, pierrec/lz4) maintain consistency with the direct dependency changes.

tests/helpers.go (2)

96-96: VM bridge enabled for E2E tests.

This change enables the VM bridge in the emulator, which is the core objective of the PR to ensure the EVM Gateway works correctly with the VM bridge enabled.


246-258: Approve CadenceOwnedAccount storage path adjustment All storage operations now use /storage/evm_coa and the public capability remains /public/evm; no other occurrences were found.


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.

@jribbink
Copy link
Copy Markdown
Contributor

jribbink commented Oct 8, 2025

The dependent changes should be available via onflow/flow-emulator@v1.8.0

@m-Peter m-Peter force-pushed the mpeter/fix-evm-vm-bridge-bootstrapping branch from 4501243 to bc4dfd0 Compare October 9, 2025 07:17
@m-Peter
Copy link
Copy Markdown
Collaborator Author

m-Peter commented Oct 9, 2025

The dependent changes should be available via onflow/flow-emulator@v1.8.0

Awesome 🙌 I have also updated the emulator version to v1.8.0, so as soon as this gets merged, we can make a new release, and then a new Flow CLI release with it.

reward: [['0x96'], ['0x96'], ['0x96']], // gas price is 150 during testing
baseFeePerGas: [1n, 1n, 1n],
gasUsedRatio: [0, 0.006205458333333334, 0]
gasUsedRatio: [0.066314225, 0.006205458333333334, 0]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: why did this change? I'm not sure I follow.

Copy link
Copy Markdown
Collaborator Author

@m-Peter m-Peter Oct 9, 2025

Choose a reason for hiding this comment

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

@janezpodhostnik A very good question 👏

The VM bridge bootstrap, executed a bunch of EVM transactions, on the genesis block. However, the EVM block was never committed with InternalEVM.commitBlockProposal(), because the system chunk transaction was not executed for the genesis block.

This means that all of the 18 EVM transactions that were executed in the genesis block, will actually be attributed to the first committed EVM block. That would be the first committed Flow block by the Emulator, since it will run the system chunk transaction. The difference is gasUsedRatio comes from the fact that before there were no EVM transactions on EVM block height 1, but now we have the 18 EVM transactions from the VM bridge bootstrap.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to fix this in fvm bootstrapping? Add The EVM block formation transaction at the end of bootstrapping?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

And to give some numbers:

total gas used = 7957707
block gas limit = 120000000
gasUsedRatio = total gas used / block gas limit = 0.066314225

Do we need to fix this in fvm bootstrapping? Add The EVM block formation transaction at the end of bootstrapping?

I think it's not really necessary. At least for unblocking the current issue. I'll give it a shot when I have some more time.

Copy link
Copy Markdown
Contributor

@janezpodhostnik janezpodhostnik left a comment

Choose a reason for hiding this comment

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

Code looks good.

@m-Peter m-Peter force-pushed the mpeter/fix-evm-vm-bridge-bootstrapping branch from bc4dfd0 to 22aaa3e Compare October 9, 2025 17:29
@m-Peter m-Peter force-pushed the mpeter/fix-evm-vm-bridge-bootstrapping branch from 22aaa3e to cb843cf Compare October 10, 2025 10:57
@m-Peter m-Peter merged commit 8248471 into main Oct 10, 2025
2 checks passed
@m-Peter m-Peter deleted the mpeter/fix-evm-vm-bridge-bootstrapping branch October 10, 2025 11:04
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.

Investigate issue with indexing when SetupVMBridgeEnabled = true

3 participants