refactor!: Using the MsgEthereumTx interface in the rpc module

[nowooj]

Description

This code doesn't cover all MsgEthereumTx implementations within the rpc module, but it does cover the query part. This code can be expanded and structured into a completely independent code structure.

Removing Hash() hasn't important to do with the PR, but it was added for backward compatibility since ethermint uses Hash as a variable.

Closes: #853

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• tackled an existing issue or discussed with a team member
• left instructions on how to review the changes
• targeted the main branch

[vladjdk]

@nowooj a few conflicts here

[vladjdk]

Why are we leaving some *evmtypes.MsgEthereumTx as pointers instead of the interface?

[nowooj]

I thought that newly execute tx, for example tx coming through ante handler or mempool, would always use the latest evmtypes.MsgEthereumTx

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

[codecov]

Codecov Report

❌ Patch coverage is 69.69697% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.80%. Comparing base (f93d05c) to head (4c48768).
⚠️ Report is 31 commits behind head on main.

Files with missing lines	Patch %	Lines
rpc/backend/comet_to_eth.go	75.00%	2 Missing and 1 partial ⚠️
rpc/types/utils.go	25.00%	3 Missing ⚠️
rpc/backend/utils.go	0.00%	2 Missing ⚠️
rpc/backend/tx_info.go	85.71%	1 Missing ⚠️
rpc/types/events.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #854      +/-   ##
==========================================
+ Coverage   63.24%   65.80%   +2.55%     
==========================================
  Files         332      332              
  Lines       23969    23743     -226     
==========================================
+ Hits        15160    15624     +464     
+ Misses       7173     6951     -222     
+ Partials     1636     1168     -468     

Files with missing lines	Coverage Δ
ante/evm/11_emit_event.go	100.00% <100.00%> (ø)
ante/tx_listener.go	80.00% <100.00%> (ø)
indexer/kv_indexer.go	73.21% <100.00%> (ø)
mempool/krakatoa_mempool.go	80.07% <100.00%> (+2.33%)	⬆️
mempool/mempool.go	70.56% <100.00%> (+40.07%)	⬆️
rpc/backend/backend.go	55.55% <ø> (+15.07%)	⬆️
x/vm/keeper/msg_server.go	97.59% <100.00%> (ø)
x/vm/types/msg.go	73.33% <ø> (-0.32%)	⬇️
rpc/backend/tx_info.go	52.94% <85.71%> (ø)
rpc/types/events.go	77.57% <0.00%> (ø)
... and 3 more

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vladjdk]

Lint and conflicts

[vladjdk]

@greptile review

[greptile-apps]

Greptile Summary

This PR introduces a new IMsgEthereumTx interface in rpc/types/interfaces and migrates the RPC query layer (and adjacent packages) from direct *evmtypes.MsgEthereumTx type assertions to the interface, as well as replacing the now-removed Hash() helper with AsTransaction().Hash() throughout.

• P1 — rpc/backend/comet_to_eth.go:298: common.Address(ethMsg.GetFrom().Bytes()) uses Go 1.20 slice-to-array conversion which panics at runtime if GetFrom() returns an empty byte slice; the previous ethMsg.GetSender() returned a zero address safely. Use common.BytesToAddress(ethMsg.GetFrom().Bytes()) instead.
• P1 — x/vm/types/msg.go now imports rpc/types/interfaces, inverting the layer dependency (core domain → RPC layer); the interface should be defined inside the x/vm/types package or a neutral shared package.

Confidence Score: 3/5

Not safe to merge as-is due to a potential runtime panic and an inverted architectural dependency.

Two P1 findings: a runtime panic risk from common.Address(GetFrom().Bytes()) replacing the safe GetSender() call, and a layer-inversion where the core domain package x/vm/types now imports rpc/types/interfaces. Multiple P1s pull the score below the 4/5 ceiling.

rpc/backend/comet_to_eth.go (panic risk on contract creation address) and x/vm/types/msg.go (inverted import dependency).

Important Files Changed

