[chore] bump peggy proto #319
Conversation
* feat: removed the deprecated pre-commit repo pre-commit-golang. Added new CI step to run golangci-lint using the github action * fix: added skip directories into the golanci lint config * fix: fixed lint errors * fix: fixed lint errors * feat: updated proto definitions for the chain upgrade v1.15 * fix: synced go.mod dependecnies with Injective core * fix: fixed lint errors * fix: fixed lint errors * feat: updated all proto definitions for v1.15 chain upgrade. Added an example for the new GetEipBaseFee request in txfees module * feat: updated all examples to query gas price from the chain instead of using the default value * Feat/sync with master v1.56.2 (#298) * (fix) Applied the fixes for EIP712 JSON unmarshalling to the master branch * fix: update pre-commit/action from v2.0.2 to v3.0.1 to ry to solve the pre-commit validation workflow error * fix: Added configuration in .golangci.yml file to exclude some dirs * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: Added quantization in the functions that convert notional values to chain format * fix/fix_manual_tx_generation (#296) * fix: fixed the manual TX generation and signing to receive the gas price * fix: solve issues in pre-commit workflow caused by using deprecated components * fix: solve issues in pre-commit workflow caused by using deprecated components * feat: updated proto definitions for the chain upgrade v1.15 * (fix) Applied the fixes for EIP712 JSON unmarshalling to the master branch * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: updated the oracle stream prices script to send the oracle type always in lowercase * fix: synced go.mod dependecnies with Injective core * fix: fixed lint errors * fix: fixed lint errors * feat: updated all proto definitions for v1.15 chain upgrade. Added an example for the new GetEipBaseFee request in txfees module * feat: updated all examples to query gas price from the chain instead of using the default value * fix: fix function parameter type * feat: updated proto definition to v1.15.0-beta.2 and v1.14.48 for Indexer * fix: update ofac.json file * Improve error messaging and enhance documentation in eip712_cosmos.go (#240) This PR improves error handling and enhances documentation in the eip712_cosmos.go file to provide clearer messages and more detailed explanations. Signed-off-by: Hwangjae Lee <meetrick@gmail.com> Co-authored-by: Abel Armoa <30988000+aarmoa@users.noreply.github.com> * fix: fixed array creatin in a unit test (#299) * fix: updated chain version to v1.15 and indexer version to v1.15.6 * fix: removed explicit inclusion of txfees response in the context initialization (not required) --------- Signed-off-by: Hwangjae Lee <meetrick@gmail.com> Co-authored-by: Hwangjae Lee <meetrick@gmail.com>
…r` branch of injective-core (future v1.16)
…ctive core (for v1.16)
… endpoint (the /websocket destination is added internally now)
… human readable values in their config
…w reduce margin ratio for derivative markets
Feat/add exchange v2 support
…_v1_57 Chore/sync dev with master v1.57
…d examples for the new queries and messages.
[CP-395] add support for v1.16
…s (with fully qualified name) every time the proto definitions are updated
Feat/add chain messages list
WalkthroughThis update introduces extensive new functionality and refactoring related to Ethereum Virtual Machine (EVM) support and Ethereum transaction handling within a Cosmos SDK-based environment. It adds new Go packages and files for EVM types, transaction formats, events, error handling, and precompiled contract bindings. The update also revises linter configurations, Makefile targets, and workflow dependencies. Minor stylistic and receiver changes are applied to existing crypto and RPC code. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CosmosSDK
participant EVMModule
participant EthereumTx
participant GoEthereum
participant ContractBinding
User->>CosmosSDK: Submit MsgEthereumTx
CosmosSDK->>EVMModule: Route message
EVMModule->>EthereumTx: Convert to go-ethereum TxData
EthereumTx->>GoEthereum: Validate and sign transaction
EVMModule->>ContractBinding: (Optional) Interact with precompiled contract
EVMModule-->>CosmosSDK: Return MsgEthereumTxResponse (with logs/errors)
CosmosSDK-->>User: Deliver transaction result
Poem
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 (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (1)
chain/evm/precompiles/exchange/i_exchange_module.abigen.go (1)
32-36: Duplicate type definition detectedThe
CosmosCoinstruct is identically defined here and in the staking module binding. This creates a duplicate type definition that could cause compatibility issues when both modules are used together.This is the same issue identified in the staking module. Consider consolidating shared types into a common package to avoid type conflicts and maintain consistency across modules.
🧹 Nitpick comments (12)
chain/evm/types/tx.go (1)
16-29: Consider edge case handling and documentation improvements.The function has good overflow protection, but there are a few considerations:
- The function always returns
math.MaxInt64whenpriorityBigdoesn't fit in int64, which might not be the intended behavior for very high gas prices.- The function comment mentions "priority reduction global variable" but the actual variable name is
DefaultPriorityReduction.Consider this improvement for better edge case handling:
func GetTxPriority(msg *MsgEthereumTx) (priority int64) { - // calculate priority based on gas price + // Calculate priority based on gas price using DefaultPriorityReduction gasPrice := msg.AsTransaction().GasPrice() - priority = math.MaxInt64 + // Default to max priority for edge cases priorityBig := new(big.Int).Quo(gasPrice, DefaultPriorityReduction.BigInt()) - // safety check + // Safety check to prevent overflow if priorityBig.IsInt64() { priority = priorityBig.Int64() + } else { + priority = math.MaxInt64 } return priority }Makefile (1)
156-165: Robust message extraction logic with minor improvement suggestion.The
extract-message-namestarget is well-implemented with proper error handling and informative output. The regex pattern correctly filters out Response messages.Consider adding error handling for the
jqcommand:extract-message-names: @echo "Extracting message names from tx.pb.go files..." @mkdir -p injective_data @find ./chain -name "tx.pb.go" -exec grep -h "proto\.RegisterType" {} \; | \ sed -n 's/.*proto\.RegisterType([^"]*"\([^"]*\)".*/\1/p' | \ grep -v 'Response$$' | \ sort -u | \ - jq -R -s 'split("\n")[:-1]' > injective_data/chain_messages_list.json + jq -R -s 'split("\n")[:-1]' > injective_data/chain_messages_list.json || { echo "Error: jq failed"; exit 1; }chain/evm/types/storage.go (1)
32-40: Consider performance optimization for String method.The current string concatenation approach could be inefficient for large storage collections due to repeated string allocations.
Consider using
strings.Builderfor better performance:// String implements the stringer interface func (s Storage) String() string { - var str string + var builder strings.Builder for _, state := range s { - str += fmt.Sprintf("%s\n", state.String()) + builder.WriteString(state.String()) + builder.WriteByte('\n') } - return str + return builder.String() }chain/evm/types/eth.go (1)
66-84: Comprehensive validation with minor improvement opportunity.The validation logic correctly checks gas parameters and costs. However, consider enhancing readability by extracting the validation error pattern.
Consider extracting a helper function to reduce duplication:
+func validateInt256(value *big.Int, errType error, fieldName string) error { + if !types.IsValidInt256(value) { + return errorsmod.Wrap(errType, fmt.Sprintf("%s out of bound", fieldName)) + } + return nil +} func (tx EthereumTx) Validate() error { // prevent txs with 0 gas to fill up the mempool if tx.Gas() == 0 { return errorsmod.Wrap(ErrInvalidGasLimit, "gas limit must not be zero") } - if !types.IsValidInt256(tx.GasPrice()) { - return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") - } - if !types.IsValidInt256(tx.GasFeeCap()) { - return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") - } - if !types.IsValidInt256(tx.GasTipCap()) { - return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") - } - if !types.IsValidInt256(tx.Cost()) { - return errorsmod.Wrap(ErrInvalidGasFee, "out of bound") - } + if err := validateInt256(tx.GasPrice(), ErrInvalidGasPrice, "gas price"); err != nil { + return err + } + if err := validateInt256(tx.GasFeeCap(), ErrInvalidGasPrice, "gas fee cap"); err != nil { + return err + } + if err := validateInt256(tx.GasTipCap(), ErrInvalidGasPrice, "gas tip cap"); err != nil { + return err + } + if err := validateInt256(tx.Cost(), ErrInvalidGasFee, "cost"); err != nil { + return err + } return nil }chain/evm/types/logs.go (1)
131-140: Optimize bloom filter calculation for better performance.The current implementation could be more efficient by avoiding repeated slice iterations.
Consider optimizing the bloom filter calculation:
func LogsBloom(logs []*ethtypes.Log) []byte { var bin ethtypes.Bloom for _, log := range logs { bin.Add(log.Address.Bytes()) - for _, b := range log.Topics { - bin.Add(b[:]) - } + for _, topic := range log.Topics { + bin.Add(topic[:]) + } } return bin[:] }While the current implementation is correct, the variable name
bcould be more descriptive astopicfor better readability.chain/evm/types/codec.go (1)
14-22: Consider deprecation warning for AminoCdc.The comment indicates that AminoCodec is deprecated, but the nolint directive suppresses the warning. Consider adding a TODO comment for future migration.
Add a migration note:
// AminoCdc is a amino codec created to support amino JSON compatible msgs. + // TODO: Migrate away from AminoCodec once legacy support is no longer needed AminoCdc = codec.NewAminoCodec(amino) //nolint:staticcheck // AminoCodec is deprecatedchain/evm/types/chain_config.go (1)
160-174: Fix inconsistent field names in error messages.The error wrapping uses inconsistent casing for field names.
- return errorsmod.Wrap(err, "GrayGlacierBlock") + return errorsmod.Wrap(err, "grayGlacierBlock") } if err := ValidateBlock(cc.MergeNetsplitBlock); err != nil { - return errorsmod.Wrap(err, "MergeNetsplitBlock") + return errorsmod.Wrap(err, "mergeNetsplitBlock") } if err := ValidateTime(cc.ShanghaiTime); err != nil { - return errorsmod.Wrap(err, "ShanghaiTime") + return errorsmod.Wrap(err, "shanghaiTime") } if err := ValidateTime(cc.CancunTime); err != nil { - return errorsmod.Wrap(err, "CancunTime") + return errorsmod.Wrap(err, "cancunTime") } if err := ValidateTime(cc.PragueTime); err != nil { - return errorsmod.Wrap(err, "PragueTime") + return errorsmod.Wrap(err, "pragueTime")chain/evm/types/params.go (2)
14-27: Consider security implications of default settings.The defaults have potential security implications:
DefaultAllowUnprotectedTxs = trueallows replay attacks on transactions without EIP-155 protection- The denomination "inj" is Injective-specific and might not be appropriate for other deployments
106-114: Optimize address comparison.The function creates new address objects on each iteration. Consider pre-computing normalized addresses.
Store normalized addresses in a map during validation for O(1) lookup:
// Add to Params struct: authorizedDeployersMap map[string]struct{} // In validation, populate the map // Then in IsAuthorisedDeployer: func (p Params) IsAuthorisedDeployer(addr ethcommon.Address) bool { _, exists := p.authorizedDeployersMap[addr.Hex()] return exists }chain/evm/types/msg.go (2)
178-180: Implement GetMsgsV2 or document why it's not implemented.The method returns "not implemented" error. If this is intentional, add a comment explaining why.
Would you like me to help implement this method or add appropriate documentation?
373-375: Consider migrating from Amino to Protobuf encoding.The method uses
AminoCdcwhich is deprecated in favor of Protobuf encoding.chain/evm/precompiles/staking/i_staking_module.abigen.go (1)
32-36: Consider consolidating shared type definitionsThe
CosmosCoinstruct is identically defined in both the staking and exchange module bindings. This duplication could lead to type compatibility issues when using both modules together.Consider moving shared types like
CosmosCointo a common package that both modules can import, such as:// In a shared types package package types type CosmosCoin struct { Amount *big.Int Denom string }Then import and reference it in both bindings rather than redefining it.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (42)
chain/auction/types/query.pb.gois excluded by!**/*.pb.gochain/auction/types/tx.pb.gois excluded by!**/*.pb.gochain/erc20/types/erc20.pb.gois excluded by!**/*.pb.gochain/erc20/types/events.pb.gois excluded by!**/*.pb.gochain/erc20/types/genesis.pb.gois excluded by!**/*.pb.gochain/erc20/types/params.pb.gois excluded by!**/*.pb.gochain/erc20/types/query.pb.gois excluded by!**/*.pb.gochain/erc20/types/tx.pb.gois excluded by!**/*.pb.gochain/evm/types/access_tuple.pb.gois excluded by!**/*.pb.gochain/evm/types/chain_config.pb.gois excluded by!**/*.pb.gochain/evm/types/events.pb.gois excluded by!**/*.pb.gochain/evm/types/genesis.pb.gois excluded by!**/*.pb.gochain/evm/types/log.pb.gois excluded by!**/*.pb.gochain/evm/types/params.pb.gois excluded by!**/*.pb.gochain/evm/types/query.pb.gois excluded by!**/*.pb.gochain/evm/types/state.pb.gois excluded by!**/*.pb.gochain/evm/types/trace_config.pb.gois excluded by!**/*.pb.gochain/evm/types/transaction_logs.pb.gois excluded by!**/*.pb.gochain/evm/types/tx.pb.gois excluded by!**/*.pb.gochain/evm/types/tx_result.pb.gois excluded by!**/*.pb.gochain/exchange/types/authz.pb.gois excluded by!**/*.pb.gochain/exchange/types/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/genesis.pb.gois excluded by!**/*.pb.gochain/exchange/types/query.pb.gois excluded by!**/*.pb.gochain/exchange/types/tx.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/authz.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/events.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/exchange.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/genesis.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/market.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/order.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/orderbook.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/proposal.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/query.pb.gois excluded by!**/*.pb.gochain/exchange/types/v2/tx.pb.gois excluded by!**/*.pb.gochain/peggy/types/genesis.pb.gois excluded by!**/*.pb.gochain/peggy/types/msgs.pb.gois excluded by!**/*.pb.gochain/peggy/types/rate_limit.pb.gois excluded by!**/*.pb.gochain/stream/types/query.pb.gois excluded by!**/*.pb.gochain/stream/types/v2/query.pb.gois excluded by!**/*.pb.gochain/txfees/types/txfees.pb.gois excluded by!**/*.pb.gochain/types/indexer.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (28)
.github/workflows/golangci-lint.yml(1 hunks).golangci.yml(1 hunks)Makefile(5 hunks)auth_vote/authz_vote.go(1 hunks)chain/crypto/ethsecp256k1/ethsecp256k1.go(2 hunks)chain/crypto/hd/algorithm.go(3 hunks)chain/crypto/hd/hd_path.go(1 hunks)chain/erc20/types/codec.go(1 hunks)chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go(1 hunks)chain/evm/precompiles/bank/i_bank_module.abigen.go(1 hunks)chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go(1 hunks)chain/evm/precompiles/exchange/i_exchange_module.abigen.go(1 hunks)chain/evm/precompiles/staking/i_staking_module.abigen.go(1 hunks)chain/evm/types/access_list.go(1 hunks)chain/evm/types/access_list_tx.go(1 hunks)chain/evm/types/chain_config.go(1 hunks)chain/evm/types/codec.go(1 hunks)chain/evm/types/dynamic_fee_tx.go(1 hunks)chain/evm/types/errors.go(1 hunks)chain/evm/types/eth.go(1 hunks)chain/evm/types/events.go(1 hunks)chain/evm/types/key.go(1 hunks)chain/evm/types/legacy_tx.go(1 hunks)chain/evm/types/logs.go(1 hunks)chain/evm/types/msg.go(1 hunks)chain/evm/types/params.go(1 hunks)chain/evm/types/storage.go(1 hunks)chain/evm/types/tx.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
chain/crypto/ethsecp256k1/ethsecp256k1.go (1)
chain/crypto/ethsecp256k1/keys.pb.go (6)
PrivKey(75-77)PrivKey(81-81)PrivKey(82-84)PubKey(30-32)PubKey(35-35)PubKey(36-38)
chain/evm/types/events.go (1)
chain/evm/types/msg.go (1)
TypeMsgEthereumTx(40-40)
chain/evm/types/tx.go (2)
chain/evm/types/utils.go (1)
DefaultPriorityReduction(23-23)chain/evm/types/errors.go (1)
ErrExecutionReverted(49-49)
chain/evm/types/eth.go (3)
chain/evm/types/msg.go (1)
NewTx(48-53)chain/types/int.go (1)
IsValidInt256(59-61)chain/evm/types/errors.go (1)
ErrInvalidGasFee(85-85)
chain/evm/types/logs.go (2)
chain/types/validation.go (1)
IsEmptyHash(12-14)chain/evm/types/utils.go (1)
HexAddress(236-241)
chain/evm/types/chain_config.go (2)
chain/evm/types/chain_config.pb.go (9)
ChainConfig(29-85)ChainConfig(89-89)ChainConfig(90-92)BlobScheduleConfig(141-146)BlobScheduleConfig(150-150)BlobScheduleConfig(151-153)BlobConfig(209-213)BlobConfig(217-217)BlobConfig(218-220)chain/evm/types/errors.go (1)
ErrInvalidChainConfig(55-55)
chain/evm/types/dynamic_fee_tx.go (5)
chain/types/int.go (2)
SafeNewIntFromBigInt(43-48)IsValidInt256(59-61)chain/evm/types/access_list.go (2)
AccessList(10-10)NewAccessList(14-34)chain/evm/types/errors.go (2)
ErrInvalidGasCap(97-97)ErrInvalidGasFee(85-85)typeddata/typed_data.go (1)
ErrInvalidChainID(1285-1285)chain/evm/types/utils.go (1)
EffectiveGasPrice(227-233)
chain/evm/precompiles/bank/i_bank_module.abigen.go (1)
ethereum/util/contract.go (2)
BoundContract(27-35)Contract(15-23)
chain/evm/types/access_list_tx.go (4)
chain/types/int.go (2)
SafeNewIntFromBigInt(43-48)IsValidInt256(59-61)chain/evm/types/access_list.go (2)
AccessList(10-10)NewAccessList(14-34)chain/evm/types/errors.go (1)
ErrInvalidGasFee(85-85)typeddata/typed_data.go (1)
ErrInvalidChainID(1285-1285)
chain/evm/precompiles/staking/i_staking_module.abigen.go (2)
ethereum/util/contract.go (2)
BoundContract(27-35)Contract(15-23)chain/evm/precompiles/exchange/i_exchange_module.abigen.go (1)
CosmosCoin(33-36)
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (1)
ethereum/util/contract.go (2)
BoundContract(27-35)Contract(15-23)
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (1)
ethereum/util/contract.go (2)
BoundContract(27-35)Contract(15-23)
🪛 checkmake (0.2.2)
Makefile
[warning] 217-217: Missing required phony target "all"
(minphony)
[warning] 217-217: Missing required phony target "clean"
(minphony)
[warning] 217-217: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: run-tests
🔇 Additional comments (45)
auth_vote/authz_vote.go (2)
17-17: LGTM - Appropriate linter disable for example code.The revive disable comment is appropriate for example/demo scripts where function length may exceed typical standards.
20-20: Check rpchttp.New Websocket DefaultsIt appears
rpchttp.Newnow only takes the RPC address (no explicit"/websocket"path). Please verify:
- The import for
rpchttp(e.g.github.com/cometbft/cometbft/rpc/client/http) is updated to the version whereNewaccepts a single argument.- That
New(network.TmEndpoint)still uses the correct default websocket endpoint under the hood.- Websocket-based calls (subscriptions/streams) continue to function as expected.
.github/workflows/golangci-lint.yml (3)
16-16: LGTM - Standard action version update.Upgrading from
actions/checkout@v3to@v4is a standard maintenance update.
22-22: LGTM - Action version update.Upgrading from
golangci/golangci-lint-action@v6to@v8brings latest improvements and bug fixes.
25-25: golangci-lint version v2.1.6 is valid
- Version v2.1.6 was released on May 4, 2025 and exists in the official GitHub releases.
- Although the latest available is v2.2.0 (June 28, 2025), pinning to v2.1.6 is acceptable for reproducible builds.
chain/crypto/ethsecp256k1/ethsecp256k1.go (2)
76-78: LGTM - Improved method receiver style.Using an unnamed receiver is appropriate here since the receiver variable is not used in the method body. This follows Go best practices.
156-158: LGTM - Consistent receiver style improvement.The unnamed receiver change is consistent with the PrivKey.Type() method and follows Go conventions for unused receivers.
chain/crypto/hd/algorithm.go (4)
23-24: LGTM - Updated project references.The comment update from "Ethermint" to "Injective" correctly reflects the project context.
27-28: LGTM - Consistent comment update.The Ledger algorithm comment update maintains consistency with the previous change.
52-54: LGTM - Improved receiver style.Using an unnamed receiver is appropriate since the receiver isn't used in the method body.
97-104: LGTM - Consistent receiver improvement.The unnamed receiver change follows the same style improvement pattern as the Name() method.
chain/evm/types/access_list.go (1)
14-34: LGTM - Well-structured conversion function.The
NewAccessListfunction correctly handles nil input and properly converts between ethereum core and protobuf-compatible formats.chain/evm/types/tx.go (1)
47-52: Revert detection is consistent – no change required
BothNewVMErrorWithRet(in chain/evm/types/errors.go) and theRevertmethod (in chain/evm/types/tx.go) use the same constantvm.ErrExecutionReverted.Error(), so the string comparison is safe and will remain in sync.chain/evm/types/events.go (1)
3-24: Well-structured event constants.The event constants are well-organized and follow consistent naming patterns. Good practice using
TypeMsgEthereumTxfrom the relevant code snippet to maintain consistency across the module.chain/evm/types/storage.go (1)
15-30: Robust validation logic with good duplicate detection.The validation implementation correctly handles duplicate keys and delegates to individual state validation. The error wrapping provides good context for debugging.
.golangci.yml (1)
1-67: Well-structured linter configuration upgrade.The upgrade to version 2 format with restructured settings is a good improvement. The detailed
reviveconfiguration with cognitive complexity limits and line length constraints will help maintain code quality. The exclusions for generated code and common false positives are appropriate.chain/evm/types/eth.go (1)
20-25: LGTM: Nil safety handled correctly.The nil check prevents potential panics when accessing the underlying Transaction fields.
chain/crypto/hd/hd_path.go (2)
7-13: LGTM: Correct Ethereum constants and clear documentation.The EIP-84 reference and BIP44 path usage are correctly implemented. The coin type 60 is the official Ethereum coin type.
21-32: Well-structured path iterator with proper error handling.The function correctly parses the derivation path and provides appropriate iterator types based on the
ledgerIterflag. Error propagation fromParseDerivationPathis handled correctly.chain/erc20/types/codec.go (2)
15-19: Standard message registration pattern implemented correctly.The codec registration follows Cosmos SDK conventions and includes all necessary message types.
35-42: Complete initialization sequence with proper codec registration.The initialization properly registers codecs and seals the Amino codec to prevent further modifications.
chain/evm/types/logs.go (3)
29-47: Thorough validation with good error messaging.The validation logic correctly checks for empty hashes, nil logs, and transaction hash consistency. The error messages are descriptive and helpful for debugging.
91-100: Efficient log conversion with proper capacity allocation.The function correctly pre-allocates the slice capacity and handles empty input gracefully.
118-129: HexAddress usage is intentional and consistentThe custom HexAddress in chain/evm/types/utils.go omits the EIP-55 checksum for performance in state-machine contexts and is used uniformly in the EVM types (e.g. logs.go, msg.go). In contrast, user-facing code and examples rely on common.Address.Hex() for checksummed output. No inconsistencies detected—no changes required.
chain/evm/types/codec.go (2)
64-76: Robust packing function with proper error handling.The function correctly validates the input type and provides meaningful error messages. The type assertion and Any packing logic are implemented correctly.
80-91: Unpacking function handles edge cases well.The function properly checks for nil input and validates the cached value type. The error handling is consistent with the packing function.
chain/evm/types/params.go (1)
170-174: Use normalized address representation for duplicate detection.Using raw bytes as map key could miss duplicates with different case representations.
- if _, found := authorisedAddresses[string(addr.Bytes())]; found { + if _, found := authorisedAddresses[addr.Hex()]; found { return fmt.Errorf("duplicate authorised address: %s", addrStr) } - authorisedAddresses[string(addr.Bytes())] = struct{}{} + authorisedAddresses[addr.Hex()] = struct{}{}Likely an incorrect or invalid review comment.
chain/evm/types/dynamic_fee_tx.go (2)
16-59: LGTM! Well-structured constructor with proper error handling.The constructor correctly converts from go-ethereum transaction types with safe integer conversions to prevent overflow issues.
269-271: LGTM! Correct EIP-1559 effective gas price calculation.The method properly delegates to the utility function that implements the EIP-1559 formula: min(baseFee + tip, feeCap).
chain/evm/types/errors.go (2)
16-40: LGTM! Well-organized error code constants.Good practice reserving code 1 for internal errors and marking code 3 as JSON-RPC compliant for execution reverted errors.
226-256: LGTM! Robust VMError implementation with proper nil handling.The implementation correctly handles both ErrExecutionReverted and other VM errors, with appropriate JSON encoding for ABCIInfo integration.
chain/evm/precompiles/bank/i_bank_module.abigen.go (1)
1-3: Auto-generated code looks standard.This file follows the expected abigen patterns for Ethereum contract bindings.
chain/evm/types/legacy_tx.go (1)
204-217: LGTM! Correct implementation for legacy transactions.The effective price methods correctly return the same values as their base counterparts since legacy transactions don't use dynamic pricing.
chain/evm/precompiles/bank/mint_burn_bank_erc20.abigen.go (1)
1-3: Auto-generated code follows standard patterns.This abigen-generated binding for MintBurnBankERC20 contract looks correct.
chain/evm/precompiles/staking/i_staking_module.abigen.go (1)
1-317: LGTM - Standard abigen-generated bindingThis auto-generated Go binding follows the expected patterns and structure for Ethereum contract interactions. The code correctly implements the standard abigen interface with proper separation of concerns (caller, transactor, filterer) and session variants.
chain/evm/precompiles/exchange/i_exchange_module.abigen.go (1)
1-926: LGTM - Comprehensive auto-generated exchange bindingThis auto-generated binding provides comprehensive coverage of the exchange contract functionality with proper type safety and method variants. The extensive struct definitions and method implementations follow abigen best practices.
chain/evm/precompiles/bank/fixed_supply_bank_erc20.abigen.go (9)
1-30: Generated code follows proper abigen patterns.The header, package declaration, and imports are correctly structured according to go-ethereum binding generation standards. The "DO NOT EDIT" warning and reference imports to suppress errors are appropriate patterns for generated code.
32-44: Metadata appears complete and well-structured.The metadata includes a comprehensive ABI with all standard ERC20 functions, events, and error types. The deprecated variables provide appropriate backward compatibility while maintaining the standard abigen pattern.
46-61: Deployment function follows correct abigen patterns.The deployment function properly handles ABI parsing, includes comprehensive error handling, and correctly initializes all binding components (Caller, Transactor, Filterer). The function signature matches the contract constructor requirements.
63-120: Type definitions provide comprehensive interaction patterns.The struct definitions cover all standard go-ethereum binding patterns: embedded caller/transactor/filterer in the main contract type, session types for convenience, and raw types for low-level access. This provides appropriate flexibility for different interaction patterns.
122-165: Constructor functions provide complete binding options.The constructor functions cover all standard binding patterns with proper error handling. The use of a helper function for the actual binding operation follows good DRY principles while providing specialized constructors for different use cases.
167-203: Raw methods provide proper low-level contract access.The raw interaction methods correctly delegate to the underlying BoundContract and follow standard go-ethereum patterns for Call, Transfer, and Transact operations. The method signatures and error handling are consistent with the binding framework.
205-389: View method bindings provide complete ERC20 read functionality.All standard ERC20 view functions are properly implemented with appropriate variants for different usage patterns. The type conversions and error handling follow correct go-ethereum binding patterns, providing reliable access to contract state.
391-452: Transaction method bindings provide complete ERC20 write functionality.All standard ERC20 transaction functions are properly implemented with appropriate variants. The use of TransactOpts and return of transaction objects follows correct patterns for paid contract interactions.
454-760: Event handling provides comprehensive log processing capabilities.The event handling implementation covers all standard patterns for Ethereum event processing: filtering, watching, parsing, and iteration. The subscription management properly handles errors, completion, and termination scenarios. Both Approval and Transfer events are fully supported with consistent patterns.
| func (al AccessList) ToEthAccessList() *ethtypes.AccessList { | ||
| var ethAccessList ethtypes.AccessList | ||
|
|
||
| for _, tuple := range al { | ||
| storageKeys := make([]common.Hash, len(tuple.StorageKeys)) | ||
|
|
||
| for i := range tuple.StorageKeys { | ||
| storageKeys[i] = common.HexToHash(tuple.StorageKeys[i]) | ||
| } | ||
|
|
||
| ethAccessList = append(ethAccessList, ethtypes.AccessTuple{ | ||
| Address: common.HexToAddress(tuple.Address), | ||
| StorageKeys: storageKeys, | ||
| }) | ||
| } | ||
|
|
||
| return ðAccessList | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for invalid hex conversions.
The ToEthAccessList function performs hex string conversions that could fail with invalid input. Consider adding validation or error handling.
-func (al AccessList) ToEthAccessList() *ethtypes.AccessList {
+func (al AccessList) ToEthAccessList() (*ethtypes.AccessList, error) {
var ethAccessList ethtypes.AccessList
for _, tuple := range al {
storageKeys := make([]common.Hash, len(tuple.StorageKeys))
for i := range tuple.StorageKeys {
+ if !common.IsHexAddress(tuple.StorageKeys[i]) {
+ return nil, fmt.Errorf("invalid hex string for storage key: %s", tuple.StorageKeys[i])
+ }
storageKeys[i] = common.HexToHash(tuple.StorageKeys[i])
}
+ if !common.IsHexAddress(tuple.Address) {
+ return nil, fmt.Errorf("invalid hex address: %s", tuple.Address)
+ }
ethAccessList = append(ethAccessList, ethtypes.AccessTuple{
Address: common.HexToAddress(tuple.Address),
StorageKeys: storageKeys,
})
}
- return ðAccessList
+ return ðAccessList, nil
}You'll also need to import fmt and update all callers to handle the returned error.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In chain/evm/types/access_list.go around lines 38 to 55, the ToEthAccessList
function converts hex strings to hashes and addresses without error handling,
which can cause silent failures on invalid input. Modify the function to return
an error along with the AccessList pointer. Add validation for each hex string
conversion using appropriate parsing functions that return errors, and if any
conversion fails, return the error immediately. Update the function signature to
include an error return value and ensure all callers handle this error. Also,
import the "fmt" package to format error messages.
| lint-all: | ||
| golangci-lint run --timeout=15m -v | ||
|
|
||
| .PHONY: copy-exchange-client tests coverage lint lint-last-commit lint-master lint-all extract-message-names |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address missing .PHONY targets.
The static analysis correctly identifies missing standard .PHONY targets that improve Makefile reliability.
Add the missing .PHONY targets:
-.PHONY: copy-exchange-client tests coverage lint lint-last-commit lint-master lint-all extract-message-names
+.PHONY: all clean test copy-exchange-client tests coverage lint lint-last-commit lint-master lint-all extract-message-namesYou'll also need to define the missing targets:
+all: copy-exchange-client copy-chain-types
+
+clean:
+ rm -rf chain exchange proto injective_data
+
+test: tests
+
copy-exchange-client: clone-injective-indexer📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .PHONY: copy-exchange-client tests coverage lint lint-last-commit lint-master lint-all extract-message-names | |
| .PHONY: all clean test copy-exchange-client tests coverage lint lint-last-commit lint-master lint-all extract-message-names | |
| all: copy-exchange-client copy-chain-types | |
| clean: | |
| <tab>rm -rf chain exchange proto injective_data | |
| test: tests | |
| copy-exchange-client: clone-injective-indexer |
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 217-217: Missing required phony target "all"
(minphony)
[warning] 217-217: Missing required phony target "clean"
(minphony)
[warning] 217-217: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
In the Makefile at line 217, the .PHONY declaration is missing some standard
targets which can cause issues with Makefile reliability. Add all missing
standard .PHONY targets to the list on line 217 to ensure they are treated as
phony targets. Additionally, define the corresponding missing targets in the
Makefile to avoid errors when these targets are invoked.
| func (tx *EthereumTx) Unmarshal(dst []byte) error { | ||
| if len(dst) == 0 { | ||
| tx.Transaction = nil | ||
| return nil | ||
| } | ||
| if tx.Transaction == nil { | ||
| tx.Transaction = new(ethtypes.Transaction) | ||
| } | ||
| return tx.UnmarshalBinary(dst) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential memory issue with nil Transaction initialization.
The current implementation creates a new Transaction on line 45 but doesn't handle the case where UnmarshalBinary might fail after creating the new Transaction instance.
Consider improving error handling:
func (tx *EthereumTx) Unmarshal(dst []byte) error {
if len(dst) == 0 {
tx.Transaction = nil
return nil
}
- if tx.Transaction == nil {
- tx.Transaction = new(ethtypes.Transaction)
- }
- return tx.UnmarshalBinary(dst)
+
+ // Create a temporary transaction to avoid corrupting the current state on error
+ tempTx := new(ethtypes.Transaction)
+ if err := tempTx.UnmarshalBinary(dst); err != nil {
+ return err
+ }
+
+ tx.Transaction = tempTx
+ return nil
}🤖 Prompt for AI Agents
In chain/evm/types/eth.go around lines 39 to 48, the method initializes
tx.Transaction with a new instance before calling UnmarshalBinary, but if
UnmarshalBinary fails, the partially initialized Transaction remains,
potentially causing memory issues. Modify the code to call UnmarshalBinary first
on a temporary variable or handle the error immediately after UnmarshalBinary,
and only assign the new Transaction to tx.Transaction if unmarshaling succeeds,
ensuring no partially initialized state persists.
| func RegisterInterfaces(registry codectypes.InterfaceRegistry) { | ||
| registry.RegisterImplementations( | ||
| (*sdk.Msg)(nil), | ||
| &MsgEthereumTx{}, | ||
| ) | ||
| registry.RegisterImplementations( | ||
| (*tx.TxExtensionOptionI)(nil), | ||
| &ExtensionOptionsEthereumTx{}, | ||
| ) | ||
| registry.RegisterImplementations( | ||
| (*sdk.Msg)(nil), | ||
| &MsgEthereumTx{}, | ||
| &MsgUpdateParams{}, | ||
| ) | ||
| registry.RegisterInterface( | ||
| "injective.evm.v1.TxData", | ||
| (*TxData)(nil), | ||
| &DynamicFeeTx{}, | ||
| &AccessListTx{}, | ||
| &LegacyTx{}, | ||
| ) | ||
|
|
||
| msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate MsgEthereumTx registration detected.
The MsgEthereumTx type is registered twice in the same interface registry, which is unnecessary and could lead to confusion.
Remove the duplicate registration:
func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations(
(*sdk.Msg)(nil),
&MsgEthereumTx{},
)
registry.RegisterImplementations(
(*tx.TxExtensionOptionI)(nil),
&ExtensionOptionsEthereumTx{},
)
registry.RegisterImplementations(
(*sdk.Msg)(nil),
- &MsgEthereumTx{},
&MsgUpdateParams{},
)
// ... rest of the function
}🤖 Prompt for AI Agents
In chain/evm/types/codec.go between lines 36 and 59, the MsgEthereumTx type is
registered twice under the sdk.Msg interface. Remove the first registry call
that registers MsgEthereumTx alone, keeping only the later registration where
MsgEthereumTx and MsgUpdateParams are registered together to avoid duplicate
registrations.
| func ObjectGasUsedKey(txIndex int) []byte { | ||
| var key [1 + 8]byte | ||
| key[0] = prefixObjectGasUsed | ||
| binary.BigEndian.PutUint64(key[1:], uint64(txIndex)) | ||
| return key[:] | ||
| } |
There was a problem hiding this comment.
Add validation for negative index values.
The function converts int to uint64 without checking for negative values. This could lead to unexpected behavior if negative indices are passed.
Add validation:
func ObjectGasUsedKey(txIndex int) []byte {
+ if txIndex < 0 {
+ panic("txIndex cannot be negative")
+ }
var key [1 + 8]byte
key[0] = prefixObjectGasUsed
binary.BigEndian.PutUint64(key[1:], uint64(txIndex))
return key[:]
}📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func ObjectGasUsedKey(txIndex int) []byte { | |
| var key [1 + 8]byte | |
| key[0] = prefixObjectGasUsed | |
| binary.BigEndian.PutUint64(key[1:], uint64(txIndex)) | |
| return key[:] | |
| } | |
| func ObjectGasUsedKey(txIndex int) []byte { | |
| if txIndex < 0 { | |
| panic("txIndex cannot be negative") | |
| } | |
| var key [1 + 8]byte | |
| key[0] = prefixObjectGasUsed | |
| binary.BigEndian.PutUint64(key[1:], uint64(txIndex)) | |
| return key[:] | |
| } |
🤖 Prompt for AI Agents
In chain/evm/types/key.go around lines 65 to 70, the function ObjectGasUsedKey
converts an int txIndex to uint64 without checking for negative values, which
can cause unexpected behavior. Add validation at the start of the function to
check if txIndex is negative; if so, handle it appropriately, such as returning
an error or a nil slice, or panicking, depending on the function's usage
context. This prevents invalid negative indices from being processed.
| tx := msg.AsTransaction() | ||
| txHash := ethSigner.Hash(tx) | ||
|
|
||
| sig, _, err := keyringSigner.SignByAddress(from, txHash.Bytes(), signing.SignMode_SIGN_MODE_TEXTUAL) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the use of SIGN_MODE_TEXTUAL for Ethereum transactions.
Using SIGN_MODE_TEXTUAL for Ethereum transaction signing seems unusual. Ethereum typically uses ECDSA signatures.
🏁 Script executed:
#!/bin/bash
# Check how signing modes are used elsewhere in the codebase
rg -A 5 "SignByAddress.*SIGN_MODE" --type goLength of output: 2281
Use SIGN_MODE_DIRECT for Ethereum transaction signing
The EVM module should use raw ECDSA signing (SIGN_MODE_DIRECT) rather than the textual mode. All other SignByAddress calls (including tests) use SIGN_MODE_DIRECT, so switch this to maintain consistency and produce a valid Ethereum signature.
• File: chain/evm/types/msg.go
Line: 231
- sig, _, err := keyringSigner.SignByAddress(from, txHash.Bytes(), signing.SignMode_SIGN_MODE_TEXTUAL)
+ sig, _, err := keyringSigner.SignByAddress(from, txHash.Bytes(), signing.SignMode_SIGN_MODE_DIRECT)🤖 Prompt for AI Agents
In chain/evm/types/msg.go at line 231, the SignByAddress call uses
SIGN_MODE_TEXTUAL, but it should use SIGN_MODE_DIRECT for Ethereum transaction
signing to ensure consistency and validity. Change the signing mode argument
from signing.SignMode_SIGN_MODE_TEXTUAL to signing.SignMode_SIGN_MODE_DIRECT in
the SignByAddress call.
| // fallback to legacy format | ||
| txData, err := UnpackTxData(msg.Data) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| msg.Raw = NewEthereumTx(txData.AsEthereumData()) | ||
| return msg.Raw.Transaction | ||
| } |
There was a problem hiding this comment.
Handle error from UnpackTxData properly.
The error from UnpackTxData is ignored, which could hide issues and cause the function to return nil.
Consider logging the error or panicking:
txData, err := UnpackTxData(msg.Data)
if err != nil {
- return nil
+ panic(fmt.Sprintf("failed to unpack legacy tx data: %v", err))
}📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // fallback to legacy format | |
| txData, err := UnpackTxData(msg.Data) | |
| if err != nil { | |
| return nil | |
| } | |
| msg.Raw = NewEthereumTx(txData.AsEthereumData()) | |
| return msg.Raw.Transaction | |
| } | |
| // fallback to legacy format | |
| txData, err := UnpackTxData(msg.Data) | |
| if err != nil { | |
| panic(fmt.Sprintf("failed to unpack legacy tx data: %v", err)) | |
| } | |
| msg.Raw = NewEthereumTx(txData.AsEthereumData()) | |
| return msg.Raw.Transaction | |
| } |
🤖 Prompt for AI Agents
In chain/evm/types/msg.go around lines 270 to 277, the error returned from
UnpackTxData is currently ignored, causing the function to return nil silently.
Modify the code to properly handle this error by either logging the error with
relevant context or panicking to avoid hiding the issue. This ensures that any
problems during UnpackTxData are visible and can be addressed promptly.
| if chainID != nil { | ||
| chainIDInt := sdkmath.NewIntFromBigInt(chainID) | ||
| tx.ChainID = &chainIDInt | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for chain ID bounds.
The chain ID is converted without checking if it fits within valid bounds.
if chainID != nil {
+ if !types.IsValidInt256(chainID) {
+ panic("chain ID out of bounds")
+ }
chainIDInt := sdkmath.NewIntFromBigInt(chainID)
tx.ChainID = &chainIDInt
}📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if chainID != nil { | |
| chainIDInt := sdkmath.NewIntFromBigInt(chainID) | |
| tx.ChainID = &chainIDInt | |
| } | |
| if chainID != nil { | |
| if !types.IsValidInt256(chainID) { | |
| panic("chain ID out of bounds") | |
| } | |
| chainIDInt := sdkmath.NewIntFromBigInt(chainID) | |
| tx.ChainID = &chainIDInt | |
| } |
🤖 Prompt for AI Agents
In chain/evm/types/access_list_tx.go around lines 177 to 180, the chain ID is
converted to an integer without validating if it falls within acceptable bounds.
Add validation logic to check that the chain ID is within the valid range before
assigning it to tx.ChainID. If the chain ID is out of bounds, handle the error
appropriately to prevent invalid values from being set.
| if tx.GetChainID() == nil { | ||
| return errorsmod.Wrap( | ||
| errortypes.ErrInvalidChainID, | ||
| "chain ID must be present on AccessList txs", | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix the error message - this is DynamicFeeTx, not AccessListTx.
The error message incorrectly refers to AccessList transactions.
- "chain ID must be present on AccessList txs",
+ "chain ID must be present on DynamicFee txs",🤖 Prompt for AI Agents
In chain/evm/types/dynamic_fee_tx.go around lines 248 to 253, the error message
incorrectly mentions "AccessList txs" instead of "DynamicFeeTx". Update the
error message string to correctly refer to "DynamicFeeTx" to accurately reflect
the transaction type.
| // AsEthereumData returns an AccessListTx transaction tx from the proto-formatted | ||
| // TxData defined on the Cosmos EVM. | ||
| func (tx *LegacyTx) AsEthereumData() ethtypes.TxData { |
There was a problem hiding this comment.
Fix the incorrect comment - this returns LegacyTx, not AccessListTx.
-// AsEthereumData returns an AccessListTx transaction tx from the proto-formatted
+// AsEthereumData returns a LegacyTx transaction tx from the proto-formatted
// TxData defined on the Cosmos EVM.🤖 Prompt for AI Agents
In chain/evm/types/legacy_tx.go around lines 123 to 125, the comment incorrectly
states that the function returns an AccessListTx transaction, but it actually
returns a LegacyTx. Update the comment to accurately describe that the function
returns a LegacyTx transaction from the proto-formatted TxData defined on the
Cosmos EVM.
|
@dbrajovic |
Checked out from tag:
v1.58.0-evm-precompiles-comet1-rc2Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores