Skip to content

Conversation

@LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Jan 8, 2026

Summary of changes

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

  • Breaking Changes

    • Error code for execution reverted scenarios updated from 11 to 3. Applications relying on the previous error code value should be updated accordingly.
  • Documentation

    • Updated changelog and documentation to reflect the new error code value and its ecosystem significance.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

Walkthrough

A breaking change that updates the execution reverted RPC error code from 11 to 3, documented in CHANGELOG.md and implemented in the error constants definition. Total 4 lines changed across 2 files.

Changes

Cohort / File(s) Summary
Error Code Update
src/rpc/methods/eth/errors.rs
Updated EXECUTION_REVERTED_CODE constant value from 11 to 3 with revised documentation explaining the new ecosystem-aligned value
Changelog
CHANGELOG.md
Added breaking change entry documenting the execution reverted error code transition from 11 to 3

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

RPC

Suggested reviewers

  • hanabi1224
  • sudo-shashank
🚥 Pre-merge checks | ✅ 3
✅ 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 change: updating the execution reverted error code from 11 to 3, which is reflected in both CHANGELOG.md and src/rpc/methods/eth/errors.rs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0af174d and 5b80074.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/rpc/methods/eth/errors.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
📚 Learning: 2026-01-05T12:54:40.850Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/cron_state.rs:8-8
Timestamp: 2026-01-05T12:54:40.850Z
Learning: In Rust code reviews, do not derive Eq for a struct if any field does not implement Eq (e.g., types from external dependencies). If a type like CronStateLotusJson includes fields wrapping external dependencies that lack Eq, derive PartialEq (or implement PartialEq manually) but avoid deriving Eq. This ensures comparisons compile and reflect actual equivalence semantics. When needed, consider implementing custom PartialEq (and possibly Eq) only after ensuring all fields (or wrappers) implement Eq, or keep PartialEq-only if full equality semantics cannot be expressed.

Applied to files:

  • src/rpc/methods/eth/errors.rs
📚 Learning: 2026-01-05T12:56:13.802Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6381
File: src/lotus_json/actors/states/evm_state.rs:41-44
Timestamp: 2026-01-05T12:56:13.802Z
Learning: In Rust codebases (e.g., Forest), do not add #[cfg(test)] to functions already annotated with #[test]. The #[test] attribute ensures the function is compiled only for tests, so a separate #[cfg(test)] is redundant and can be removed if present. Apply this check to all Rust files that contain #[test] functions.

Applied to files:

  • src/rpc/methods/eth/errors.rs
⏰ 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). (11)
  • GitHub Check: tests-release
  • GitHub Check: Coverage
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: Calibnet eth mapping check
  • GitHub Check: Diff snapshot export checks
  • GitHub Check: Calibnet api test-stateful check
  • GitHub Check: V2 snapshot export checks
  • GitHub Check: Bootstrap checks - Lotus
  • GitHub Check: Wallet tests
  • GitHub Check: Forest CLI checks
  • GitHub Check: Calibnet check
🔇 Additional comments (2)
CHANGELOG.md (1)

30-31: LGTM - Clear breaking change documentation.

The changelog entry properly documents this breaking change. The description is concise and accurate.

src/rpc/methods/eth/errors.rs (1)

11-12: The error code 3 standard is correctly documented, but confirm Lotus PR #13467 merge status before merging.

The claim that error code 3 was introduced in geth v1.9.15 and is the standard for execution reverted errors in the Ethereum ecosystem is accurate per official Geth release notes and ecosystem tooling (QuickNode, Alchemy). The code comment is correct.

However, per the PR objectives, manually confirm that the referenced Lotus PR (filecoin-project/lotus#13467) has been merged before merging this change. This cannot be verified automatically.


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

@LesnyRumcajs LesnyRumcajs marked this pull request as ready for review January 8, 2026 13:49
@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner January 8, 2026 13:49
@LesnyRumcajs LesnyRumcajs requested review from hanabi1224 and sudo-shashank and removed request for a team January 8, 2026 13:49
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.

2 participants