disable eth_simulateV1 by marcello33 · Pull Request #2161 · 0xPolygon/bor

marcello33 · 2026-03-25T16:27:56Z

Description

Temporarily disable eth_simulateV1

curl -s -X POST http://127.0.0.1:54384 -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","method":"eth_simulateV1","params":[{"blockStateCalls":[{"calls":[]}]},null],"id":1}' | jq                                              
{                                                                                                                                                                                                                                       
  "jsonrpc": "2.0",                                                                                                                                                                                                                     
  "id": 1,                                                                                                                                                                                                                              
  "error": {                                                                                                                                                                                                                            
    "code": -32000,                                                                                                                                                                                                                     
    "message": "eth_simulateV1 is not supported on Bor"                                                                                                                                                                                 
  }                                                                                                                                                                                                                                     
}

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

marcello33 · 2026-03-25T16:29:04Z

@claude review

sonarqubecloud · 2026-03-25T16:29:22Z

Quality Gate failed

Failed conditions
13.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

claude · 2026-03-25T16:29:22Z

Claude finished @marcello33's task in 53s —— View job

Code Review: Disable `eth_simulateV1`

Gather context and review PR diff
Review implementation
Post findings

Overall: Clean, minimal change that achieves the goal. A few observations:

1. `//nolint:govet` placement — verify it works (LOW)

internal/ethapi/api.go:1073 — The //nolint:govet directive is placed on the line after the early return. Some linter configurations associate the directive with the line it's on, not the lines below it. Verify that golangci-lint actually suppresses the "unreachable code" warning here. If not, you may need to restructure slightly (e.g., wrap the disabled code in an if false { ... } block, or place the directive differently).

Check this →

2. Missing `t.Skip` on `TestSimulateV1ChainLinkage` — already skipped for a different reason (INFO)

internal/ethapi/api_test.go:2935 — TestSimulateV1ChainLinkage is already skipped with a different reason ("bor: skipping because we use total difficulty and beacon expects 0"), so it doesn't need the new skip. Just noting for completeness — this is fine as-is since it's already not running.

3. New test `TestSimulateV1Disabled` looks good (POSITIVE)

internal/ethapi/api_test.go:1565-1574 — Good addition. It verifies the error message and nil result, uses t.Parallel(), and is concise. The require.EqualError assertion pins the exact error string, which ensures the user-facing message stays correct.

4. Consider a doc comment or TODO for re-enablement (SUGGESTION)

The skip messages say "unskip when re-enabling" which is helpful. A brief // TODO: comment on the early return in api.go (e.g., linking to a tracking issue) would make it easier to find and revert later. Not blocking.

Verdict: Looks good — straightforward disable with proper test coverage. The only thing worth double-checking is that the //nolint directive actually suppresses the linter warning (point 1).

claude · 2026-03-25T16:32:08Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Copilot

Pull request overview

Temporarily disables the eth_simulateV1 JSON-RPC method in the Bor ethapi implementation so calls return a consistent “not supported” error, and adjusts the test suite accordingly.

Changes:

Hard-disable BlockChainAPI.SimulateV1 by returning an error immediately.
Add a new unit test asserting SimulateV1 is disabled and skip existing simulate-related tests in internal/ethapi.
Skip ethclient simulate integration tests while the RPC is disabled.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
`internal/ethapi/api.go`	Forces `SimulateV1` to return a “not supported on Bor” error.
`internal/ethapi/api_test.go`	Adds a disabled-behavior test and skips simulate tests.
`ethclient/ethclient_test.go`	Skips simulate tests that would now fail due to the RPC being disabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/ethapi/api.go

internal/ethapi/api_test.go

ethclient/ethclient_test.go

codecov · 2026-03-25T16:56:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (v2.7.0-candidate@61ad294). Learn more about missing BASE report.

Additional details and impacted files

@@                 Coverage Diff                 @@
##             v2.7.0-candidate    #2161   +/-   ##
===================================================
  Coverage                    ?   51.32%           
===================================================
  Files                       ?      882           
  Lines                       ?   154210           
  Branches                    ?        0           
===================================================
  Hits                        ?    79146           
  Misses                      ?    69915           
  Partials                    ?     5149

Files with missing lines	Coverage Δ
internal/ethapi/api.go	`39.13% <100.00%> (ø)`

Files with missing lines	Coverage Δ
internal/ethapi/api.go	`39.13% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ngotchac · 2026-03-29T11:13:23Z

Curious to understand why was this disabled? @cffls @marcello33

marcello33 · 2026-03-29T11:40:50Z

Hey @ngotchac we are testing some things internally and will most probably re-enable it in the next version

ngotchac · 2026-03-29T12:17:06Z

@marcello33 OK thx for the update.

disable eth_simulateV1

2583d31

Copilot AI review requested due to automatic review settings March 25, 2026 16:27

claude bot reviewed Mar 25, 2026

View reviewed changes

Copilot started reviewing on behalf of marcello33 March 25, 2026 16:28 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pratikspatil024 approved these changes Mar 25, 2026

View reviewed changes

marcello33 requested a review from a team March 25, 2026 16:43

cffls approved these changes Mar 25, 2026

View reviewed changes

marcello33 merged commit 1a59b5e into v2.7.0-candidate Mar 25, 2026
30 of 31 checks passed

Conversation

marcello33 commented Mar 25, 2026

Description

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

marcello33 commented Mar 25, 2026

Uh oh!

sonarqubecloud bot commented Mar 25, 2026

Quality Gate failed

Uh oh!

claude bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: Disable eth_simulateV1

1. //nolint:govet placement — verify it works (LOW)

2. Missing t.Skip on TestSimulateV1ChainLinkage — already skipped for a different reason (INFO)

3. New test TestSimulateV1Disabled looks good (POSITIVE)

4. Consider a doc comment or TODO for re-enablement (SUGGESTION)

Uh oh!

claude bot commented Mar 25, 2026

Code review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

ngotchac commented Mar 29, 2026

Uh oh!

marcello33 commented Mar 29, 2026

Uh oh!

ngotchac commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

claude bot commented Mar 25, 2026 •

edited

Loading

Code Review: Disable `eth_simulateV1`

1. `//nolint:govet` placement — verify it works (LOW)

2. Missing `t.Skip` on `TestSimulateV1ChainLinkage` — already skipped for a different reason (INFO)

3. New test `TestSimulateV1Disabled` looks good (POSITIVE)

codecov bot commented Mar 25, 2026 •

edited

Loading