Skip to content

Commit

Permalink
wip! TDD test cases (red)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Unique-Divine
Unique-Divine committed Dec 6, 2024
1 parent 46cb6ae commit 5577dce
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 36 deletions.
65 changes: 37 additions & 28 deletions x/evm/keeper/bank_extension.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,43 @@ func (bk NibiruBankKeeper) BurnCoins(
return nil
}

func (bk NibiruBankKeeper) ForceConstantGas(
// 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()
baseGasConsumed := uint64(0)
baseOpGasConsumed := uint64(0)
// Start baseGasConsumed at 0 in case we panic before BaseOp completes and
// baseGasConsumed gets a value assignment
defer func() {
gasMeterBefore.RefundGas(gasMeterBefore.GasConsumed(), "")
gasMeterBefore.ConsumeGas(gasConsumedBefore+baseGasConsumed, "NibiruBankKeeper invariant")
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()))

err := BaseOp(ctx)
baseGasConsumed = ctx.GasMeter().GasConsumed()
baseOpGasConsumed = ctx.GasMeter().GasConsumed()
if err != nil {
return err
}
Expand All @@ -94,29 +114,18 @@ func (bk NibiruBankKeeper) SendCoins(
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
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) SendCoins_Original(
Expand Down
154 changes: 146 additions & 8 deletions x/evm/keeper/bank_extension_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import (
"github.com/NibiruChain/nibiru/v2/x/evm"
"github.com/NibiruChain/nibiru/v2/x/evm/evmtest"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/rs/zerolog/log"
staking "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// TODO: UD-DEBUG:
// non-nil StaetDB bank send and record gas consumed
// nil StaetDB bank send and record gas consumed
// Both should be equal.

// TestGasConsumedInvariant: The "NibiruBankKeeper" is defined such that
// 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) TestGasConsumedInvariant() {
func (s *Suite) TestGasConsumedInvariantSend() {
to := evmtest.NewEthPrivAcc() // arbitrary constant

testCases := []struct {
Expand Down Expand Up @@ -93,7 +93,8 @@ func (scenario GasConsumedInvariantScenario) Run(
if nilStateDB {
s.Require().Nil(deps.EvmKeeper.Bank.StateDB)
} else {
s.populateStateDB(&deps)
deps.NewStateDB()
s.NotNil(deps.EvmKeeper.Bank.StateDB)
}

sendCoins := sdk.NewCoins(sdk.NewInt64Coin(bankDenom, 420))
Expand All @@ -108,9 +109,6 @@ func (scenario GasConsumedInvariantScenario) Run(
),
)
gasConsumedAfter := deps.Ctx.GasMeter().GasConsumed()
log.Debug().Msgf("gasConsumedBefore: %d", gasConsumedBefore)
log.Debug().Msgf("gasConsumedAfter: %d", gasConsumedAfter)
log.Debug().Msgf("nilStateDB: %v", nilStateDB)

s.GreaterOrEqualf(gasConsumedAfter, gasConsumedBefore,
"gas meter consumed should not be negative: gas consumed after = %d, gas consumed before = %d ",
Expand All @@ -120,7 +118,7 @@ func (scenario GasConsumedInvariantScenario) Run(
return gasConsumedAfter - gasConsumedBefore
}

func (s *Suite) TestGasConsumedInvariantNotNibi() {
func (s *Suite) TestGasConsumedInvariantSendNotNibi() {
to := evmtest.NewEthPrivAcc() // arbitrary constant

testCases := []struct {
Expand Down Expand Up @@ -177,5 +175,145 @@ func (s *Suite) TestGasConsumedInvariantNotNibi() {
)
})
}
}

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",
)
}

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)

})

}

0 comments on commit 5577dce

Please sign in to comment.