Filename	Overview
rpc/types/interfaces/interface.go	New interface IMsgEthereumTx introduced; includes mutation methods (FromEthereumTx, FromSignedEthereumTx, UnmarshalBinary) that are never called via this interface in the rpc module, violating interface segregation.
x/vm/types/msg.go	Adds compile-time assertion that MsgEthereumTx satisfies interfaces.IMsgEthereumTx and imports rpc/types/interfaces; removes Hash() helper. The import creates an inverted dependency (core domain → RPC layer).
rpc/backend/comet_to_eth.go	Changes EthMsgsFromCometBlock and ReceiptsFromCometBlock to use IMsgEthereumTx; replaces ethMsg.GetSender() with common.Address(ethMsg.GetFrom().Bytes()) which can panic on empty From.
rpc/backend/tx_info.go	Type assertions updated from *evmtypes.MsgEthereumTx to interfaces.IMsgEthereumTx; hash comparisons updated to use AsTransaction().Hash(); no logic changes.
rpc/types/utils.go	RawTxToEthTx, NewTransactionFromMsg, and RPCMarshalBlock updated to use []interfaces.IMsgEthereumTx; functionally equivalent.
rpc/types/events.go	Unchecked type assertion updated to use interfaces.IMsgEthereumTx; behavior is identical — still panics if non-ethereum msg is passed (pre-existing).
rpc/backend/utils.go	Type assertions in getAccountNonce and ProcessBlock updated to interface; no logic changes.
ante/evm/11_emit_event.go	Replaces msg.Hash() (now removed) with msg.AsTransaction().Hash(); semantically identical.
mempool/krakatoa_mempool.go	Replaces msgEthereumTx.Hash() with msgEthereumTx.AsTransaction().Hash(); no logic change.
mempool/mempool.go	Replaces two ethMsg.Hash() calls with ethMsg.AsTransaction().Hash(); no logic change.
rpc/backend/backend.go	Interface method EthMsgsFromCometBlock return type updated to []interfaces.IMsgEthereumTx.
tests/integration/rpc/backend/test_backend_suite.go	Test helpers updated to use []interfaces.IMsgEthereumTx slices; no logic changes.
tests/integration/rpc/backend/test_tx_info.go	Test assertions updated to use AsTransaction().Hash(); no logic changes.
rpc/backend/tx_info_test.go	Unit tests updated to use interface slice and AsTransaction().Hash(); no logic changes.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class IMsgEthereumTx {
        <<interface>>
        +FromEthereumTx(tx)
        +FromSignedEthereumTx(tx, signer) error
        +GetFrom() AccAddress
        +GetGas() uint64
        +GetEffectiveFee(baseFee) *big.Int
        +AsTransaction() *Transaction
        +GetSenderLegacy(signer) Address,error
        +AsMessage(baseFee) *Message
        +UnmarshalBinary(b, signer) error
    }

    class MsgEthereumTx {
        +From []byte
        +Raw EthereumTx
        +FromEthereumTx(tx)
        +FromSignedEthereumTx(tx, signer) error
        +GetFrom() AccAddress
        +GetGas() uint64
        +AsTransaction() *Transaction
    }

    class Backend {
        +EthMsgsFromCometBlock() []IMsgEthereumTx
        +ReceiptsFromCometBlock(msgs []IMsgEthereumTx)
    }

    class RPCUtils {
        +RawTxToEthTx() []IMsgEthereumTx
        +NewTransactionFromMsg(msg IMsgEthereumTx)
        +RPCMarshalBlock(msgs []IMsgEthereumTx)
    }

    MsgEthereumTx ..|> IMsgEthereumTx : implements
    Backend --> IMsgEthereumTx : uses
    RPCUtils --> IMsgEthereumTx : uses
    note for MsgEthereumTx "x/vm/types now imports rpc/types/interfaces (inverted dependency)"

Loading

_{Reviews (1): Last reviewed commit: "refactor: remove Hash() func for backwar..." | Re-trigger Greptile}

[greptile-apps]

Unsafe slice-to-array conversion may panic

