Skip to content

fix(v11): install missing EIP-2935 history-storage contract#149

Closed
aluque-peersyst wants to merge 2 commits into
xrplevm:mainfrom
aluque-peersyst:fix/missing-eip-2935-history-storage
Closed

fix(v11): install missing EIP-2935 history-storage contract#149
aluque-peersyst wants to merge 2 commits into
xrplevm:mainfrom
aluque-peersyst:fix/missing-eip-2935-history-storage

Conversation

@aluque-peersyst

@aluque-peersyst aluque-peersyst commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • The v11 upgrade now automatically installs missing default EVM preinstalls (including the EIP-2935 history-storage contract when required) during the upgrade flow.
    • This runs as a post-configuration step to ensure required EVM code is present before upgrade completion.
  • Bug Fixes
    • Added logic to detect already-deployed preinstalls, preventing reinstallation when the contract already exists and improving upgrade reliability.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb8a08ea-b187-43eb-b17c-8941c24be51b

📥 Commits

Reviewing files that changed from the base of the PR and between c9fa17b and 6ca74b9.

📒 Files selected for processing (1)
  • app/upgrades/v11/upgrades.go

📝 Walkthrough

Walkthrough

The v11 upgrade wiring now passes an EVM keeper into the handler. During the upgrade, the handler checks default EVM preinstalls and adds any that are missing.

Changes

v11 EVM preinstall upgrade

Layer / File(s) Summary
Keeper wiring and interface
app/upgrades.go, app/upgrades/v11/keepers.go, app/upgrades/v11/upgrades.go
setupUpgradeHandlers passes app.EvmKeeper into v11.CreateUpgradeHandler, the v11 upgrade handler accepts the extra keeper dependency, and v11.EvmKeeper defines the IsContract and AddPreinstalls methods used by the handler.
Missing preinstall installation
app/upgrades/v11/upgrades.go
CreateUpgradeHandler adds a post-escrow step that scans evmtypes.DefaultPreinstalls, skips addresses that already have code, and calls AddPreinstalls with any missing entries.

Sequence Diagram(s)

sequenceDiagram
  participant CreateUpgradeHandler as "v11.CreateUpgradeHandler"
  participant installMissingPreinstalls as "installMissingPreinstalls"
  participant EvmKeeper
  participant DefaultPreinstalls as "evmtypes.DefaultPreinstalls"
  CreateUpgradeHandler->>installMissingPreinstalls: run after escrow withdrawal
  installMissingPreinstalls->>DefaultPreinstalls: iterate preinstalls
  installMissingPreinstalls->>EvmKeeper: IsContract(addr)
  alt preinstall is missing
    installMissingPreinstalls->>EvmKeeper: AddPreinstalls(preinstalls)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • xrplevm/node#127 — Updates the same v11 upgrade handler flow in app/upgrades/v11/upgrades.go.
  • xrplevm/node#145 — Changes the same v11 upgrade wiring and handler signature in app/upgrades.go and app/upgrades/v11/upgrades.go.
  • xrplevm/node#147 — Extends the same v11 upgrade path with additional keeper-driven upgrade behavior.

Suggested reviewers

  • AdriaCarrera

Poem

A bunny hopped to v11's gate,
checked the preinstalls, did not wait.
If one was missing, hop and install,
then off I went, proud after all.
🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning No PR description was provided, so the required Motivation, Changes, Considerations, and Dependencies sections are missing. Add a PR description using the repository template with Motivation, Changes, Considerations, and Dependencies sections filled in.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: installing missing EIP-2935 preinstalls during the v11 upgrade.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant