Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(evm): StateDB multistore cache for precompile reversion and a safer Nibiru bank keeper that respects the EVM #2094

Merged
merged 10 commits into from
Oct 29, 2024
+563 −223
Merged

fix(evm): StateDB multistore cache for precompile reversion and a safer Nibiru bank keeper that respects the EVM #2094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
295a2d9
statedb: add cacheing for multistore before precompile runs
Unique-Divine Oct 25, 2024
80dd2d7
messy, working first version that allows for precompile reversion
Unique-Divine Oct 26, 2024
c624912
wip!: Save checkpoint.
Unique-Divine Oct 26, 2024
7f904a0
chore: changelog
Unique-Divine Oct 26, 2024
08a73ee
finalize bank keeper changes
Unique-Divine Oct 28, 2024
717ce1c
revert to previous commit 7f904a07ac4d5555d8c088411024fc50ff65d085
Unique-Divine Oct 29, 2024
04a6897
fix strange ignored file issue
Unique-Divine Oct 29, 2024
7c34423
remove new bank keeper
Unique-Divine Oct 29, 2024
4e3bf3c
chore: comments from self-review
Unique-Divine Oct 29, 2024
576cf6c
Merge branch 'main' into ud/db-cache
k-yang Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ consistent setup and dynamic gas calculations, addressing the following tickets.
- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution
- [#2091](https://github.com/NibiruChain/nibiru/pull/2091) - feat(evm): add fun token creation fee validation
- [#2094](https://github.com/NibiruChain/nibiru/pull/2094) - fix(evm): Following
from the changs in #2086, this pull request implements a new `JournalChange`
struct that saves a deep copy of the state multi store before each
state-modifying, Nibiru-specific precompiled contract is called (`OnRunStart`).
Additionally, we commit the `StateDB` there as well. This guarantees that the
non-EVM and EVM state will be in sync even if there are complex, multi-step
Ethereum transactions, such as in the case of an EthereumTx that influences the
`StateDB`, then calls a precompile that also changes non-EVM state, and then EVM
reverts inside of a try-catch.
- [#2098](https://github.com/NibiruChain/nibiru/pull/2098) - test(evm): statedb tests for race conditions within funtoken precompile

#### Nibiru EVM | Before Audit 1 - 2024-10-18
Expand Down
7 changes: 5 additions & 2 deletions app/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ type AppKeepers struct {
}

type privateKeepers struct {
bankBaseKeeper bankkeeper.BaseKeeper
capabilityKeeper *capabilitykeeper.Keeper
slashingKeeper slashingkeeper.Keeper
crisisKeeper crisiskeeper.Keeper
Expand Down Expand Up @@ -262,13 +263,15 @@ func (app *NibiruApp) InitKeepers(
sdk.GetConfig().GetBech32AccountAddrPrefix(),
govModuleAddr,
)
app.BankKeeper = bankkeeper.NewBaseKeeper(

app.bankBaseKeeper = bankkeeper.NewBaseKeeper(
appCodec,
keys[banktypes.StoreKey],
app.AccountKeeper,
BlockedAddresses(),
govModuleAddr,
)
app.BankKeeper = app.bankBaseKeeper
app.StakingKeeper = stakingkeeper.NewKeeper(
appCodec,
keys[stakingtypes.StoreKey],
Expand Down Expand Up @@ -605,7 +608,7 @@ func (app *NibiruApp) initAppModules(
),
auth.NewAppModule(appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts, app.GetSubspace(authtypes.ModuleName)),
vesting.NewAppModule(app.AccountKeeper, app.BankKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
bank.NewAppModule(appCodec, app.bankBaseKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
Copy link
Member Author

@Unique-Divine Unique-Divine Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This line's required to appease the module manager when it does a type assertion on an object of type bankkeeper.Keeper (an interface), asserting that it's actually a bankkeeper.BaseKeeper struct. I didn't see a simple way to force x/bank to use the NibiruBankKeeper struct without editing the Bank module directly, so I opted for what's written here, which is quite simple.

Edit: Moved the NibiruBankKeeper to #2095. The rationale behind these lines are in that PR

capability.NewAppModule(appCodec, *app.capabilityKeeper, false),
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
Expand Down
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ require (
replace (
cosmossdk.io/api => cosmossdk.io/api v0.3.1

github.com/CosmWasm/wasmd => github.com/NibiruChain/wasmd v0.44.0-nibiru
github.com/cosmos/cosmos-sdk => github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru
k-yang marked this conversation as resolved.
Show resolved Hide resolved

github.com/cosmos/iavl => github.com/cosmos/iavl v0.20.0

github.com/ethereum/go-ethereum => github.com/NibiruChain/go-ethereum v1.10.27-nibiru
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,6 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
github.com/BurntSushi/toml v1.1.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/ChainSafe/go-schnorrkel v1.0.0 h1:3aDA67lAykLaG1y3AOjs88dMxC88PgUuHRrLeDnvGIM=
github.com/ChainSafe/go-schnorrkel v1.0.0/go.mod h1:dpzHYVxLZcp8pjlV+O+UR8K0Hp/z7vcchBSbMBEhCw4=
github.com/CosmWasm/wasmd v0.44.0 h1:2sbcoCAvfjCs1O0SWt53xULKjkV06dbSFthEViIC6Zg=
github.com/CosmWasm/wasmd v0.44.0/go.mod h1:tDyYN050qUcdd7LOxGeo2e185sEShyO3nJGl2Cf59+k=
github.com/CosmWasm/wasmvm v1.5.5 h1:XlZI3xO5iUhiBqMiyzsrWEfUtk5gcBMNYIdHnsTB+NI=
github.com/CosmWasm/wasmvm v1.5.5/go.mod h1:Q0bSEtlktzh7W2hhEaifrFp1Erx11ckQZmjq8FLCyys=
github.com/DATA-DOG/go-sqlmock v1.3.3/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM=
Expand All @@ -237,8 +235,12 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
github.com/NibiruChain/collections v0.5.0 h1:33pXpVTe1PK/tfdZlAJF1JF7AdzGNARG+iL9G/z3X7k=
github.com/NibiruChain/collections v0.5.0/go.mod h1:43L6yjuF0BMre/mw4gqn/kUOZz1c2Y3huZ/RQfBFrOQ=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru h1:PgFpxDe+7+OzWHs4zXlml5j2i9sGq2Zpd3ndYQG29/0=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru/go.mod h1:ADjORYzUQqQv/FxDi0H0K5gW/rAk1CiDR3ZKsExfJV0=
github.com/NibiruChain/go-ethereum v1.10.27-nibiru h1:o6lRFt57izoYwzN5cG8tnnBtJcaO3X7MjjN7PGGNCFg=
github.com/NibiruChain/go-ethereum v1.10.27-nibiru/go.mod h1:kvvL3nDceUcB+1qGUBAsVf5dW23RBR77fqxgx2PGNrQ=
github.com/NibiruChain/wasmd v0.44.0-nibiru h1:b+stNdbMFsl0+o4KedXyF83qRnEpB/jCiTGZZgv2h2U=
github.com/NibiruChain/wasmd v0.44.0-nibiru/go.mod h1:inrbdsixQ0Kdu4mFUg1u7fn3XPOEkzqieGv0H/gR0ck=
github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw=
github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk=
github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE=
Expand Down Expand Up @@ -426,8 +428,6 @@ github.com/cosmos/cosmos-db v1.0.2 h1:hwMjozuY1OlJs/uh6vddqnk9j7VamLv+0DBlbEXbAK
github.com/cosmos/cosmos-db v1.0.2/go.mod h1:Z8IXcFJ9PqKK6BIsVOB3QXtkKoqUOp1vRvPT39kOXEA=
github.com/cosmos/cosmos-proto v1.0.0-beta.5 h1:eNcayDLpip+zVLRLYafhzLvQlSmyab+RC5W7ZfmxJLA=
github.com/cosmos/cosmos-proto v1.0.0-beta.5/go.mod h1:hQGLpiIUloJBMdQMMWb/4wRApmI9hjHH05nefC0Ojec=
github.com/cosmos/cosmos-sdk v0.47.11 h1:0Qx7eORw0RJqPv+mvDuU8NQ1LV3nJJKJnPoYblWHolc=
github.com/cosmos/cosmos-sdk v0.47.11/go.mod h1:ADjORYzUQqQv/FxDi0H0K5gW/rAk1CiDR3ZKsExfJV0=
github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y=
github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY=
github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw=
Expand Down
13 changes: 0 additions & 13 deletions x/evm/deps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package evm
import (
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
bank "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

Expand Down Expand Up @@ -32,18 +31,6 @@ type AccountKeeper interface {
SetModuleAccount(ctx sdk.Context, macc authtypes.ModuleAccountI)
}

// BankKeeper defines the expected interface needed to retrieve account balances.
type BankKeeper interface {
authtypes.BankKeeper
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin
SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error

GetDenomMetaData(ctx sdk.Context, denom string) (metadata bank.Metadata, isFound bool)
SetDenomMetaData(ctx sdk.Context, denomMetaData bank.Metadata)
}

// StakingKeeper returns the historical headers kept in store.
type StakingKeeper interface {
GetHistoricalInfo(ctx sdk.Context, height int64) (stakingtypes.HistoricalInfo, bool)
Expand Down
3 changes: 2 additions & 1 deletion x/evm/evmtest/test_deps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ func NewTestDeps() TestDeps {
}

func (deps TestDeps) StateDB() *statedb.StateDB {
return statedb.New(deps.Ctx, &deps.App.EvmKeeper,
return deps.EvmKeeper.NewStateDB(
deps.Ctx,
Comment on lines +49 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Direct statedb.New usage needs to be migrated to EvmKeeper.NewStateDB

Found inconsistent StateDB creation patterns that should be standardized:

  • app/evmante/evmante_can_transfer.go: Uses direct statedb.New instead of EvmKeeper.NewStateDB
  • x/evm/statedb/statedb_test.go: Uses direct statedb.New in tests

The only legitimate direct usage is in x/evm/keeper/statedb.go which implements the NewStateDB method itself.

🔗 Analysis chain

LGTM! Improved StateDB creation pattern.

This change properly delegates StateDB creation to EvmKeeper, ensuring test scenarios use the same state management path as production code. This alignment strengthens test reliability and supports the PR's goal of safer state handling.

Let's verify consistent StateDB creation patterns across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any direct statedb.New calls that should use EvmKeeper.NewStateDB
# Expected: Only EvmKeeper should directly call statedb.New

# Search for direct statedb.New usage
echo "Checking for direct statedb.New usage..."
rg "statedb\.New\(" -A 3

# Search for EvmKeeper.NewStateDB usage
echo "Checking for EvmKeeper.NewStateDB usage..."
rg "\.NewStateDB\(" -A 3

Length of output: 1451

statedb.NewEmptyTxConfig(
gethcommon.BytesToHash(deps.Ctx.HeaderHash().Bytes()),
),
Expand Down
203 changes: 114 additions & 89 deletions x/evm/evmtest/tx.go
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved these to the bottom since the workhorse functions in this file are below this

Original file line number Diff line number Diff line change
Expand Up @@ -26,95 +26,6 @@
"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
)

type GethTxType = uint8

func TxTemplateAccessListTx() *gethcore.AccessListTx {
return &gethcore.AccessListTx{
GasPrice: big.NewInt(1),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}
}

func TxTemplateLegacyTx() *gethcore.LegacyTx {
return &gethcore.LegacyTx{
GasPrice: big.NewInt(1),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}
}

func TxTemplateDynamicFeeTx() *gethcore.DynamicFeeTx {
return &gethcore.DynamicFeeTx{
GasFeeCap: big.NewInt(10),
GasTipCap: big.NewInt(2),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}
}

func NewEthTxMsgFromTxData(
deps *TestDeps,
txType GethTxType,
innerTxData []byte,
nonce uint64,
to *gethcommon.Address,
value *big.Int,
gas uint64,
accessList gethcore.AccessList,
) (*evm.MsgEthereumTx, error) {
if innerTxData == nil {
innerTxData = []byte{}
}

var ethCoreTx *gethcore.Transaction
switch txType {
case gethcore.LegacyTxType:
innerTx := TxTemplateLegacyTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
ethCoreTx = gethcore.NewTx(innerTx)
case gethcore.AccessListTxType:
innerTx := TxTemplateAccessListTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.AccessList = accessList
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
ethCoreTx = gethcore.NewTx(innerTx)
case gethcore.DynamicFeeTxType:
innerTx := TxTemplateDynamicFeeTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
innerTx.AccessList = accessList
ethCoreTx = gethcore.NewTx(innerTx)
default:
return nil, fmt.Errorf(
"received unknown tx type (%v) in NewEthTxMsgFromTxData", txType)
}

ethTxMsg := new(evm.MsgEthereumTx)
if err := ethTxMsg.FromEthereumTx(ethCoreTx); err != nil {
return ethTxMsg, err
}

ethTxMsg.From = deps.Sender.EthAddr.Hex()
return ethTxMsg, ethTxMsg.Sign(deps.GethSigner(), deps.Sender.KeyringSigner)
}

// ExecuteNibiTransfer executes nibi transfer
func ExecuteNibiTransfer(deps *TestDeps, t *testing.T) *evm.MsgEthereumTx {
nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr)
Expand Down Expand Up @@ -326,6 +237,10 @@
return err
}

// --------------------------------------------------
// Templates
// --------------------------------------------------

// ValidLegacyTx: Useful initial condition for tests
// Exported only for use in tests.
func ValidLegacyTx() *evm.LegacyTx {
Expand All @@ -342,3 +257,113 @@
S: []byte{},
}
}

// GethTxType represents different Ethereum transaction types as defined in
// go-ethereum, such as Legacy, AccessList, and DynamicFee transactions.
type GethTxType = uint8

func TxTemplateAccessListTx() *gethcore.AccessListTx {
return &gethcore.AccessListTx{
GasPrice: big.NewInt(1),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}

Check warning on line 272 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L265-L272 

Added lines #L265 - L272 were not covered by tests
}

func TxTemplateLegacyTx() *gethcore.LegacyTx {
return &gethcore.LegacyTx{
GasPrice: big.NewInt(1),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}
}

func TxTemplateDynamicFeeTx() *gethcore.DynamicFeeTx {
return &gethcore.DynamicFeeTx{
GasFeeCap: big.NewInt(10),
GasTipCap: big.NewInt(2),
Gas: gethparams.TxGas,
To: &gethcommon.Address{},
Value: big.NewInt(0),
Data: []byte{},
}

Check warning on line 293 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L285-L293 

Added lines #L285 - L293 were not covered by tests
}

// NewEthTxMsgFromTxData creates an Ethereum transaction message based on
// the specified txType (Legacy, AccessList, or DynamicFee). This function
// populates transaction fields like nonce, recipient, value, and gas, with
// an optional access list for AccessList and DynamicFee types. The transaction
// is signed using the provided dependencies.
//
// Parameters:
// - deps: Required dependencies including the sender address and signer.
// - txType: Transaction type (Legacy, AccessList, or DynamicFee).
// - innerTxData: Byte slice of transaction data (input).
// - nonce: Transaction nonce.
// - to: Recipient address.
// - value: ETH value (in wei) to transfer.
// - gas: Gas limit for the transaction.
// - accessList: Access list for AccessList and DynamicFee types.
//
// Returns:
// - *evm.MsgEthereumTx: Ethereum transaction message ready for submission.
// - error: Any error encountered during creation or signing.
func NewEthTxMsgFromTxData(
deps *TestDeps,
txType GethTxType,
innerTxData []byte,
nonce uint64,
to *gethcommon.Address,
value *big.Int,
gas uint64,
accessList gethcore.AccessList,
) (*evm.MsgEthereumTx, error) {
if innerTxData == nil {
innerTxData = []byte{}
}

var ethCoreTx *gethcore.Transaction
switch txType {
case gethcore.LegacyTxType:
innerTx := TxTemplateLegacyTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
ethCoreTx = gethcore.NewTx(innerTx)
case gethcore.AccessListTxType:
innerTx := TxTemplateAccessListTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.AccessList = accessList
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
ethCoreTx = gethcore.NewTx(innerTx)
case gethcore.DynamicFeeTxType:
innerTx := TxTemplateDynamicFeeTx()
innerTx.Nonce = nonce
innerTx.Data = innerTxData
innerTx.To = to
innerTx.Value = value
innerTx.Gas = gas
innerTx.AccessList = accessList
ethCoreTx = gethcore.NewTx(innerTx)
default:
return nil, fmt.Errorf(
"received unknown tx type (%v) in NewEthTxMsgFromTxData", txType)

Check warning on line 359 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L339-L359 

Added lines #L339 - L359 were not covered by tests
}

ethTxMsg := new(evm.MsgEthereumTx)
if err := ethTxMsg.FromEthereumTx(ethCoreTx); err != nil {
return ethTxMsg, err
}

Check warning on line 365 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L364-L365 

Added lines #L364 - L365 were not covered by tests

ethTxMsg.From = deps.Sender.EthAddr.Hex()
return ethTxMsg, ethTxMsg.Sign(deps.GethSigner(), deps.Sender.KeyringSigner)
}
Comment on lines +315 to +369
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding input validation and refactoring common field assignments.

The function is well-documented and handles different transaction types correctly. Consider these improvements:

  1. Add validation for the value parameter to prevent potential issues with nil or negative values.
  2. Extract common field assignments to reduce code duplication.

Here's a suggested refactoring:

 func NewEthTxMsgFromTxData(
     deps *TestDeps,
     txType GethTxType,
     innerTxData []byte,
     nonce uint64,
     to *gethcommon.Address,
     value *big.Int,
     gas uint64,
     accessList gethcore.AccessList,
 ) (*evm.MsgEthereumTx, error) {
+    if value == nil {
+        return nil, fmt.Errorf("value cannot be nil")
+    }
+    if value.Sign() < 0 {
+        return nil, fmt.Errorf("negative value not allowed")
+    }
+
     if innerTxData == nil {
         innerTxData = []byte{}
     }
 
+    // Common fields for all transaction types
+    commonFields := struct {
+        nonce uint64
+        data  []byte
+        to    *gethcommon.Address
+        value *big.Int
+        gas   uint64
+    }{nonce, innerTxData, to, value, gas}
+
     var ethCoreTx *gethcore.Transaction
     switch txType {
     case gethcore.LegacyTxType:
         innerTx := TxTemplateLegacyTx()
-        innerTx.Nonce = nonce
-        innerTx.Data = innerTxData
-        innerTx.To = to
-        innerTx.Value = value
-        innerTx.Gas = gas
+        innerTx.Nonce = commonFields.nonce
+        innerTx.Data = commonFields.data
+        innerTx.To = commonFields.to
+        innerTx.Value = commonFields.value
+        innerTx.Gas = commonFields.gas
         ethCoreTx = gethcore.NewTx(innerTx)
     // ... similar changes for other cases

Committable suggestion was skipped due to low confidence.

6 changes: 1 addition & 5 deletions x/evm/keeper/erc20.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,8 @@ func (k Keeper) CallContractWithInput(
// sent by a user
txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0)))

// Using tmp context to not modify the state in case of evm revert
tmpCtx, commitCtx := ctx.CacheContext()

evmResp, evmObj, err = k.ApplyEvmMsg(
tmpCtx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig,
ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig,
)
if err != nil {
// We don't know the actual gas used, so consuming the gas limit
Expand All @@ -245,7 +242,6 @@ func (k Keeper) CallContractWithInput(
} else {
// Success, committing the state to ctx
if commit {
commitCtx()
totalGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
if err != nil {
k.ResetGasMeterAndConsumeGas(ctx, ctx.GasMeter().Limit())
Expand Down
Loading
Loading