Skip to content

Commit

Permalink
fix(evm): StateDB multistore cache for precompile reversion and a saf…
Browse files Browse the repository at this point in the history
…er Nibiru bank keeper that respects the EVM (#2094)

* statedb: add cacheing for multistore before precompile runs

* messy, working first version that allows for precompile reversion

* wip!: Save checkpoint.
1. Created NibiruBankKeeper with safety around NIBI transfers inside of
   EthereumTx.
2. The "PrecompileCalled" JournalChange now has a propery implementation
   and a strong test case to show that reverting the precompile calls
   works as intended.
3. Remove unneeded functions created for testing with low-level struct
   fields.

* chore: changelog

* finalize bank keeper changes

* revert to previous commit 7f904a0

* fix strange ignored file issue

* remove new bank keeper

* chore: comments from self-review
  • Loading branch information
Unique-Divine authored Oct 29, 2024
1 parent 16393ca commit f3cbcae
Show file tree
Hide file tree
Showing 25 changed files with 563 additions and 223 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
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
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)),
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
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

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
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
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
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,
statedb.NewEmptyTxConfig(
gethcommon.BytesToHash(deps.Ctx.HeaderHash().Bytes()),
),
Expand Down
203 changes: 114 additions & 89 deletions x/evm/evmtest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,95 +26,6 @@ import (
"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 @@ func TransferWei(
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 @@ func ValidLegacyTx() *evm.LegacyTx {
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{},
}
}

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{},
}
}

// 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)
}

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)
}
6 changes: 1 addition & 5 deletions x/evm/keeper/erc20.go
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

0 comments on commit f3cbcae

Please sign in to comment.