feat: merge dev into main #426
Conversation
* chore(hardhat-deploy-config): add Sepolia deploy configs for v2.1.0 * chore(contracts): add v2.1.0 deployments on Sepolia
Prevents KromaMPT block from being handled as unsafe payload over P2P. This ensures that nodes can continue the migration process, even if they receive a new unsafe payload from the sequencer before receiving the batch containing the KromaMPT parent block, allowing them to still process the safe attribute.
Fix incorrect error message in checkL1BlockAddrAndData function
* chore: add v2.1.1 deployment config on mainnet * chore: add v2.1.1 deployments on Mainnet
WalkthroughThe pull request introduces changes related to the Kroma MPT (Merkle Patricia Tree) upgrade across multiple components of the Kroma blockchain infrastructure. The modifications primarily focus on error handling, configuration updates, and payload processing logic. Key changes include updating error messages, adding new error variables, modifying configuration timestamps, and implementing a new method to check KromaMPT activation blocks. These changes aim to provide more precise handling and reporting of events surrounding the KromaMPT upgrade. Changes
Sequence DiagramsequenceDiagram
participant Node as OpNode
participant Payload as L2 Payload
participant MPTCheck as KromaMPT Activation Check
Node->>MPTCheck: Check payload timestamp
MPTCheck-->>Node: Is KromaMPT Activation Block?
alt Is Activation Block
Node->>Node: Log and Ignore Payload
else Not Activation Block
Node->>Payload: Process Payload Normally
end
The sequence diagram illustrates the new payload processing logic added to the OpNode, specifically the check for KromaMPT activation blocks before payload processing. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit c41ac14.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
op-batcher/batcher/channel_builder.go (1)
24-25: Consider using typed errors for maintainability.
Defining these new error variables is a step forward in clarity. However, you might consider creating custom typed errors (e.g.,type KromaMPTError struct { ... }) in the future for even more structured error handling and to incorporate additional contextual data.op-batcher/batcher/channel_manager_test.go (1)
538-538: Assert the correct ActivationBlock error.
ErrReachedKromaMPTBlockclarifies that the code handles the exact Kroma MPT block. Double-check that the error logic won’t overlap or conflict with earlier blocks that cross the threshold.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumops-devnet/docker-compose.ymlis excluded by!**/*.ymlpackages/contracts/deploy-config/mainnet.jsonis excluded by!**/*.jsonpackages/contracts/deploy-config/sepolia.jsonis excluded by!**/*.jsonpackages/contracts/deployments/mainnet/Colosseum.jsonis excluded by!**/*.jsonpackages/contracts/deployments/mainnet/KromaPortal.jsonis excluded by!**/*.jsonpackages/contracts/deployments/mainnet/ValidatorManager.jsonis excluded by!**/*.jsonpackages/contracts/deployments/mainnet/ZKProofVerifier.jsonis excluded by!**/*.jsonpackages/contracts/deployments/mainnet/ZKProofVerifierProxy.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/Colosseum.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/KromaPortal.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/ValidatorManager.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/ZKProofVerifier.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/ZKProofVerifierProxy.jsonis excluded by!**/*.jsonpackages/contracts/deployments/sepolia/solcInputs/9e0b1e1e73ac46b8a3d915d840fa123d.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
kroma-chain-ops/cmd/check-kroma-mpt/main.go(1 hunks)op-batcher/batcher/channel_builder.go(2 hunks)op-batcher/batcher/channel_manager_test.go(3 hunks)op-node/chaincfg/chains.go(1 hunks)op-node/chaincfg/chains_test.go(1 hunks)op-node/node/node.go(1 hunks)op-node/rollup/types.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- kroma-chain-ops/cmd/check-kroma-mpt/main.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests
🔇 Additional comments (9)
op-node/chaincfg/chains.go (1)
45-45: Confirm KromaMPTTime correctness.
AssigningKromaMPTTimeto1739250001looks fine, but please ensure that this timestamp aligns with your intended activation schedule. Any off-by-one or environment-specific discrepancy could cause unexpected behaviors around upgrade time.Would you like me to generate a shell script to scan for all references to
KromaMPTTimeacross the codebase to confirm usage?op-node/chaincfg/chains_test.go (1)
72-72: Maintain consistency in tests.
Good job keepingKromaMPTTimeconsistent between the production config and test config. Just verify that your tests are indeed covering the logic exposed by this timestamp (e.g., boundary cases around1739250001).op-batcher/batcher/channel_builder.go (1)
184-187: Validate block time checks.
UsingIsKromaMPTParentBlockandIsKromaMPTActivationBlockprovides more precise error reporting. Be sure these checks handle edge cases (e.g., clock skew or bridging logic). Potential off-by-one issues around the activation boundary can lead to unexpected channel closures.op-batcher/batcher/channel_manager_test.go (4)
10-14: Reorganized imports.
The updated imports seem correct for the newly introduced logic and error checks. Just confirm that no references to removed packages remain.
489-489: Clearer test naming.
RenamingTestChannelManager_Close_BeforeMPTBlocktoTestChannelManager_CheckFullErrorsOnKromaMPTappropriately clarifies the scope of the test. Nice improvement for readability.
528-528: Check coverage for 3rd block addition.
Adding the 3rd L2 block (mptBlock) is critical for verifying edge conditions. Ensure that your test includes boundary checks around the Kroma MPT activation (e.g. just before and exactly at the activation time).
533-533: Assert the correct ParentBlock error.
ErrReachedKromaMPTParentBlockensures granular distinction for the block just before the Kroma MPT activation. This improves error clarity, but ensure thatIsKromaMPTParentBlockindeed triggers only at the correct boundary.op-node/rollup/types.go (1)
374-380: Ensure edge-case coverage for activation logic.
This method follows the same pattern asIsEcotoneActivationBlockand appears logically sound for skipping Genesis. It properly checksl2BlockTime >= c.BlockTimeto avoid incorrectly evaluating negative wrap-around underuint64. However, please ensure the test suite covers edge cases wherel2BlockTime < c.BlockTimeandKromaMPTTimeis zero or absent, so we don’t inadvertently treat an all-genesis scenario as activation.op-node/node/node.go (1)
597-603: Confirm the correctness of ignoring KromaMPT activation blocks.
The approach to ignore payloads at KromaMPT activation blocks (logging and returning early) is consistent with the intention to skip special-processing blocks. This prevents potential double-processing or inconsistent state transitions. Ensure that you have test coverage verifying that the node handles activation-block payloads in the intended manner (ignoring them) and transitions cleanly afterward.
Description
This PR is for v2.1.1 release.