-
Notifications
You must be signed in to change notification settings - Fork 342
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
chore(deps): bump geth to v1.13.7
#1374
Conversation
WalkthroughThe changes reflect a broad update across various Go files, primarily involving restructuring of ABI definitions in smart contract bindings, modifications to logging mechanisms, adjustments in Ethereum-related type references, and simplifications in configuration management. These updates seem to streamline interfaces, enhance logging capabilities, and remove outdated features like DevMode, suggesting an overall refinement of the system's architecture and interaction patterns. 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 X ? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (17)
- build/tools/go.mod
- build/tools/go.sum
- contracts/go.mod
- contracts/go.sum
- cosmos/go.mod
- cosmos/go.sum
- e2e/localnet/go.mod
- e2e/localnet/go.sum
- e2e/precompile/go.mod
- e2e/precompile/go.sum
- e2e/testapp/go.mod
- e2e/testapp/go.sum
- eth/go.mod
- eth/go.sum
- go.work.sum
- lib/go.mod
- lib/go.sum
Files selected for processing (29)
- contracts/bindings/cosmos/lib/cosmos_types.abigen.go (1 hunks)
- contracts/bindings/cosmos/precompile/bank/i_bank_module.abigen.go (1 hunks)
- contracts/bindings/cosmos/precompile/distribution/i_distribution_module.abigen.go (2 hunks)
- contracts/bindings/cosmos/precompile/governance/i_governance_module.abigen.go (1 hunks)
- contracts/bindings/cosmos/precompile/staking/i_staking_module.abigen.go (1 hunks)
- contracts/bindings/testing/consume_gas.abigen.go (1 hunks)
- contracts/bindings/testing/distribution_testing_helper.abigen.go (1 hunks)
- contracts/bindings/testing/governance/governance_wrapper.abigen.go (1 hunks)
- contracts/bindings/testing/liquid_staking.abigen.go (1 hunks)
- contracts/bindings/testing/mock_methods.abigen.go (1 hunks)
- contracts/bindings/testing/mock_precompile_interface.abigen.go (1 hunks)
- contracts/bindings/testing/precompile_constructor.abigen.go (1 hunks)
- contracts/bindings/testing/solmate_erc20.abigen.go (1 hunks)
- cosmos/config/config.go (1 hunks)
- cosmos/config/flags/flags.go (1 hunks)
- cosmos/config/template.go (1 hunks)
- cosmos/runtime/logging.go (1 hunks)
- cosmos/runtime/runtime.go (4 hunks)
- cosmos/x/evm/plugins/state/events/mock/log_factory.mock.go (4 hunks)
- cosmos/x/evm/plugins/state/events/mock/logsdb.mock.go (4 hunks)
- eth/core/mock/block_plugin.mock.go (8 hunks)
- eth/core/mock/historical_plugin.mock.go (15 hunks)
- eth/core/state/mocks/polar_state_db.go (1 hunks)
- eth/core/state/statedb.go (1 hunks)
- eth/core/vm/interfaces.go (1 hunks)
- eth/core/vm/mock/evm.go (1 hunks)
- eth/eth.go (3 hunks)
- eth/polar/api_backend.go (2 hunks)
- eth/polar/backend.go (1 hunks)
Files not summarized due to errors (1)
- contracts/bindings/testing/liquid_staking.abigen.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- cosmos/config/flags/flags.go
- eth/core/state/mocks/polar_state_db.go
Additional comments: 14
contracts/bindings/cosmos/lib/cosmos_types.abigen.go (1)
- 58-63: The restructuring of the
ABI
field inCosmosTypesMetaData
variable is noted. It's important to ensure that the new ABI structure is consistent with the smart contract's functions and events. The reordering and explicit specification of internal types should not affect the contract's interaction with other components, provided that the ABI is correctly generated from the updated smart contract.contracts/bindings/cosmos/precompile/bank/i_bank_module.abigen.go (1)
- 37-41: The changes in the
ABI
field ofBankModuleMetaData
variable reflect a more structured JSON representation. This should improve readability and maintainability of the code. However, it's crucial to verify that the new ABI aligns with the actual contract implementation to prevent any runtime issues.contracts/bindings/cosmos/precompile/distribution/i_distribution_module.abigen.go (2)
43-47: The addition of new functions
GetDelegatorValidatorReward
andGetTotalDelegatorReward
in theDistributionModuleMetaData
ABI suggests new functionality has been added to the contract. It's important to ensure that these new functions are properly tested and documented, and that any frontend or dependent services are updated to handle these new contract capabilities.226-255: The implementation of
GetDelegatorValidatorReward
and related session functions appears to be correct. Ensure that the corresponding smart contract method returns the expected[]CosmosCoin
type and that the method is properly tested, especially since it involves multiple return values.contracts/bindings/cosmos/precompile/governance/i_governance_module.abigen.go (1)
- 145-149: The complete restructuring of the
ABI
field inGovernanceModuleMetaData
is significant. This change likely indicates a major version update or a significant refactor of the governance module. It's critical to ensure that all dependent systems and interfaces are updated to interact correctly with the new contract ABI.contracts/bindings/cosmos/precompile/staking/i_staking_module.abigen.go (1)
- 123-129: The ABI definition for
StakingModuleMetaData
has been updated to a more concise format. This change should be verified to ensure that it matches the new contract ABI specification and that all dependent code has been updated to interact with the contract using the new ABI structure.contracts/bindings/testing/consume_gas.abigen.go (1)
- 31-37: The
ConsumeGasMetaData
variable's ABI field has been restructured. Ensure that the new ABI is correctly implemented in the contract and that all interactions with theconsumeGas
function are updated accordingly to reflect these changes.contracts/bindings/testing/distribution_testing_helper.abigen.go (1)
- 31-38: The
DistributionWrapperMetaData
variable's ABI and Bin fields have been updated. It is crucial to verify that the new ABI and bytecode match the updated contract and that all interactions with the contract are compatible with these changes.contracts/bindings/testing/governance/governance_wrapper.abigen.go (1)
- 79-85: The ABI for
GovernanceWrapperMetaData
has been significantly altered. It is important to ensure that the new ABI accurately reflects the updated contract interface and that all dependent systems are updated to interact with the contract using the new ABI.cosmos/runtime/logging.go (5)
31-38: The
ethHandler
struct is correctly defined to implement theslog.Handler
interface. Thelogger
field is of typelog.Logger
, which is appropriate for logging operations.40-44: The
newEthHandler
function is a constructor forethHandler
and correctly returns aslog.Handler
interface. This is a standard pattern for creating new instances in Go.46-50: The
With
method correctly returns a newethHandler
with additional context. This is a common pattern for context logging.77-85: The
WithAttrs
method correctly creates a newethHandler
with additional attributes. This follows the builder pattern commonly used in Go for constructing objects with various options.87-91: The
WithGroup
method is a convenience wrapper aroundWithAttrs
for adding a group name to the logging context. This is a good use of method chaining to simplify the API.
// Handle is a method on ethHandler that logs a message at the | ||
// appropriate level with context key/value pairs. | ||
func (h *ethHandler) Handle(_ context.Context, r slog.Record) error { | ||
polarisGethHandler := h.logger.With("module", "polaris-geth") | ||
x := r.NumAttrs() | ||
attrs := make([]interface{}, 0, x*2) //nolint:gomnd // 2 times. | ||
r.Attrs(func(a slog.Attr) bool { | ||
attrs = append(attrs, a.Key) | ||
attrs = append(attrs, a.Value) | ||
x-- | ||
return x != -1 | ||
}) | ||
switch r.Level { //nolint:nolintlint,exhaustive // linter is bugged. | ||
case slog.LevelDebug: | ||
polarisGethHandler.Debug(r.Message, attrs...) | ||
case slog.LevelInfo: | ||
polarisGethHandler.Info(r.Message, attrs...) | ||
case slog.LevelWarn: | ||
polarisGethHandler.Error(r.Message, attrs...) | ||
case slog.LevelError: | ||
polarisGethHandler.Error(r.Message, attrs...) | ||
} | ||
return nil |
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.
The Handle
method has a potential issue with the polarisGethHandler
variable being redeclared for each log level. This could be optimized by declaring it once outside the switch statement. Additionally, the Error
method is used for both slog.LevelWarn
and slog.LevelError
, which might be a mistake as warnings typically should not be logged as errors.
func (h *ethHandler) Handle(_ context.Context, r slog.Record) error {
polarisGethHandler := h.logger.With("module", "polaris-geth")
// ... rest of the code ...
switch r.Level {
case slog.LevelDebug:
polarisGethHandler.Debug(r.Message, attrs...)
case slog.LevelInfo:
polarisGethHandler.Info(r.Message, attrs...)
case slog.LevelWarn:
- polarisGethHandler.Error(r.Message, attrs...)
+ polarisGethHandler.Warn(r.Message, attrs...)
case slog.LevelError:
polarisGethHandler.Error(r.Message, attrs...)
}
return nil
}
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// Handle is a method on ethHandler that logs a message at the | |
// appropriate level with context key/value pairs. | |
func (h *ethHandler) Handle(_ context.Context, r slog.Record) error { | |
polarisGethHandler := h.logger.With("module", "polaris-geth") | |
x := r.NumAttrs() | |
attrs := make([]interface{}, 0, x*2) //nolint:gomnd // 2 times. | |
r.Attrs(func(a slog.Attr) bool { | |
attrs = append(attrs, a.Key) | |
attrs = append(attrs, a.Value) | |
x-- | |
return x != -1 | |
}) | |
switch r.Level { //nolint:nolintlint,exhaustive // linter is bugged. | |
case slog.LevelDebug: | |
polarisGethHandler.Debug(r.Message, attrs...) | |
case slog.LevelInfo: | |
polarisGethHandler.Info(r.Message, attrs...) | |
case slog.LevelWarn: | |
polarisGethHandler.Error(r.Message, attrs...) | |
case slog.LevelError: | |
polarisGethHandler.Error(r.Message, attrs...) | |
} | |
return nil | |
// Handle is a method on ethHandler that logs a message at the | |
// appropriate level with context key/value pairs. | |
func (h *ethHandler) Handle(_ context.Context, r slog.Record) error { | |
polarisGethHandler := h.logger.With("module", "polaris-geth") | |
x := r.NumAttrs() | |
attrs := make([]interface{}, 0, x*2) //nolint:gomnd // 2 times. | |
r.Attrs(func(a slog.Attr) bool { | |
attrs = append(attrs, a.Key) | |
attrs = append(attrs, a.Value) | |
x-- | |
return x != -1 | |
}) | |
switch r.Level { //nolint:nolintlint,exhaustive // linter is bugged. | |
case slog.LevelDebug: | |
polarisGethHandler.Debug(r.Message, attrs...) | |
case slog.LevelInfo: | |
polarisGethHandler.Info(r.Message, attrs...) | |
case slog.LevelWarn: | |
polarisGethHandler.Warn(r.Message, attrs...) | |
case slog.LevelError: | |
polarisGethHandler.Error(r.Message, attrs...) | |
} | |
return nil | |
} |
// Enabled reports whether l emits log records at the given context and level. | ||
func (*ethHandler) Enabled(_ context.Context, _ slog.Level) bool { | ||
return true |
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.
The Enabled
method always returns true
, which means all log levels are enabled. This might not be desirable in production environments where debug logs could be voluminous. Consider adding a configuration option to control this behavior.
// Enabled should be configurable based on the desired log level.
func (*ethHandler) Enabled(_ context.Context, level slog.Level) bool {
return level >= configuredLogLevel
}
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- contracts/foundry.toml
Files selected for processing (16)
- contracts/bindings/cosmos/lib/cosmos_types.abigen.go (1 hunks)
- contracts/bindings/testing/consume_gas.abigen.go (1 hunks)
- contracts/bindings/testing/distribution_testing_helper.abigen.go (1 hunks)
- contracts/bindings/testing/governance/governance_wrapper.abigen.go (1 hunks)
- contracts/bindings/testing/liquid_staking.abigen.go (1 hunks)
- contracts/bindings/testing/precompile_constructor.abigen.go (1 hunks)
- contracts/bindings/testing/solmate_erc20.abigen.go (1 hunks)
- contracts/src/cosmos/CosmosTypes.sol (1 hunks)
- contracts/src/cosmos/precompile/Bank.sol (1 hunks)
- contracts/src/cosmos/precompile/Distribution.sol (1 hunks)
- contracts/src/cosmos/precompile/Governance.sol (1 hunks)
- contracts/src/cosmos/precompile/Staking.sol (1 hunks)
- contracts/src/cosmos/precompile/examples/Deploy.sol (1 hunks)
- contracts/src/cosmos/precompile/examples/LiquidStaking.sol (1 hunks)
- contracts/src/testing/ConsumeGas.sol (1 hunks)
- contracts/src/testing/SolmateERC20.sol (1 hunks)
Files not summarized due to errors (3)
- contracts/bindings/testing/governance/governance_wrapper.abigen.go: Error: Message exceeds token limit
- contracts/bindings/testing/liquid_staking.abigen.go: Error: Message exceeds token limit
- contracts/bindings/testing/solmate_erc20.abigen.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (5)
- contracts/src/cosmos/CosmosTypes.sol
- contracts/src/cosmos/precompile/Governance.sol
- contracts/src/cosmos/precompile/examples/Deploy.sol
- contracts/src/cosmos/precompile/examples/LiquidStaking.sol
- contracts/src/testing/ConsumeGas.sol
Files skipped from review as they are similar to previous changes (3)
- contracts/bindings/cosmos/lib/cosmos_types.abigen.go
- contracts/bindings/testing/consume_gas.abigen.go
- contracts/bindings/testing/precompile_constructor.abigen.go
Additional comments: 9
contracts/bindings/testing/distribution_testing_helper.abigen.go (1)
- 34-35: The ABI and Bin fields of the
DistributionWrapperMetaData
variable have been updated. Ensure that the changes in the contract's constructor and function definitions, including their inputs, outputs, and state mutability, are consistent with the intended functionality and that they align with the updated smart contract logic.contracts/bindings/testing/governance/governance_wrapper.abigen.go (1)
- 82-83: The ABI and Bin fields of the
GovernanceWrapperMetaData
variable have been updated. Verify that the changes in the contract's constructor, function definitions, and error messages are correct and reflect the new contract logic accurately.contracts/bindings/testing/liquid_staking.abigen.go (1)
- 34-35: The ABI and Bin fields of the
LiquidStakingMetaData
variable have been updated. It's important to check that the changes in the contract's constructor, function definitions, and events are intentional and correctly implemented in the smart contract.contracts/bindings/testing/solmate_erc20.abigen.go (2)
- 31-38: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [26-35]
The ABI string for
SolmateERC20MetaData
has been updated. Ensure that the ABI changes are consistent with the intended contract interface modifications and that all dependent systems are updated to interact correctly with the new ABI.
- 31-38: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [26-26]
The
pragma solidity
version has been updated to0.8.23
. Confirm that all contract code is compatible with this compiler version and that no features or behaviors have changed in a way that affects the contract's functionality.contracts/src/cosmos/precompile/Bank.sol (1)
- 23-29: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [21-21]
The
pragma solidity
version has been updated to0.8.23
. Verify that the contract code is compatible with the new compiler version and that there are no changes in the Solidity language between versions0.8.22
and0.8.23
that could affect the contract's behavior.contracts/src/cosmos/precompile/Distribution.sol (1)
- 21-21: The
pragma solidity
version has been updated to0.8.23
. It is important to ensure that the contract code is fully compatible with this new compiler version and that there are no unexpected changes in behavior due to the version change.contracts/src/cosmos/precompile/Staking.sol (1)
- 26-26: The
pragma solidity
version has been updated to0.8.23
. Confirm that the contract code is compatible with the new compiler version and that there are no changes in the Solidity language between versions0.8.22
and0.8.23
that could affect the contract's behavior.contracts/src/testing/SolmateERC20.sol (1)
- 26-26: The
pragma solidity
version has been updated to0.8.23
. Ensure that the contract code is compatible with this new compiler version and that there are no changes in the Solidity language between versions0.8.22
and0.8.23
that could affect the contract's behavior.
Summary by CodeRabbit
Refactor
Documentation
ABI
field documentation in multiple contracts to reflect the updated contract interfaces and structures.Style
ABI
field for improved clarity and maintainability.