Skip to content

feat: add EVM migration support and refactor tx building pipeline#14

Open
akobrin1 wants to merge 1 commit intomainfrom
evm-support
Open

feat: add EVM migration support and refactor tx building pipeline#14
akobrin1 wants to merge 1 commit intomainfrom
evm-support

Conversation

@akobrin1
Copy link
Copy Markdown
Contributor

@akobrin1 akobrin1 commented Apr 3, 2026

Summary

Add EVM migration module support, refactor the transaction building pipeline into composable stages, and replace the local ethsecp256k1 implementation with the upstream cosmos/evm package.

Changes

EVM Migration Module

  • New EVMigrationClient in blockchain/evmigration.go with query operations:
    • Params — current module parameters
    • MigrationRecord — lookup by legacy address
    • MigrationRecordByNewAddress — lookup by new address
    • MigrationEstimate — pre-flight migration estimate
    • MigrationStats — aggregate statistics
  • Wired into blockchain.Client alongside existing Action/SuperNode/Claim modules

Transaction Building Refactor

  • Introduced TxBuildOptions struct for fine-grained tx control:
    • Multi-message support (Messages []sdk.Msg)
    • Explicit AccountNumber/Sequence (skip on-chain query when provided)
    • GasLimit override & SkipSimulation flag
    • Custom FeeAmount
  • Decomposed buildAndSignTx into composable stages:
    • PrepareTx — builds unsigned builder + resolves signer metadata
    • SignPreparedTx — signs a prepared builder with explicit signer info
    • BuildAndSignTxWithOptions — end-to-end convenience
  • Added BroadcastAndWait for broadcast + wait-for-inclusion in a single call
  • Existing BuildAndSignTx / BuildAndSignTxWithGasAdjustment delegate to new pipeline (backwards compatible)

Crypto / ethsecp256k1

  • Removed local pkg/crypto/ethsecp256k1/ package
  • Replaced with upstream github.com/cosmos/evm/crypto/ethsecp256k1
  • Updated NewKeyring to use evmhd.EthSecp256k1Option() and evmcryptocodec.RegisterInterfaces
  • Registered additional module interfaces in NewDefaultTxConfig (bank, staking, distribution, authz, claim, supernode) for proper encoding/decoding

Config & Defaults

  • Moved applyConfigDefaults into base.New, applying MaxRecvMsgSize/MaxSendMsgSize (50 MB) defaults
  • Consolidated WaitTx defaults application

Dependency Bumps

Package Old New
Go 1.25.5 1.26.1
cosmos-sdk v0.53.5 v0.53.6
cometbft v0.38.20 v0.38.21
gRPC v1.77.0 v1.79.2
LumeraProtocol/lumera v1.10.0 v1.11.0
cosmos/evm v0.6.0 (new)

Cleanup

  • Removed deprecated blockchain/messages.go
  • Updated docs/DEVELOPER_GUIDE.md and .github/copilot-instructions.md for Go 1.26.1

Tests

  • TestBuildAndSignTxWithOptions_ManualSignerInfo — offline signing with explicit account/sequence
  • TestBuildAndSignTxWithOptions_QueriesAccountInfoAndSimulates — full pipeline with mock gRPC
  • TestNewAppliesDefaultMessageSizes — validates config defaults

Note

Local replace directives for lumera and supernode are active for development. These should be commented out before merging/releasing.

@roomote-v0
Copy link
Copy Markdown

roomote-v0 bot commented Apr 3, 2026

Rooviewer Clock   See task

Reviewed the EVM migration module, tx building refactor, crypto/ethsecp256k1 replacement, and dependency bumps. Found 2 issues to address before merging.

  • go.mod: Local replace directives for lumera and supernode are active (uncommented) -- this will break any consumer of the module. Comment them out before merging.
  • blockchain/base/tx.go: int64(gas) cast in resolveFeeAmount can silently overflow for large uint64 gas values, now that GasLimit is user-controllable via TxBuildOptions.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

- Add EVMigration module client with query operations (Params,
  MigrationRecord, MigrationRecordByNewAddress, MigrationEstimate,
  MigrationStats) and wire it into blockchain.Client

- Refactor tx building into composable stages:
  - PrepareTx: builds unsigned tx builder and resolves signer metadata
  - SignPreparedTx: signs a prepared builder with explicit signer info
  - BuildAndSignTxWithOptions: end-to-end build+sign with TxBuildOptions
  - BroadcastAndWait: broadcast + wait-for-inclusion convenience method
  - Support explicit AccountNumber/Sequence, GasLimit, SkipSimulation,
    FeeAmount, and multi-message transactions via TxBuildOptions

- Replace local ethsecp256k1 implementation with cosmos/evm upstream
  (github.com/cosmos/evm/crypto/ethsecp256k1), removing
  pkg/crypto/ethsecp256k1/ package

- Register additional module interfaces in NewDefaultTxConfig (bank,
  staking, distribution, authz, claim, supernode) for proper tx
  encoding/decoding

- Bump dependencies:
  - Go 1.25.5 -> 1.26.1
  - cosmos-sdk v0.53.5 -> v0.53.6
  - cometbft v0.38.20 -> v0.38.21
  - grpc v1.77.0 -> v1.79.2
  - LumeraProtocol/lumera v1.10.0 -> v1.11.0
  - Add github.com/cosmos/evm v0.6.0

- Add comprehensive tests for tx building (manual signer info,
  queried account info + simulation, default message sizes)

- Apply config defaults (MaxRecvMsgSize/MaxSendMsgSize) in base.New
- Remove deprecated blockchain/messages.go
- Update docs and copilot-instructions for Go 1.26.1
Comment on lines +9 to +10
github.com/LumeraProtocol/lumera => ../lumera
github.com/LumeraProtocol/supernode/v2 => ../supernode
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local replace directives are uncommented (active). As the PR description itself notes, these must be commented out before merging. With these active, any consumer running go get github.com/LumeraProtocol/sdk-go will fail because ../lumera and ../supernode don't exist on their machine. This is a merge blocker.

Suggested change
github.com/LumeraProtocol/lumera => ../lumera
github.com/LumeraProtocol/supernode/v2 => ../supernode
// Local development - uncomment these for local testing
// Comment these lines before releasing
// github.com/LumeraProtocol/lumera => ../lumera
// github.com/LumeraProtocol/supernode/v2 => ../supernode

Fix it with Roo Code or mention @roomote and request a fix.

if !opts.FeeAmount.Empty() {
return opts.FeeAmount, nil
}
feeDec := c.config.GasPrice.MulInt64(int64(gas)).Ceil().TruncateInt()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int64(gas) silently wraps to a negative number when gas > math.MaxInt64. Now that GasLimit is directly user-controllable via TxBuildOptions, a caller passing a very large value (or a value computed from a large simulation result with a high adjustment factor) would produce an incorrect fee. Consider adding a bounds check or using sdkmath.NewIntFromUint64(gas) to avoid the cast.

Suggested change
feeDec := c.config.GasPrice.MulInt64(int64(gas)).Ceil().TruncateInt()
feeDec := c.config.GasPrice.MulInt(sdkmath.NewIntFromUint64(gas)).Ceil().TruncateInt()

Fix it with Roo Code or mention @roomote and request a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant