feat: add Kroma Burgundy upgrade time #268
Conversation
WalkthroughThese updates introduce new functionalities and configurations for Ethereum smart contracts, specifically focusing on the Kroma Burgundy network upgrade. A new file adds predeploy configurations for smart contracts, while other changes incorporate the Kroma Burgundy time offset and activation into the system. This includes adjustments to genesis configurations, the setup process for end-to-end tests, and rollup type configurations, ensuring seamless integration and activation of the Kroma Burgundy hard fork. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sumops-devnet/docker-compose.ymlis excluded by:!**/*.yml
Files selected for processing (5)
- kroma-bindings/predeploys/predeploy.go (1 hunks)
- kroma-chain-ops/genesis/config.go (3 hunks)
- kroma-chain-ops/genesis/genesis.go (1 hunks)
- op-e2e/e2eutils/setup.go (4 hunks)
- op-node/rollup/types.go (4 hunks)
Additional comments: 10
kroma-bindings/predeploys/predeploy.go (2)
- 7-11: The
DeployConfiginterface defines methods for governance and timing configurations. It's crucial to ensure that these methods are implemented correctly in all concrete types that satisfy this interface. The inclusion ofCanyonTimeandBurgundyTimemethods indicates support for specific network upgrades, which is a good practice for future-proofing the codebase.- 13-17: The
Predeploystruct is well-defined, encapsulating the necessary information for a predeployed contract, including its address, a flag for proxy disablement, and a function to determine its enabled state based on the deployment configuration. This struct is a good example of encapsulating contract-related data in a clear and maintainable manner.kroma-chain-ops/genesis/genesis.go (1)
- 67-67: The addition of
BurgundyTimeto thekromaChainConfigwithin theNewL2Genesisfunction is a critical update for the Burgundy upgrade. It ensures that the activation time of the Burgundy hard fork is correctly set based on the block time and the specified offset. This change is essential for the smooth transition and activation process of the upgrade.op-e2e/e2eutils/setup.go (2)
- 27-27: Changing the file permission to
0o600for writing the JWT secret enhances security by ensuring that the file is only accessible to the user who created it. This is a good practice for handling sensitive information.- 216-222: The addition of the
BurgundyTimeOffsetfunction is a thoughtful update to manage the time offset for the Burgundy upgrade during setup. This ensures that the testing environments can accurately simulate the timing requirements of the upgrade, which is crucial for thorough and effective testing.op-node/rollup/types.go (3)
- 112-114: The addition of the
BurgundyTimefield to theConfigstruct is a key update for supporting the Kroma Burgundy network upgrade. This field allows for the activation time of the upgrade to be set and checked against the block timestamp, which is essential for conditional activation of new features or changes.- 317-320: The
IsBurgundymethod provides a straightforward way to check if the Burgundy hardfork is active based on the current timestamp. This method is crucial for conditional logic that depends on whether the Burgundy upgrade has been activated. It's implemented correctly and follows the pattern established by other upgrade checks.- 357-358: Updating the description output to include the Burgundy network upgrade is a valuable addition for logging and debugging purposes. It ensures that the configuration related to the Burgundy upgrade is visible and easily accessible, which can be helpful for operators and developers.
kroma-chain-ops/genesis/config.go (2)
- 257-259: The addition of the
L2GenesisBurgundyTimeOffsetfield to theDeployConfigstruct is correctly implemented. It follows the existing pattern for time offset fields related to other hard forks, allowing for the control of the activation time of the Kroma Burgundy hard fork through an offset. This change is consistent with the PR objectives and aligns with the existing code structure.- 600-609: The implementation of the
BurgundyTimemethod is correct and follows the pattern established by similar methods for calculating the activation time of hard forks based on the genesis time and a specified offset. The method correctly handles the case where theL2GenesisBurgundyTimeOffsetisnil, indicating that the Burgundy hard fork is disabled, by returningnil. This ensures that the method's behavior is consistent with the expectations for hard fork activation time calculations.
|
While not currently used, it would be nice to have the flag code in the Op-e2e descendant for consistency. (setup.go) |
Pangssu
force-pushed
the
feat/add-kroma-burgundy
branch
from
7762631 to
ec57fdd
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sumops-devnet/docker-compose.ymlis excluded by:!**/*.yml
Files selected for processing (6)
- kroma-bindings/predeploys/predeploy.go (1 hunks)
- kroma-chain-ops/genesis/config.go (3 hunks)
- kroma-chain-ops/genesis/genesis.go (1 hunks)
- op-e2e/e2eutils/setup.go (3 hunks)
- op-e2e/setup.go (2 hunks)
- op-node/rollup/types.go (4 hunks)
Files skipped from review as they are similar to previous changes (5)
- kroma-bindings/predeploys/predeploy.go
- kroma-chain-ops/genesis/config.go
- kroma-chain-ops/genesis/genesis.go
- op-e2e/e2eutils/setup.go
- op-node/rollup/types.go
Additional comments: 4
op-e2e/setup.go (4)
- 73-73: The removal of
var (and)around thetestingJWTSecretdeclaration simplifies the variable declaration. This change is straightforward and improves readability.- 486-486: The addition of the
BurgundyTimefield initialization in theStartfunction is crucial for the Kroma Burgundy upgrade. Ensure that theDeployConfig.BurgundyTimemethod correctly calculates the Burgundy hard fork activation time based on the provided timestamp.Verification successful
The
DeployConfig.BurgundyTimemethod correctly calculates the Burgundy hard fork activation time based on the providedgenesisTimeand an offset (L2GenesisBurgundyTimeOffset). It returns nil if no offset is set, indicating no specific Burgundy time, and otherwise calculates the activation time by adding the offset to thegenesisTime. This aligns with the requirements for the Kroma Burgundy upgrade.* 70-76: > 📝 **NOTE** > This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the DeployConfig.BurgundyTime method's correctness. # This script is a placeholder for the verification process and should be replaced with actual verification steps. echo "Verify DeployConfig.BurgundyTime method's correctness."Length of output: 121
Script:
#!/bin/bash # Search for the DeployConfig.BurgundyTime method within the codebase to verify its implementation. rg "func \(.*DeployConfig.*\) BurgundyTime" -A 10Length of output: 677
Ensure the package declaration and imports are correctly organized and that there are no unused imports. This helps maintain code cleanliness and readability.
Consider running a linter or formatter to automatically organize imports and remove unused ones.
- 486-486: Adding the
BurgundyTimefield to the rollup configuration is essential for the upgrade. However, ensure that all dependent configurations and systems are also updated to recognize and use this new field appropriately.Verification successful
The verification process has revealed that the
BurgundyTimefield has been integrated across various parts of the codebase, including rollup configuration-related files, types definitions, and genesis configurations. This indicates that dependent configurations and systems have been updated to recognize and use the new field appropriately. Based on this evidence, the review comment is verified as the necessary updates appear to have been made.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all configurations and systems that depend on the rollup configuration are updated to use BurgundyTime. echo "Check for dependencies on the rollup configuration that need to be updated to use BurgundyTime."Length of output: 200
Script:
#!/bin/bash # Search for references to the rollup configuration and specifically the usage of the BurgundyTime field across the codebase. echo "Searching for references to the rollup configuration..." rg "rollup configuration" --context 5 echo "Searching for usage of the BurgundyTime field..." rg "BurgundyTime" --context 5Length of output: 14116
kroma-chain-ops/genesis/genesis.go
Outdated
| @@ -64,6 +64,7 @@ func NewL2Genesis(config *DeployConfig, block *types.Block) (*core.Genesis, erro | |||
| // [Kroma: START] | |||
| // kroma-geth is not yet ready. it should be upstream to v1.101304.2 | |||
| // InteropTime: config.InteropTime(block.Time()), | |||
| BurgundyTime: config.BurgundyTime(block.Time()), | |||
| // | |||
Pangssu
force-pushed
the
feat/add-kroma-burgundy
branch
from
ec57fdd to
2e1afdb
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sumops-devnet/docker-compose.ymlis excluded by:!**/*.yml
Files selected for processing (6)
- kroma-bindings/predeploys/predeploy.go (1 hunks)
- kroma-chain-ops/genesis/config.go (3 hunks)
- kroma-chain-ops/genesis/genesis.go (1 hunks)
- op-e2e/e2eutils/setup.go (3 hunks)
- op-e2e/setup.go (2 hunks)
- op-node/rollup/types.go (4 hunks)
Files skipped from review as they are similar to previous changes (6)
- kroma-bindings/predeploys/predeploy.go
- kroma-chain-ops/genesis/config.go
- kroma-chain-ops/genesis/genesis.go
- op-e2e/e2eutils/setup.go
- op-e2e/setup.go
- op-node/rollup/types.go
|
Closing this PR because our team have made an internal decision to not create |
The Kroma Burgundy upgrade activates the governance token.
This PR includes the implementation of configurations for the Burgundy upgrade.
Summary by CodeRabbit
New Features
L2GenesisBurgundyTimeOffsetfield to control the activation time of the Kroma Burgundy hard fork.Refactor
Chores