-
Notifications
You must be signed in to change notification settings - Fork 284
feat: upgrade L1GasOracle predeploy in GalileoV2 upgrade #1259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds GalileoV2 hard-fork support: new config fields and detection, rollup rcfg constants, a misc.ApplyGalileoV2HardFork implementation, and integration points that invoke the fork during prestate application, state processing, chain generation, miner worker flow, plus witness state read; bumps VersionPatch. Changes
Sequence DiagramsequenceDiagram
participant Config as ChainConfig
participant BlockProc as Block Processor
participant Misc as misc.ApplyGalileoV2HardFork
participant StateDB as StateDB / L1GasPriceOracle
Note over Config,BlockProc: Per-block processing checks for transitions
BlockProc->>Config: IsGalileoV2TransitionBlock(parentTS, ts)?
alt transition == true
BlockProc->>Misc: ApplyGalileoV2HardFork(statedb)
Misc->>StateDB: Set L1GasPriceOracle bytecode -> GalileoV2
Misc->>StateDB: Set IsGalileoSlot = 1
StateDB-->>Misc: ack
Misc-->>BlockProc: fork applied
else
BlockProc-->>Config: continue normal processing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the GalileoV2 hard fork upgrade, which updates the L1GasPriceOracle predeploy contract with new gas calculation logic. The upgrade follows the established pattern of previous Scroll hard forks (e.g., Feynman).
- Adds GalileoV2 fork configuration and detection logic
- Updates the L1GasPriceOracle contract bytecode with the GalileoV2 version
- Ensures witness generation includes the new
isGalileostorage slot for revm compatibility
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| params/version.go | Bumps patch version from 15 to 16 |
| params/config.go | Adds GalileoV2Time configuration field, fork detection methods (IsGalileoV2, IsGalileoV2TransitionBlock), and updates Rules struct and String() method |
| rollup/rcfg/config.go | Adds IsGalileoSlot storage slot constant and GalileoV2L1GasPriceOracleBytecode with verification instructions |
| consensus/misc/galileoV2.go | New file implementing ApplyGalileoV2HardFork to upgrade contract bytecode and initialize isGalileo storage slot |
| core/state_processor.go | Applies GalileoV2 hard fork during block processing at transition block |
| core/chain_makers.go | Applies GalileoV2 hard fork during test chain generation at transition block |
| miner/scroll_worker.go | Applies GalileoV2 hard fork during block mining at transition block |
| cmd/evm/internal/t8ntool/execution.go | Applies GalileoV2 hard fork during EVM transition tool execution at transition block |
| eth/api.go | Ensures IsGalileoSlot is accessed during witness generation for revm compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
params/config.go(8 hunks)rollup/rcfg/config.go(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-09T19:18:56.758Z
Learnt from: Thegaram
Repo: scroll-tech/go-ethereum PR: 1245
File: crypto/kzg4844/kzg4844_gokzg.go:118-150
Timestamp: 2025-10-09T19:18:56.758Z
Learning: The scroll-tech/go-ethereum repository uses Go 1.22+ and supports range-over-integer syntax such as `for range n` and `for i := range len(slice)`, which are valid Go 1.22 language features.
Applied to files:
params/config.go
🧬 Code graph analysis (1)
rollup/rcfg/config.go (2)
common/types.go (1)
BigToHash(62-62)common/bytes.go (1)
Hex2Bytes(79-82)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
params/config.go (5)
337-337: GalileoV2 fork not yet scheduled on Sepolia.The GalileoV2Time is correctly set to
nil, indicating the fork is not yet activated on the Sepolia testnet. This is consistent with a preparatory PR.
885-915: String representation correctly updated.The String() method properly formats GalileoV2Time for display, following the same pattern as other fork times (Darwin, Euclid, Feynman, Galileo).
1033-1036: IsFeynmanTransitionBlock helper added.This new helper method detects the first Feynman block by checking if the current block is post-Feynman but the parent is pre-Feynman. This follows the same pattern as IsGalileoV2TransitionBlock and is useful for fork transition logic.
1043-1051: GalileoV2 detection methods implemented correctly.Both
IsGalileoV2()andIsGalileoV2TransitionBlock()follow the established pattern from previous forks. The transition block helper will be useful for applying the L1GasPriceOracle upgrade at the exact fork boundary.
1281-1311: Rules struct properly extended.The
IsGalileoV2field is correctly added to the Rules struct and initialized in the Rules() method, maintaining consistency with other fork flags.rollup/rcfg/config.go (1)
96-105: Build instructions reference incorrect file paths.The review comment provides build instructions that contain errors:
Source file location: The comment references
src/L1GasPriceOracle.sol, but the actual contract is located atsrc/L2/predeploys/L1GasPriceOracle.sol.Artifact path: The jq command should extract from
artifacts/src/L2/predeploys/L1GasPriceOracle.sol/L1GasPriceOracle.jsoninstead ofartifacts/src/L1GasPriceOracle.sol/L1GasPriceOracle.json.PR association: The commit
dfffa0f04bbd1de31ef342e1642a2f9ad9a620feis confirmed to be associated with PR #164 only. There is no reference to PR #163 in the commit message.The corrected build instructions should be:
git checkout dfffa0f04bbd1de31ef342e1642a2f9ad9a620fe yarn forge build cat artifacts/src/L2/predeploys/L1GasPriceOracle.sol/L1GasPriceOracle.json | jq -r .deployedBytecode.object
See also:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.