Skip to content

chore: self-contained revert tests, contract reorg, and failure analysis#3033

Merged
mojtaba-esk merged 14 commits intomainfrom
mojtaba/self-contained-revert-tests,-contract-reorg,-and-failure-analysis
Mar 13, 2026
Merged

chore: self-contained revert tests, contract reorg, and failure analysis#3033
mojtaba-esk merged 14 commits intomainfrom
mojtaba/self-contained-revert-tests,-contract-reorg,-and-failure-analysis

Conversation

@mojtaba-esk
Copy link
Contributor

Describe your changes and provide context

This PR proposes a number of improvements on EVM RPC tests. The failing tests' fixtures are manually analyzed and updated to have Ethereum-expected behavior with adding the required steps and tags.
Updated the README and analysis doc describe the suite, removed tests, and failure breakdown.

Replaces 4 execution-apis revert/estimateGas fixtures that relied on fixed addresses with Sei fixtures that use a deployable reverter contract (REVERTER); script sets SEI_EVM_IO_REVERTER_ADDRESS.

Related: PLT-158

Testing performed to validate your change

Latest run: 162 tests, 133 passed, 29 failed (82.1% pass) which previously was 61.6%
Failed cases are documented in FAILED_TEST_ANALYSIS.md with suggested RPC fixes.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMar 13, 2026, 7:03 AM

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMar 6, 2026, 12:02 PM

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.32%. Comparing base (9029c60) to head (cde1eb1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3033      +/-   ##
==========================================
- Coverage   58.32%   58.32%   -0.01%     
==========================================
  Files        2076     2078       +2     
  Lines      171478   170782     -696     
==========================================
- Hits       100020    99608     -412     
+ Misses      62549    62263     -286     
- Partials     8909     8911       +2     
Flag Coverage Δ
sei-chain-pr 79.34% <100.00%> (?)
sei-db 70.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
integration_test/evm_module/rpc_io_test/io.go 79.34% <100.00%> (+0.45%) ⬆️

... and 261 files with indirect coverage changes

🚀 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.

Copy link
Collaborator

@masih masih left a comment

Choose a reason for hiding this comment

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

Left a couple of comments about the assertion logic and missing coverage on panic/other failure modes.

Great to see irrelevant tests getting removed 🙌

>> {"jsonrpc":"2.0","id":1,"method":"eth_call","params":[{"from":"0x0000000000000000000000000000000000000000","gas":"0x186a0","input":"0x01","to":"0x0ee3ab1371c93e7c0c281cc0c2107cdebc8b1930"},"latest"]}
// Self-contained: script deploys reverter contract; runner substitutes __REVERTER__ with its address.
// Expects eth_call to return JSON-RPC error code 3 with ABI-encoded Error("user error").
>> {"jsonrpc":"2.0","id":1,"method":"eth_call","params":[{"from":"0x0000000000000000000000000000000000000000","gas":"0x186a0","input":"0x01","to":"__REVERTER__"},"latest"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This now only covers the error path; we are missing coverage for panic now that *-panic.io is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!
we were only covering the error path. I've restored panic coverage.

// speconly: client response is only checked for schema validity.
>> {"jsonrpc":"2.0","id":1,"method":"eth_estimateGas","params":[{"from":"0x0102030000000000000000000000000000000000","input":"0x01","to":"0x0ee3ab1371c93e7c0c281cc0c2107cdebc8b1930"}]}
// Self-contained: script deploys reverter; runner substitutes __REVERTER__. Expects eth_estimateGas to return error (revert).
>> {"jsonrpc":"2.0","id":1,"method":"eth_estimateGas","params":[{"from":"0x0000000000000000000000000000000000000000","input":"0x01","to":"__REVERTER__"}]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to eth_call comment, i think now that estimate-failed-call.io is removed we are missing tests for panic and other revert/failure modes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I've added the panic fixture.

| eth_simulateV1 | ethSimulate-transfer-over-BlockStateCalls.iox | FAIL | Sei | Spec-only check failed | response kind mismatch: expected result=true error=false, actual result=false error=true |
| eth_simulateV1 | ethSimulate-two-blocks-with-complete-eth-sends.iox | FAIL | Sei | Spec-only check failed | response kind mismatch: expected result=true error=false, actual result=false error=true |
| eth_simulateV1 | ethSimulate-use-as-many-features-as-possible.iox | FAIL | Sei | Spec-only check failed | response kind mismatch: expected result=true error=false, actual result=false error=true |
| eth_syncing | check-syncing.iox | FAIL | Sei | Spec-only check failed | response kind mismatch: expected result=true error=false, actual result=false error=true |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see we are removing simualatev1 tests.

@mojtaba-esk mojtaba-esk requested a review from masih March 9, 2026 11:53
Copy link
Collaborator

@masih masih left a comment

Choose a reason for hiding this comment

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

Apart from || true LGTM 🙌

Copy link

@bdchatham bdchatham left a comment

Choose a reason for hiding this comment

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

A few minor concerns with files moved and slight inconsistencies between FAILED_TEST_ANALYSIS and RPC_IO_README but non-blockers

Choose a reason for hiding this comment

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

Was this move intention? I'm guessing yes so that the test runner doesn't find it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Thank you.
Actually the move was unintentional. The file ended up at repo root probably by a mouse drag :D I've restored it.

Copy link
Contributor

@monty-sei monty-sei left a comment

Choose a reason for hiding this comment

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

No noticeable issues stand out. All good on my end ✅

@mojtaba-esk mojtaba-esk enabled auto-merge March 13, 2026 07:02
@mojtaba-esk mojtaba-esk added this pull request to the merge queue Mar 13, 2026
Merged via the queue into main with commit 410a6ec Mar 13, 2026
38 checks passed
@mojtaba-esk mojtaba-esk deleted the mojtaba/self-contained-revert-tests,-contract-reorg,-and-failure-analysis branch March 13, 2026 07:33
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.

4 participants