Skip to content

Commit

Permalink
fix(evm): Guarantee that gas consumed during any send operation of th…
Browse files Browse the repository at this point in the history
…e "NibiruBankKeeper" depends only on the "bankkeeper.BaseKeeper"'s gas consumption. (#2119)

* fix(evm): messy working version with consistent gas behavior not dependent on StateDB existence wip!

* wip! TDD test cases (red)

* wip! TDD test cases (green)

* clean up PR

* fix tests + correct use of suite.Run
  • Loading branch information
Unique-Divine authored Dec 6, 2024
1 parent 39ebc44 commit 5a7f90d
Show file tree
Hide file tree
Showing 15 changed files with 474 additions and 95 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions app/ante/commission_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)),
Expand Down
20 changes: 10 additions & 10 deletions app/ante/fixed_gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand All @@ -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)),
Expand Down Expand Up @@ -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)),
Expand All @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion app/evmante/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion app/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion eth/rpc/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x/common/paginate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions x/common/testutil/testnetwork/tx_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package testnetwork_test

import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"

"cosmossdk.io/math"
Expand Down Expand Up @@ -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,
Expand All @@ -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{}
Expand Down
3 changes: 2 additions & 1 deletion x/devgas/v1/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion x/devgas/v1/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion x/devgas/v1/types/msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
}
Expand Down
Loading

0 comments on commit 5a7f90d

Please sign in to comment.