-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(evm): StateDB multistore cache for precompile reversion and a safer Nibiru bank keeper that respects the EVM #2094
Changes from 4 commits
295a2d9
80dd2d7
c624912
7f904a0
08a73ee
717ce1c
04a6897
7c34423
4e3bf3c
576cf6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Direct Found inconsistent StateDB creation patterns that should be standardized:
The only legitimate direct usage is in 🔗 Analysis chainLGTM! Improved StateDB creation pattern. This change properly delegates StateDB creation to EvmKeeper, ensuring test scenarios use the same state management path as production code. This alignment strengthens test reliability and supports the PR's goal of safer state handling. Let's verify consistent StateDB creation patterns across the codebase: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any direct statedb.New calls that should use EvmKeeper.NewStateDB
# Expected: Only EvmKeeper should directly call statedb.New
# Search for direct statedb.New usage
echo "Checking for direct statedb.New usage..."
rg "statedb\.New\(" -A 3
# Search for EvmKeeper.NewStateDB usage
echo "Checking for EvmKeeper.NewStateDB usage..."
rg "\.NewStateDB\(" -A 3
Length of output: 1451 |
||
statedb.NewEmptyTxConfig( | ||
gethcommon.BytesToHash(deps.Ctx.HeaderHash().Bytes()), | ||
), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
package keeper | ||
|
||
import ( | ||
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/NibiruChain/nibiru/v2/eth" | ||
"github.com/NibiruChain/nibiru/v2/x/evm" | ||
"github.com/NibiruChain/nibiru/v2/x/evm/statedb" | ||
) | ||
|
||
var ( | ||
_ bankkeeper.Keeper = &NibiruBankKeeper{} | ||
_ bankkeeper.SendKeeper = &NibiruBankKeeper{} | ||
) | ||
|
||
type NibiruBankKeeper struct { | ||
bankkeeper.BaseKeeper | ||
StateDB *statedb.StateDB | ||
balanceChangesForStateDB uint64 | ||
} | ||
|
||
func (evmKeeper *Keeper) NewStateDB( | ||
ctx sdk.Context, txConfig statedb.TxConfig, | ||
) *statedb.StateDB { | ||
stateDB := statedb.New(ctx, evmKeeper, txConfig) | ||
bk := evmKeeper.bankKeeper | ||
bk.StateDB = stateDB | ||
bk.balanceChangesForStateDB = 0 | ||
return stateDB | ||
} | ||
|
||
// BalanceChangesForStateDB returns the count of [statedb.JournalChange] entries | ||
// that were added to the current [statedb.StateDB] | ||
func (bk *NibiruBankKeeper) BalanceChangesForStateDB() uint64 { return bk.balanceChangesForStateDB } | ||
|
||
func (bk NibiruBankKeeper) MintCoins( | ||
ctx sdk.Context, | ||
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(evm.ModuleName) | ||
bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) | ||
} | ||
return nil | ||
} | ||
|
||
func (bk NibiruBankKeeper) BurnCoins( | ||
ctx sdk.Context, | ||
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(evm.ModuleName) | ||
bk.SyncStateDBWithAccount(ctx, moduleBech32Addr) | ||
} | ||
return nil | ||
} | ||
|
||
func (bk NibiruBankKeeper) SendCoins( | ||
ctx sdk.Context, | ||
fromAddr sdk.AccAddress, | ||
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 | ||
} | ||
|
||
func (bk *NibiruBankKeeper) SyncStateDBWithAccount( | ||
ctx sdk.Context, acc sdk.AccAddress, | ||
) { | ||
// If there's no StateDB set, it means we're not in an EthereumTx. | ||
if bk.StateDB == nil { | ||
return | ||
} | ||
balanceWei := evm.NativeToWei( | ||
bk.GetBalance(ctx, acc, evm.EVMBankDenom).Amount.BigInt(), | ||
) | ||
bk.StateDB.SetBalanceWei(eth.NibiruAddrToEthAddr(acc), balanceWei) | ||
bk.balanceChangesForStateDB += 1 | ||
} | ||
|
||
func findEtherBalanceChangeFromCoins(coins sdk.Coins) (found bool) { | ||
for _, c := range coins { | ||
if c.Denom == evm.EVMBankDenom { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func (bk NibiruBankKeeper) SendCoinsFromAccountToModule( | ||
ctx sdk.Context, | ||
senderAddr sdk.AccAddress, | ||
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 | ||
} | ||
|
||
func (bk NibiruBankKeeper) SendCoinsFromModuleToAccount( | ||
ctx sdk.Context, | ||
senderModule string, | ||
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 | ||
} | ||
|
||
func (bk NibiruBankKeeper) SendCoinsFromModuleToModule( | ||
ctx sdk.Context, | ||
senderModule string, | ||
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 | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was here because of the repeated calls during gas estimation, not for EVM reversions |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,7 @@ | |
} | ||
} | ||
} | ||
|
||
func (k *Keeper) IsPrecompile(addr gethcommon.Address) bool { | ||
return k.precompiles.Has(addr) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line's required to appease the module manager when it does a type assertion on an object of type
bankkeeper.Keeper
(an interface), asserting that it's actually abankkeeper.BaseKeeper
struct. I didn't see a simple way to force x/bank to use theNibiruBankKeeper
struct without editing the Bank module directly, so I opted for what's written here, which is quite simple.Edit: Moved the
NibiruBankKeeper
to #2095. The rationale behind these lines are in that PR