From f874263ad0d43930f1c5c6661b7e3b673dc74848 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Fri, 6 Dec 2024 17:01:09 +0530 Subject: [PATCH] wip! TDD test cases (green) --- CHANGELOG.md | 6 +- x/evm/keeper/bank_extension.go | 152 +++++++++++++--------------- x/evm/keeper/bank_extension_test.go | 18 +--- 3 files changed, 77 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da4a32363..001109469 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] +- [#2xxx](https://github.com/NibiruChain/nibiru/pull/2xxx) - 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/x/evm/keeper/bank_extension.go b/x/evm/keeper/bank_extension.go index 4c66ee26e..e4afbba09 100644 --- a/x/evm/keeper/bank_extension.go +++ b/x/evm/keeper/bank_extension.go @@ -1,12 +1,9 @@ package keeper import ( - // storetypes "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" - // "github.com/rs/zerolog/log" "github.com/NibiruChain/nibiru/v2/eth" "github.com/NibiruChain/nibiru/v2/x/evm" @@ -36,15 +33,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( @@ -52,15 +53,19 @@ 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 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.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 @@ -128,37 +133,6 @@ func (bk NibiruBankKeeper) SendCoins( ) } -func (bk NibiruBankKeeper) SendCoins_Original( - ctx sdk.Context, - fromAddr sdk.AccAddress, - toAddr sdk.AccAddress, - coins sdk.Coins, -) error { - // Force constant gas regardless of the whether the StateDB is defined or - // whether the "findEtherBalance*" block would consume gas. - gasMeterBefore := ctx.GasMeter() - gasConsumedBefore := gasMeterBefore.GasConsumed() - baseGasConsumed := uint64(0) - defer func() { - gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "") - gasMeterBefore.ConsumeGas(gasConsumedBefore+baseGasConsumed, "NibiruBankKeeper invariant") - }() - ctx = ctx.WithGasMeter(sdk.NewGasMeter(gasMeterBefore.Limit())) - - // Use the embedded function from [bankkeeper.Keeper] - err := bk.BaseKeeper.SendCoins(ctx, fromAddr, toAddr, coins) - baseGasConsumed = ctx.GasMeter().GasConsumed() - if err != nil { - return err - } - - if findEtherBalanceChangeFromCoins(coins) { - bk.SyncStateDBWithAccount(ctx, fromAddr) - bk.SyncStateDBWithAccount(ctx, toAddr) - } - return nil -} - func (bk *NibiruBankKeeper) SyncStateDBWithAccount( ctx sdk.Context, acc sdk.AccAddress, ) { @@ -188,16 +162,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( @@ -206,16 +184,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( @@ -224,15 +206,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 index 271aba7ea..dd15941de 100644 --- a/x/evm/keeper/bank_extension_test.go +++ b/x/evm/keeper/bank_extension_test.go @@ -3,11 +3,12 @@ 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" - sdk "github.com/cosmos/cosmos-sdk/types" - staking "github.com/cosmos/cosmos-sdk/x/staking/types" ) // TODO: UD-DEBUG: @@ -70,13 +71,6 @@ func (s *Suite) TestGasConsumedInvariantSend() { ) }) } - -} - -func (s *Suite) populateStateDB(deps *evmtest.TestDeps) { - // evmtest.DeployAndExecuteERC20Transfer(deps, s.T()) - deps.NewStateDB() - s.NotNil(deps.EvmKeeper.Bank.StateDB) } type GasConsumedInvariantScenario struct { @@ -241,7 +235,6 @@ func (s *Suite) TestGasConsumedInvariantOther() { ) }, }.Run(s) - }) s.Run("BurnCoins", func() { @@ -259,7 +252,6 @@ func (s *Suite) TestGasConsumedInvariantOther() { ) }, }.Run(s) - }) s.Run("SendCoinsFromAccountToModule", func() { @@ -277,7 +269,6 @@ func (s *Suite) TestGasConsumedInvariantOther() { ) }, }.Run(s) - }) s.Run("SendCoinsFromModuleToAccount", func() { @@ -295,7 +286,6 @@ func (s *Suite) TestGasConsumedInvariantOther() { ) }, }.Run(s) - }) s.Run("SendCoinsFromModuleToModule", func() { @@ -313,7 +303,5 @@ func (s *Suite) TestGasConsumedInvariantOther() { ) }, }.Run(s) - }) - }