diff --git a/CHANGELOG.md b/CHANGELOG.md index da4a32363..5ee83562c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Nibiru EVM -#### Nibiru EVM | Before Audit 2 [Nov, 2024] +- [#2119](https://github.com/NibiruChain/nibiru/pull/2119) - fix(evm): Guarantee +that gas consumed during any send operation of the "NibiruBankKeeper" depends +only on the "bankkeeper.BaseKeeper"'s gas consumption. + +#### Nibiru EVM | Before Audit 2 - 2024-12-06 The codebase went through a third-party [Code4rena Zenith](https://code4rena.com/zenith) Audit, running from 2024-10-07 until diff --git a/app/ante/commission_test.go b/app/ante/commission_test.go index 9cbd18982..fd4d6ca50 100644 --- a/app/ante/commission_test.go +++ b/app/ante/commission_test.go @@ -1,8 +1,6 @@ package ante_test import ( - "testing" - "cosmossdk.io/math" sdkclienttx "github.com/cosmos/cosmos-sdk/client/tx" sdk "github.com/cosmos/cosmos-sdk/types" @@ -109,7 +107,7 @@ func (s *AnteTestSuite) TestAnteDecoratorStakingCommission() { wantErr: ante.ErrMaxValidatorCommission.Error(), }, } { - s.T().Run(tc.name, func(t *testing.T) { + s.Run(tc.name, func() { txGasCoins := sdk.NewCoins( sdk.NewCoin("unibi", math.NewInt(1_000)), sdk.NewCoin("utoken", math.NewInt(500)), diff --git a/app/ante/fixed_gas_test.go b/app/ante/fixed_gas_test.go index 0d11b3b2d..4e45f6b6b 100644 --- a/app/ante/fixed_gas_test.go +++ b/app/ante/fixed_gas_test.go @@ -11,7 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/cosmos/cosmos-sdk/x/bank/types" + bank "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/NibiruChain/nibiru/v2/app/ante" "github.com/NibiruChain/nibiru/v2/app/appconst" @@ -62,7 +62,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { Feeder: addr.String(), Validator: addr.String(), }, - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), @@ -74,7 +74,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { { name: "Two messages in a transaction, one of them is an oracle vote message should fail (with MsgAggregateExchangeRatePrevote) permutation 2", messages: []sdk.Msg{ - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), @@ -97,7 +97,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { Feeder: addr.String(), Validator: addr.String(), }, - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), @@ -109,7 +109,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { { name: "Two messages in a transaction, one of them is an oracle vote message should fail (with MsgAggregateExchangeRateVote) permutation 2", messages: []sdk.Msg{ - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), @@ -169,7 +169,7 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { Feeder: addr.String(), Validator: addr.String(), }, - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), @@ -186,25 +186,25 @@ func (suite *AnteTestSuite) TestOraclePostPriceTransactionsHaveFixedPrice() { { name: "Other two messages", messages: []sdk.Msg{ - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 100)), }, - &types.MsgSend{ + &bank.MsgSend{ FromAddress: addr.String(), ToAddress: addr.String(), Amount: sdk.NewCoins(sdk.NewInt64Coin(appconst.BondDenom, 200)), }, }, - expectedGas: 67193, + expectedGas: 38175, expectedErr: nil, }, } for _, tc := range tests { tc := tc - suite.T().Run(tc.name, func(t *testing.T) { + suite.Run(tc.name, func() { suite.SetupTest() // setup suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() diff --git a/app/evmante/suite_test.go b/app/evmante/suite_test.go index 2150aebe3..621778582 100644 --- a/app/evmante/suite_test.go +++ b/app/evmante/suite_test.go @@ -60,7 +60,7 @@ func (s *TestSuite) TestGenesis() { wantErr: "min_commission must be positive", }, } { - s.T().Run(tc.name, func(t *testing.T) { + s.Run(tc.name, func() { genStakingJson := s.encCfg.Codec.MustMarshalJSON(tc.gen) err := app.StakingModule{}.ValidateGenesis( s.encCfg.Codec, diff --git a/app/modules_test.go b/app/modules_test.go index 5abd63267..44e59599f 100644 --- a/app/modules_test.go +++ b/app/modules_test.go @@ -60,7 +60,7 @@ func (s *TestSuite) TestGenesis() { wantErr: "min_commission must be positive", }, } { - s.T().Run(tc.name, func(t *testing.T) { + s.Run(tc.name, func() { genStakingJson := s.encCfg.Codec.MustMarshalJSON(tc.gen) err := app.StakingModule{}.ValidateGenesis( s.encCfg.Codec, diff --git a/eth/rpc/block_test.go b/eth/rpc/block_test.go index fde99b14c..1fd502272 100644 --- a/eth/rpc/block_test.go +++ b/eth/rpc/block_test.go @@ -7,6 +7,7 @@ import ( grpctypes "github.com/cosmos/cosmos-sdk/types/grpc" "github.com/ethereum/go-ethereum/common" + "github.com/rs/zerolog/log" "github.com/stretchr/testify/suite" "google.golang.org/grpc/metadata" ) @@ -129,7 +130,7 @@ func (s *BlockSuite) TestUnmarshalBlockNumberOrHash() { } for _, tc := range testCases { - fmt.Printf("Case %s", tc.msg) + log.Info().Msgf("Case %s", tc.msg) // reset input bnh = new(BlockNumberOrHash) err := bnh.UnmarshalJSON(tc.input) diff --git a/go.mod b/go.mod index 4c44032c7..a081ef206 100644 --- a/go.mod +++ b/go.mod @@ -66,6 +66,7 @@ require ( golang.org/x/exp v0.0.0-20231006140011-7918f672742d golang.org/x/net v0.23.0 golang.org/x/text v0.14.0 + github.com/rs/zerolog v1.32.0 ) require ( @@ -197,7 +198,6 @@ require ( github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect github.com/rjeczalik/notify v0.9.1 // indirect github.com/rogpeppe/go-internal v1.11.0 // indirect - github.com/rs/zerolog v1.32.0 // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect github.com/sasha-s/go-deadlock v0.3.1 // indirect diff --git a/x/common/paginate_test.go b/x/common/paginate_test.go index 4aa410041..f3a3744be 100644 --- a/x/common/paginate_test.go +++ b/x/common/paginate_test.go @@ -74,7 +74,7 @@ func (s *paginateSuite) TestParsePagination() { wantOffset: 99, }, } { - s.T().Run(tc.name, func(t *testing.T) { + s.Run(tc.name, func() { gotPageReq, gotPage, gotErr := common.ParsePagination(tc.pageReq) s.EqualValues(tc.wantPage, gotPage) diff --git a/x/common/testutil/testnetwork/tx_test.go b/x/common/testutil/testnetwork/tx_test.go index 94a9db6bd..7912b8cea 100644 --- a/x/common/testutil/testnetwork/tx_test.go +++ b/x/common/testutil/testnetwork/tx_test.go @@ -1,8 +1,6 @@ package testnetwork_test import ( - "testing" - sdk "github.com/cosmos/cosmos-sdk/types" "cosmossdk.io/math" @@ -37,7 +35,7 @@ func (s *TestSuite) TestExecTx() { s.NoError(err) s.EqualValues(0, txResp.Code) - s.T().Run("test tx option changes", func(t *testing.T) { + s.Run("test tx option changes", func() { defaultOpts := testnetwork.DEFAULT_TX_OPTIONS opts := testnetwork.WithTxOptions(testnetwork.TxOptionChanges{ BroadcastMode: &defaultOpts.BroadcastMode, @@ -52,7 +50,7 @@ func (s *TestSuite) TestExecTx() { s.EqualValues(0, txResp.Code) }) - s.T().Run("fail when validators are missing", func(t *testing.T) { + s.Run("fail when validators are missing", func() { networkNoVals := new(testnetwork.Network) *networkNoVals = *s.network networkNoVals.Validators = []*testnetwork.Validator{} diff --git a/x/devgas/v1/ante/ante_test.go b/x/devgas/v1/ante/ante_test.go index 8e9a6f5c8..75a974e04 100644 --- a/x/devgas/v1/ante/ante_test.go +++ b/x/devgas/v1/ante/ante_test.go @@ -236,7 +236,8 @@ func (suite *AnteTestSuite) TestDevGasPayout() { } for _, tc := range testCases { - suite.T().Run(tc.name, func(t *testing.T) { + suite.Run(tc.name, func() { + t := suite.T() bapp, ctx := tc.setup() ctx = ctx.WithChainID("mock-chain-id") anteDecorator := devgasante.NewDevGasPayoutDecorator( diff --git a/x/devgas/v1/genesis_test.go b/x/devgas/v1/genesis_test.go index 332f10f2b..7c450f608 100644 --- a/x/devgas/v1/genesis_test.go +++ b/x/devgas/v1/genesis_test.go @@ -109,7 +109,7 @@ func (s *GenesisTestSuite) TestGenesis() { } for _, tc := range testCases { - s.T().Run(fmt.Sprintf("Case %s", tc.name), func(t *testing.T) { + s.Run(fmt.Sprintf("Case %s", tc.name), func() { s.SetupTest() // reset if tc.expPanic { diff --git a/x/devgas/v1/types/msg_test.go b/x/devgas/v1/types/msg_test.go index fb41cba0f..38ef569cf 100644 --- a/x/devgas/v1/types/msg_test.go +++ b/x/devgas/v1/types/msg_test.go @@ -276,7 +276,7 @@ func (s *MsgsTestSuite) TestQuery_ValidateBasic() { }, }, } { - s.T().Run(tc.name, func(t *testing.T) { + s.Run(tc.name, func() { tc.test() }) } diff --git a/x/evm/keeper/bank_extension.go b/x/evm/keeper/bank_extension.go index cb94ccc59..7bbbefb5a 100644 --- a/x/evm/keeper/bank_extension.go +++ b/x/evm/keeper/bank_extension.go @@ -1,7 +1,7 @@ package keeper import ( - storetypes "github.com/cosmos/cosmos-sdk/store/types" + store "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" auth "github.com/cosmos/cosmos-sdk/x/auth/types" bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" @@ -34,15 +34,19 @@ func (bk NibiruBankKeeper) MintCoins( moduleName string, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.MintCoins(ctx, moduleName, coins); err != nil { - return err - } - if findEtherBalanceChangeFromCoins(coins) { - moduleBech32Addr := auth.NewModuleAddress(moduleName) - bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) - } - return nil + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + // Use the embedded function from [bankkeeper.Keeper] + return bk.BaseKeeper.MintCoins(ctx, moduleName, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + moduleBech32Addr := auth.NewModuleAddress(moduleName) + bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) + } + }, + ) } func (bk NibiruBankKeeper) BurnCoins( @@ -50,14 +54,65 @@ func (bk NibiruBankKeeper) BurnCoins( moduleName string, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.BurnCoins(ctx, moduleName, coins); err != nil { + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + // Use the embedded function from [bankkeeper.Keeper] + return bk.BaseKeeper.BurnCoins(ctx, moduleName, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + moduleBech32Addr := auth.NewModuleAddress(moduleName) + bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) + } + }, + ) +} + +// Each Send* operation on the [NibiruBankKeeper] can be described as having a +// base operation (BaseOp) where the [bankkeeper.BaseKeeper] executes some +// business logic and an operation that occurs afterward (AfterOp), where we +// post-process and provide automatic alignment with the EVM [statedb.StateDB]. +// +// Each "AfterOp" tends to consume a negligible amount of gas (<2000 gas), while +// a each "BaseOp" is around 27000 for a single coin transfer. +// +// Although each "AfterOp" consumes a negligible amount of gas, that +// amount of gas consumed is nonzero and depends on whether the current bank +// transaction message occurs within an Ethereum tx or not. +// +// Consistent gas consumption independent of status of the EVM StateDB is brought +// about in [ForceGasInvariant] by consuming only the gas used for the BaseOp. +// This makes sure that post-processing for the EVM [statedb.StateDB] will not +// result in nondeterminism. +func (bk NibiruBankKeeper) ForceGasInvariant( + ctx sdk.Context, + BaseOp func(ctx sdk.Context) error, + AfterOp func(ctx sdk.Context), +) error { + gasMeterBefore := ctx.GasMeter() + gasConsumedBefore := gasMeterBefore.GasConsumed() + // Start baseGasConsumed at 0 in case we panic before BaseOp completes and + // baseGasConsumed gets a value assignment + baseOpGasConsumed := uint64(0) + defer func() { + gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") + gasMeterBefore.ConsumeGas(gasConsumedBefore+baseOpGasConsumed, "NibiruBankKeeper invariant") + }() + // Note that because the ctx gas meter uses private variables to track gas, + // we have to branch off with a new gas meter instance to avoid mutating the + // "true" gas meter (called GasMeterBefore here). + ctx = ctx. + WithGasMeter(sdk.NewGasMeter(gasMeterBefore.Limit())). + WithKVGasConfig(store.GasConfig{}) + + err := BaseOp(ctx) + baseOpGasConsumed = ctx.GasMeter().GasConsumed() + if err != nil { return err } - if findEtherBalanceChangeFromCoins(coins) { - moduleBech32Addr := auth.NewModuleAddress(moduleName) - bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) - } + + AfterOp(ctx) return nil } @@ -67,15 +122,18 @@ func (bk NibiruBankKeeper) SendCoins( toAddr sdk.AccAddress, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.SendCoins(ctx, fromAddr, toAddr, coins); err != nil { - return err - } - if findEtherBalanceChangeFromCoins(coins) { - bk.SyncStateDBWithAccount(ctx, fromAddr) - bk.SyncStateDBWithAccount(ctx, toAddr) - } - return nil + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + return bk.BaseKeeper.SendCoins(ctx, fromAddr, toAddr, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + bk.SyncStateDBWithAccount(ctx, fromAddr) + bk.SyncStateDBWithAccount(ctx, toAddr) + } + }, + ) } func (bk *NibiruBankKeeper) SyncStateDBWithAccount( @@ -86,13 +144,6 @@ func (bk *NibiruBankKeeper) SyncStateDBWithAccount( return } - cachedGasConfig := ctx.KVGasConfig() - defer func() { - ctx = ctx.WithKVGasConfig(cachedGasConfig) - }() - - // set gas cost to zero for this conditional operation - ctx = ctx.WithKVGasConfig(storetypes.GasConfig{}) balanceWei := evm.NativeToWei( bk.GetBalance(ctx, acc, evm.EVMBankDenom).Amount.BigInt(), ) @@ -114,16 +165,20 @@ func (bk NibiruBankKeeper) SendCoinsFromAccountToModule( recipientModule string, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, recipientModule, coins); err != nil { - return err - } - if findEtherBalanceChangeFromCoins(coins) { - bk.SyncStateDBWithAccount(ctx, senderAddr) - moduleBech32Addr := auth.NewModuleAddress(recipientModule) - bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) - } - return nil + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + // Use the embedded function from [bankkeeper.Keeper] + return bk.BaseKeeper.SendCoinsFromAccountToModule(ctx, senderAddr, recipientModule, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + bk.SyncStateDBWithAccount(ctx, senderAddr) + moduleBech32Addr := auth.NewModuleAddress(recipientModule) + bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) + } + }, + ) } func (bk NibiruBankKeeper) SendCoinsFromModuleToAccount( @@ -132,16 +187,20 @@ func (bk NibiruBankKeeper) SendCoinsFromModuleToAccount( recipientAddr sdk.AccAddress, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.SendCoinsFromModuleToAccount(ctx, senderModule, recipientAddr, coins); err != nil { - return err - } - if findEtherBalanceChangeFromCoins(coins) { - moduleBech32Addr := auth.NewModuleAddress(senderModule) - bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) - bk.SyncStateDBWithAccount(ctx, recipientAddr) - } - return nil + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + // Use the embedded function from [bankkeeper.Keeper] + return bk.BaseKeeper.SendCoinsFromModuleToAccount(ctx, senderModule, recipientAddr, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + moduleBech32Addr := auth.NewModuleAddress(senderModule) + bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) + bk.SyncStateDBWithAccount(ctx, recipientAddr) + } + }, + ) } func (bk NibiruBankKeeper) SendCoinsFromModuleToModule( @@ -150,15 +209,19 @@ func (bk NibiruBankKeeper) SendCoinsFromModuleToModule( recipientModule string, coins sdk.Coins, ) error { - // Use the embedded function from [bankkeeper.Keeper] - if err := bk.BaseKeeper.SendCoinsFromModuleToModule(ctx, senderModule, recipientModule, coins); err != nil { - return err - } - if findEtherBalanceChangeFromCoins(coins) { - senderBech32Addr := auth.NewModuleAddress(senderModule) - recipientBech32Addr := auth.NewModuleAddress(recipientModule) - bk.SyncStateDBWithAccount(ctx, senderBech32Addr) - bk.SyncStateDBWithAccount(ctx, recipientBech32Addr) - } - return nil + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + // Use the embedded function from [bankkeeper.Keeper] + return bk.BaseKeeper.SendCoinsFromModuleToModule(ctx, senderModule, recipientModule, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + senderBech32Addr := auth.NewModuleAddress(senderModule) + recipientBech32Addr := auth.NewModuleAddress(recipientModule) + bk.SyncStateDBWithAccount(ctx, senderBech32Addr) + bk.SyncStateDBWithAccount(ctx, recipientBech32Addr) + } + }, + ) } diff --git a/x/evm/keeper/bank_extension_test.go b/x/evm/keeper/bank_extension_test.go new file mode 100644 index 000000000..64ae4dea9 --- /dev/null +++ b/x/evm/keeper/bank_extension_test.go @@ -0,0 +1,303 @@ +package keeper_test + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + staking "github.com/cosmos/cosmos-sdk/x/staking/types" + + "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" + "github.com/NibiruChain/nibiru/v2/x/evm" + "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" +) + +// TestGasConsumedInvariantSend: The "NibiruBankKeeper" is defined such that +// send operations are meant to have consistent gas consumption regardless of the +// whether the active "StateDB" instance is defined or undefined (nil). This is +// important to prevent consensus failures resulting from nodes reaching an +// inconsistent state after processing the same state transitions and gettng +// conflicting gas results. +func (s *Suite) TestGasConsumedInvariantSend() { + to := evmtest.NewEthPrivAcc() // arbitrary constant + + testCases := []struct { + GasConsumedInvariantScenario + name string + }{ + { + name: "StateDB nil", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: evm.EVMBankDenom, + NilStateDB: true, + }, + }, + + { + name: "StateDB defined", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: evm.EVMBankDenom, + NilStateDB: false, + }, + }, + + { + name: "StateDB nil, uniba", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: "uniba", + NilStateDB: true, + }, + }, + } + + s.T().Log("Check that gas consumption is equal in all scenarios") + var first uint64 + for idx, tc := range testCases { + s.Run(tc.name, func() { + gasConsumed := tc.GasConsumedInvariantScenario.Run(s, to) + if idx == 0 { + first = gasConsumed + return + } + // Each elem being equal to "first" implies that each elem is equal + s.Equalf( + fmt.Sprintf("%d", first), + fmt.Sprintf("%d", gasConsumed), + "Gas consumed should be equal", + ) + }) + } +} + +type GasConsumedInvariantScenario struct { + BankDenom string + NilStateDB bool +} + +func (scenario GasConsumedInvariantScenario) Run( + s *Suite, + to evmtest.EthPrivKeyAcc, +) (gasConsumed uint64) { + bankDenom, nilStateDB := scenario.BankDenom, scenario.NilStateDB + deps := evmtest.NewTestDeps() + if nilStateDB { + s.Require().Nil(deps.EvmKeeper.Bank.StateDB) + } else { + deps.NewStateDB() + s.NotNil(deps.EvmKeeper.Bank.StateDB) + } + + sendCoins := sdk.NewCoins(sdk.NewInt64Coin(bankDenom, 420)) + s.NoError( + testapp.FundAccount(deps.App.BankKeeper, deps.Ctx, deps.Sender.NibiruAddr, sendCoins), + ) + + gasConsumedBefore := deps.Ctx.GasMeter().GasConsumed() + s.NoError( + deps.App.BankKeeper.SendCoins( + deps.Ctx, deps.Sender.NibiruAddr, to.NibiruAddr, sendCoins, + ), + ) + gasConsumedAfter := deps.Ctx.GasMeter().GasConsumed() + + s.GreaterOrEqualf(gasConsumedAfter, gasConsumedBefore, + "gas meter consumed should not be negative: gas consumed after = %d, gas consumed before = %d ", + gasConsumedAfter, gasConsumedBefore, + ) + + return gasConsumedAfter - gasConsumedBefore +} + +func (s *Suite) TestGasConsumedInvariantSendNotNibi() { + to := evmtest.NewEthPrivAcc() // arbitrary constant + + testCases := []struct { + GasConsumedInvariantScenario + name string + }{ + { + name: "StateDB nil A", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: "dummy_token_A", + NilStateDB: true, + }, + }, + + { + name: "StateDB defined A", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: "dummy_token_A", + NilStateDB: false, + }, + }, + + { + name: "StateDB nil B", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: "dummy_token_B", + NilStateDB: true, + }, + }, + + { + name: "StateDB defined B", + GasConsumedInvariantScenario: GasConsumedInvariantScenario{ + BankDenom: "dummy_token_B", + NilStateDB: false, + }, + }, + } + + s.T().Log("Check that gas consumption is equal in all scenarios") + var first uint64 + for idx, tc := range testCases { + s.Run(tc.name, func() { + gasConsumed := tc.GasConsumedInvariantScenario.Run(s, to) + if idx == 0 { + first = gasConsumed + return + } + // Each elem being equal to "first" implies that each elem is equal + s.Equalf( + fmt.Sprintf("%d", first), + fmt.Sprintf("%d", gasConsumed), + "Gas consumed should be equal", + ) + }) + } +} + +type FunctionalGasConsumedInvariantScenario struct { + Setup func(deps *evmtest.TestDeps) + Measure func(deps *evmtest.TestDeps) +} + +func (f FunctionalGasConsumedInvariantScenario) Run(s *Suite) { + var ( + gasConsumedA uint64 // nil StateDB + gasConsumedB uint64 // not nil StateDB + ) + + { + deps := evmtest.NewTestDeps() + s.Nil(deps.EvmKeeper.Bank.StateDB) + + f.Setup(&deps) + + gasConsumedBefore := deps.Ctx.GasMeter().GasConsumed() + f.Measure(&deps) + gasConsumedAfter := deps.Ctx.GasMeter().GasConsumed() + gasConsumedA = gasConsumedAfter - gasConsumedBefore + } + + { + deps := evmtest.NewTestDeps() + deps.NewStateDB() + s.NotNil(deps.EvmKeeper.Bank.StateDB) + + f.Setup(&deps) + + gasConsumedBefore := deps.Ctx.GasMeter().GasConsumed() + f.Measure(&deps) + gasConsumedAfter := deps.Ctx.GasMeter().GasConsumed() + gasConsumedB = gasConsumedAfter - gasConsumedBefore + } + + s.Equalf( + fmt.Sprintf("%d", gasConsumedA), + fmt.Sprintf("%d", gasConsumedB), + "Gas consumed should be equal", + ) +} + +// See [Suite.TestGasConsumedInvariantSend]. +func (s *Suite) TestGasConsumedInvariantOther() { + to := evmtest.NewEthPrivAcc() // arbitrary constant + bankDenom := evm.EVMBankDenom + coins := sdk.NewCoins(sdk.NewInt64Coin(bankDenom, 420)) // arbitrary constant + // Use this value because the gas token is involved in both the BaseOp and + // AfterOp of the "ForceGasInvariant" function. + s.Run("MintCoins", func() { + FunctionalGasConsumedInvariantScenario{ + Setup: func(deps *evmtest.TestDeps) { + s.NoError( + testapp.FundAccount(deps.App.BankKeeper, deps.Ctx, deps.Sender.NibiruAddr, coins), + ) + }, + Measure: func(deps *evmtest.TestDeps) { + s.NoError( + deps.App.BankKeeper.MintCoins( + deps.Ctx, evm.ModuleName, coins, + ), + ) + }, + }.Run(s) + }) + + s.Run("BurnCoins", func() { + FunctionalGasConsumedInvariantScenario{ + Setup: func(deps *evmtest.TestDeps) { + s.NoError( + testapp.FundModuleAccount(deps.App.BankKeeper, deps.Ctx, evm.ModuleName, coins), + ) + }, + Measure: func(deps *evmtest.TestDeps) { + s.NoError( + deps.App.BankKeeper.BurnCoins( + deps.Ctx, evm.ModuleName, coins, + ), + ) + }, + }.Run(s) + }) + + s.Run("SendCoinsFromAccountToModule", func() { + FunctionalGasConsumedInvariantScenario{ + Setup: func(deps *evmtest.TestDeps) { + s.NoError( + testapp.FundAccount(deps.App.BankKeeper, deps.Ctx, deps.Sender.NibiruAddr, coins), + ) + }, + Measure: func(deps *evmtest.TestDeps) { + s.NoError( + deps.App.BankKeeper.SendCoinsFromAccountToModule( + deps.Ctx, deps.Sender.NibiruAddr, evm.ModuleName, coins, + ), + ) + }, + }.Run(s) + }) + + s.Run("SendCoinsFromModuleToAccount", func() { + FunctionalGasConsumedInvariantScenario{ + Setup: func(deps *evmtest.TestDeps) { + s.NoError( + testapp.FundModuleAccount(deps.App.BankKeeper, deps.Ctx, evm.ModuleName, coins), + ) + }, + Measure: func(deps *evmtest.TestDeps) { + s.NoError( + deps.App.BankKeeper.SendCoinsFromModuleToAccount( + deps.Ctx, evm.ModuleName, to.NibiruAddr, coins, + ), + ) + }, + }.Run(s) + }) + + s.Run("SendCoinsFromModuleToModule", func() { + FunctionalGasConsumedInvariantScenario{ + Setup: func(deps *evmtest.TestDeps) { + s.NoError( + testapp.FundModuleAccount(deps.App.BankKeeper, deps.Ctx, evm.ModuleName, coins), + ) + }, + Measure: func(deps *evmtest.TestDeps) { + s.NoError( + deps.App.BankKeeper.SendCoinsFromModuleToModule( + deps.Ctx, evm.ModuleName, staking.NotBondedPoolName, coins, + ), + ) + }, + }.Run(s) + }) +} diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index b7a3ec9a8..e1d9dccb7 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -255,17 +255,28 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, fullRefundLeftoverGas bool, ) (resp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) { var ( - ret []byte // return bytes from evm execution - vmErr error // vm errors do not effect consensus and are therefore not assigned to err + // return bytes from evm execution + ret []byte + // vm errors do not imply a failed query, thus they don't populate the + // function's "err" value. + vmErr error + ) + + var ( + stateDB *statedb.StateDB + // save a reference to return to the previous stateDB + oldStateDB *statedb.StateDB = k.Bank.StateDB ) - // save a reference to return to the previous stateDB - oldStateDb := k.Bank.StateDB defer func() { - k.Bank.StateDB = oldStateDb + if commit && err == nil && resp != nil && !resp.Failed() { + k.Bank.StateDB = stateDB + } else { + k.Bank.StateDB = oldStateDB + } }() - stateDB := k.NewStateDB(ctx, txConfig) + stateDB = k.NewStateDB(ctx, txConfig) evmObj = k.NewEVM(ctx, msg, evmConfig, tracer, stateDB) leftoverGas := msg.Gas()