From 86d6b3ff64bdf7efa77a6ff2c5247aa0dc863a33 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 6 Jan 2025 01:43:01 -0500 Subject: [PATCH 1/2] fix(bank): add additional missing bank keeper method overrides to sync with StateDB (#2142) * fix(bank): add additional missing bank keeper method overrides to sync with stateDB * chore: update changelog --- CHANGELOG.md | 5 ++- x/evm/keeper/bank_extension.go | 69 ++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cb880339..068b3d0be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,15 +45,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#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. -- [#2120](https://github.com/NibiruChain/nibiru/pull/2120) - fix: Use canonical hexadecimal strings for Eip155 address encoding +- [#2120](https://github.com/NibiruChain/nibiru/pull/2120) - fix: Use canonical hexadecimal strings for Eip155 address encoding - [#2122](https://github.com/NibiruChain/nibiru/pull/2122) - test(evm): more bank extension tests and EVM ABCI integration tests to prevent regressions - [#2124](https://github.com/NibiruChain/nibiru/pull/2124) - refactor(evm): Remove unnecessary argument in the `VerifyFee` function, which returns the token payment required based on the effective fee from the tx data. Improve documentation. -- [#2125](https://github.com/NibiruChain/nibiru/pull/2125) - feat(evm-precompile):Emit EVM events created to reflect the ABCI events that occur outside the EVM to make sure that block explorers and indexers can find indexed ABCI event information. +- [#2125](https://github.com/NibiruChain/nibiru/pull/2125) - feat(evm-precompile):Emit EVM events created to reflect the ABCI events that occur outside the EVM to make sure that block explorers and indexers can find indexed ABCI event information. - [#2129](https://github.com/NibiruChain/nibiru/pull/2129) - fix(evm): issue with infinite recursion in erc20 funtoken contracts - [#2134](https://github.com/NibiruChain/nibiru/pull/2134) - fix(evm): query of NIBI should use bank state, not the StateDB +- [#2142](https://github.com/NibiruChain/nibiru/pull/2142) - fix(bank): add additional missing methods to the NibiruBankKeeper #### Nibiru EVM | Before Audit 2 - 2024-12-06 diff --git a/x/evm/keeper/bank_extension.go b/x/evm/keeper/bank_extension.go index 7a14b1db2..34cf1fbd6 100644 --- a/x/evm/keeper/bank_extension.go +++ b/x/evm/keeper/bank_extension.go @@ -5,16 +5,14 @@ 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" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "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{} -) +var _ bankkeeper.Keeper = &NibiruBankKeeper{} type NibiruBankKeeper struct { bankkeeper.BaseKeeper @@ -29,6 +27,69 @@ func (evmKeeper *Keeper) NewStateDB( return stateDB } +func (bk NibiruBankKeeper) InputOutputCoins( + ctx sdk.Context, + input []banktypes.Input, + output []banktypes.Output, +) error { + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + return bk.BaseKeeper.InputOutputCoins(ctx, input, output) + }, + func(ctx sdk.Context) { + for _, input := range input { + if findEtherBalanceChangeFromCoins(input.Coins) { + bk.SyncStateDBWithAccount(ctx, sdk.MustAccAddressFromBech32(input.Address)) + } + } + for _, output := range output { + if findEtherBalanceChangeFromCoins(output.Coins) { + bk.SyncStateDBWithAccount(ctx, sdk.MustAccAddressFromBech32(output.Address)) + } + } + }, + ) +} + +func (bk NibiruBankKeeper) DelegateCoins( + ctx sdk.Context, + delegatorAddr sdk.AccAddress, + moduleBech32Addr sdk.AccAddress, + coins sdk.Coins, +) error { + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + return bk.BaseKeeper.DelegateCoins(ctx, delegatorAddr, moduleBech32Addr, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + bk.SyncStateDBWithAccount(ctx, delegatorAddr) + } + }, + ) +} + +func (bk NibiruBankKeeper) UndelegateCoins( + ctx sdk.Context, + delegatorAddr sdk.AccAddress, + moduleBech32Addr sdk.AccAddress, + coins sdk.Coins, +) error { + return bk.ForceGasInvariant( + ctx, + func(ctx sdk.Context) error { + return bk.BaseKeeper.UndelegateCoins(ctx, delegatorAddr, moduleBech32Addr, coins) + }, + func(ctx sdk.Context) { + if findEtherBalanceChangeFromCoins(coins) { + bk.SyncStateDBWithAccount(ctx, delegatorAddr) + } + }, + ) +} + func (bk NibiruBankKeeper) MintCoins( ctx sdk.Context, moduleName string, From 8cd4ceb0342411450607d67022b26a89f4769f18 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 6 Jan 2025 02:03:49 -0500 Subject: [PATCH 2/2] refactor: simplify account retrieval (#2141) * refactor: simplify account retrieval * Update CHANGELOG.md * test: fix tests --------- Co-authored-by: Oleg Nikonychev --- CHANGELOG.md | 1 + x/evm/keeper/grpc_query.go | 7 +++++-- x/evm/keeper/grpc_query_test.go | 22 ++++------------------ x/evm/keeper/statedb.go | 25 ++++--------------------- 4 files changed, 14 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 068b3d0be..d268fd875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ documentation. - [#2125](https://github.com/NibiruChain/nibiru/pull/2125) - feat(evm-precompile):Emit EVM events created to reflect the ABCI events that occur outside the EVM to make sure that block explorers and indexers can find indexed ABCI event information. - [#2129](https://github.com/NibiruChain/nibiru/pull/2129) - fix(evm): issue with infinite recursion in erc20 funtoken contracts - [#2134](https://github.com/NibiruChain/nibiru/pull/2134) - fix(evm): query of NIBI should use bank state, not the StateDB +- [#2141](https://github.com/NibiruChain/nibiru/pull/2141) - refactor: simplify account retrieval operation in `nibid q evm account`. - [#2142](https://github.com/NibiruChain/nibiru/pull/2142) - fix(bank): add additional missing methods to the NibiruBankKeeper #### Nibiru EVM | Before Audit 2 - 2024-12-06 diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index fd71412a1..b96cd5ddb 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -64,7 +64,10 @@ func (k Keeper) EthAccount( } ctx := sdk.UnwrapSDKContext(goCtx) - acct := k.GetAccountOrEmpty(ctx, addrEth) + acct := k.getAccountWithoutBalance(ctx, addrEth) + if acct == nil { + return nil, fmt.Errorf("account not found for %s", addrEth.Hex()) + } balNative := k.Bank.GetBalance(ctx, addrBech32, evm.EVMBankDenom).Amount.BigInt() return &evm.QueryEthAccountResponse{ @@ -205,7 +208,7 @@ func (k Keeper) Code( ctx := sdk.UnwrapSDKContext(goCtx) address := gethcommon.HexToAddress(req.Address) - acct := k.GetAccountWithoutBalance(ctx, address) + acct := k.getAccountWithoutBalance(ctx, address) var code []byte if acct != nil && acct.IsContract() { diff --git a/x/evm/keeper/grpc_query_test.go b/x/evm/keeper/grpc_query_test.go index c0a807fb7..28da63173 100644 --- a/x/evm/keeper/grpc_query_test.go +++ b/x/evm/keeper/grpc_query_test.go @@ -144,17 +144,10 @@ func (s *Suite) TestQueryEvmAccount() { req = &evm.QueryEthAccountRequest{ Address: ethAcc.EthAddr.String(), } - wantResp = &evm.QueryEthAccountResponse{ - Balance: "0", - BalanceWei: "0", - CodeHash: gethcommon.BytesToHash(evm.EmptyCodeHash).Hex(), - Nonce: 0, - EthAddress: ethAcc.EthAddr.String(), - Bech32Address: ethAcc.NibiruAddr.String(), - } + wantResp = nil return req, wantResp }, - wantErr: "", + wantErr: "account not found for", }, { name: "happy: nonexistent account (bech32 input)", @@ -163,17 +156,10 @@ func (s *Suite) TestQueryEvmAccount() { req = &evm.QueryEthAccountRequest{ Address: ethAcc.NibiruAddr.String(), } - wantResp = &evm.QueryEthAccountResponse{ - Balance: "0", - BalanceWei: "0", - CodeHash: gethcommon.BytesToHash(evm.EmptyCodeHash).Hex(), - Nonce: 0, - EthAddress: ethAcc.EthAddr.String(), - Bech32Address: ethAcc.NibiruAddr.String(), - } + wantResp = nil return req, wantResp }, - wantErr: "", + wantErr: "account not found for", }, } diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index b85a595a5..f46c491e6 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -25,7 +25,7 @@ var _ statedb.Keeper = &Keeper{} // Implements the `statedb.Keeper` interface. // Returns nil if the account does not exist or has the wrong type. func (k *Keeper) GetAccount(ctx sdk.Context, addr gethcommon.Address) *statedb.Account { - acct := k.GetAccountWithoutBalance(ctx, addr) + acct := k.getAccountWithoutBalance(ctx, addr) if acct == nil { return nil } @@ -184,11 +184,10 @@ func (k *Keeper) DeleteAccount(ctx sdk.Context, addr gethcommon.Address) error { return nil } -// GetAccountWithoutBalance load nonce and codehash without balance, +// getAccountWithoutBalance load nonce and codehash without balance, // more efficient in cases where balance is not needed. -func (k *Keeper) GetAccountWithoutBalance(ctx sdk.Context, addr gethcommon.Address) *statedb.Account { - nibiruAddr := sdk.AccAddress(addr.Bytes()) - acct := k.accountKeeper.GetAccount(ctx, nibiruAddr) +func (k *Keeper) getAccountWithoutBalance(ctx sdk.Context, addr gethcommon.Address) *statedb.Account { + acct := k.accountKeeper.GetAccount(ctx, eth.EthAddrToNibiruAddr(addr)) if acct == nil { return nil } @@ -204,19 +203,3 @@ func (k *Keeper) GetAccountWithoutBalance(ctx sdk.Context, addr gethcommon.Addre CodeHash: codeHash, } } - -// GetAccountOrEmpty returns empty account if not exist, returns error if it's not `EthAccount` -func (k *Keeper) GetAccountOrEmpty( - ctx sdk.Context, addr gethcommon.Address, -) statedb.Account { - acct := k.GetAccount(ctx, addr) - if acct != nil { - return *acct - } - - // empty account - return statedb.Account{ - BalanceNative: new(big.Int), - CodeHash: evm.EmptyCodeHash, - } -}