common.Address(ethMsg.GetFrom().Bytes()) is a Go 1.20 slice-to-array conversion that panics at runtime if len(GetFrom().Bytes()) < 20. While msg.From is normally 20 bytes, an uninitialized or malformed message (e.g., msg.From == nil) will cause a nil-dereference / bounds panic here, whereas the old ethMsg.GetSender() would safely return a zero address. Since GetSender() is not part of IMsgEthereumTx, use the safe helper instead:

Suggested change

	contractAddress = crypto.CreateAddress(common.Address(ethMsg.GetFrom().Bytes()), tx.Nonce())
	contractAddress = crypto.CreateAddress(common.BytesToAddress(ethMsg.GetFrom().Bytes()), tx.Nonce())

[nowooj]

d6e7fb6

[greptile-apps]

Inverted dependency: core domain imports RPC layer

x/vm/types (core domain) now imports rpc/types/interfaces (API/RPC layer). This inverts the normal layering direction — RPC packages should depend on domain types, not the reverse. Any consumer that uses x/vm/types without the RPC stack will now transitively pull in the entire rpc/types/interfaces package. The interface definition should live in a neutral package (e.g. inside x/vm/types itself, or a standalone types/interfaces under the vm module) so the domain layer stays independent of the RPC layer.

[nowooj]

cfab9c8

[greptile-apps]

Interface includes unused mutating methods

FromEthereumTx, FromSignedEthereumTx, and UnmarshalBinary are construction/mutation methods. None of the consumers in the rpc module (query paths in comet_to_eth.go, tx_info.go, utils.go, events.go) ever call these methods via the interface — they only call AsTransaction(), GetFrom(), GetGas(), and GetSenderLegacy(). Including write methods in a read-oriented query interface violates the Interface Segregation Principle, makes it harder to satisfy the interface with a read-only wrapper, and leaks implementation concerns into the abstraction.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[nowooj]

bcbd9af

[aljo242]

ci has not run: fork pr, all workflows action_required. also 34 commits behind main.

Hash() removal is a bundled breaking change: the pr description acknowledges this was kept for ethermint backward compat. removing it breaks any downstream calling .Hash() directly on *MsgEthereumTx. move this to a separate pr with an explicit changelog entry.

undocumented behavioral change in contract address derivation: comet_to_eth.go:299 changes ethMsg.GetSender() (recovered from signature) to ethMsg.GetFrom() (stored field). these should be equivalent for well-formed txs, but the difference isn't documented and could silently produce wrong contract addresses for edge cases. needs a comment or test confirming equivalence.

GetSenderLegacy in interface but unused: no call site in this diff uses it through the interface -- either document why it's required or remove it.

[nowooj]

ci has not run: fork pr, all workflows action_required. also 34 commits behind main.

Hash() removal is a bundled breaking change: the pr description acknowledges this was kept for ethermint backward compat. removing it breaks any downstream calling .Hash() directly on *MsgEthereumTx. move this to a separate pr with an explicit changelog entry.

undocumented behavioral change in contract address derivation: comet_to_eth.go:299 changes ethMsg.GetSender() (recovered from signature) to ethMsg.GetFrom() (stored field). these should be equivalent for well-formed txs, but the difference isn't documented and could silently produce wrong contract addresses for edge cases. needs a comment or test confirming equivalence.

GetSenderLegacy in interface but unused: no call site in this diff uses it through the interface -- either document why it's required or remove it.

@aljo242 Thanks, addressed the actionable parts.

• Added an API-BREAKING changelog entry for the MsgEthereumTx.Hash() removal and documented AsTransaction().Hash() as the replacement.
• Updated contract creation receipt address derivation to use GetSenderLegacy(signer) instead of GetFrom() directly, preserving the fallback sender recovery for older decoded txs with an empty From.
• GetSenderLegacy is used through RPCMsgEthereumTxI in RPC/query paths (getAccountNonce, GetTransactionReceipt, GetBlockReceipts), so I kept it in the interface.
• The branch has also been updated with latest main.