Skip to content
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.

Sign up for GitHub

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): Use a NibiruBankKeeper to automatically record ether balance changes on the StateDB journal #2095

Merged
merged 24 commits into from
Oct 31, 2024
Merged

fix(evm): Use a NibiruBankKeeper to automatically record ether balance changes on the StateDB journal #2095

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
295a2d9
statedb: add cacheing for multistore before precompile runs
Unique-Divine Oct 25, 2024
80dd2d7
messy, working first version that allows for precompile reversion
Unique-Divine Oct 26, 2024
c624912
wip!: Save checkpoint.
Unique-Divine Oct 26, 2024
7f904a0
chore: changelog
Unique-Divine Oct 26, 2024
efbd52e
finalize bank keeper changes
Unique-Divine Oct 28, 2024
a8c100e
chore changelog
Unique-Divine Oct 28, 2024
0ba8c0f
reset cache
Unique-Divine Oct 29, 2024
08a73ee
finalize bank keeper changes
Unique-Divine Oct 28, 2024
717ce1c
revert to previous commit 7f904a07ac4d5555d8c088411024fc50ff65d085
Unique-Divine Oct 29, 2024
04a6897
fix strange ignored file issue
Unique-Divine Oct 29, 2024
17df448
fix strange ignored file issue
Unique-Divine Oct 29, 2024
7c34423
remove new bank keeper
Unique-Divine Oct 29, 2024
4e3bf3c
chore: comments from self-review
Unique-Divine Oct 29, 2024
8056328
chore: changelo g
Unique-Divine Oct 29, 2024
cc6a6a9
Merge branch 'ud/db-cache' into ud/db-cache2-bank
Unique-Divine Oct 29, 2024
c4ca8a2
fix(deps): update bank module to remove concrete type assertion on Ap…
Unique-Divine Oct 29, 2024
6a64e47
Merge branch 'main' into ud/db-cache2-bank
Unique-Divine Oct 30, 2024
70b6e5e
chore: merge conflicts
Unique-Divine Oct 30, 2024
53e8c59
revert experiment
Unique-Divine Oct 31, 2024
9f7160b
Merge branch 'main' into ud/db-cache2-bank
Unique-Divine Oct 31, 2024
a04fd06
finishing touch
Unique-Divine Oct 31, 2024
9aa3a64
fix incorrect arugmetn
Unique-Divine Oct 31, 2024
18892d2
feat(evmtest-erc20: Add optional descriptions to the assertions
Unique-Divine Oct 31, 2024
c88e9c5
Merge branch 'main' into ud/db-cache2-bank
Unique-Divine Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions CHANGELOG.md
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to better about having meaningful changelog entry descriptions and keep it organized rather than simply copying PR titles.

Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ consistent setup and dynamic gas calculations, addressing the following tickets.
- <https://github.com/code-423n4/2024-10-nibiru-zenith/issues/47>
- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution
- [#2090](https://github.com/NibiruChain/nibiru/pull/2090) - fix(evm): Account
for (1) ERC20 transfers with tokens that return false success values instead of
throwing an error and (2) ERC20 transfers with other operations that don't bring
about the expected resulting balance for the transfer recipient.
- [#2091](https://github.com/NibiruChain/nibiru/pull/2091) - feat(evm): add fun token creation fee validation
- [#2092](https://github.com/NibiruChain/nibiru/pull/2092) - feat(evm): add validation for wasm multi message execution
- [#2094](https://github.com/NibiruChain/nibiru/pull/2094) - fix(evm): Following
from the changs in #2086, this pull request implements a new `JournalChange`
struct that saves a deep copy of the state multi store before each
Expand All @@ -78,12 +83,17 @@ non-EVM and EVM state will be in sync even if there are complex, multi-step
Ethereum transactions, such as in the case of an EthereumTx that influences the
`StateDB`, then calls a precompile that also changes non-EVM state, and then EVM
reverts inside of a try-catch.
- [#2098](https://github.com/NibiruChain/nibiru/pull/2098) - test(evm): statedb tests for race conditions within funtoken precompile
- [#2090](https://github.com/NibiruChain/nibiru/pull/2090) - fix(evm): Account
for (1) ERC20 transfers with tokens that return false success values instead of
throwing an error and (2) ERC20 transfers with other operations that don't bring
about the expected resulting balance for the transfer recipient.
- [#2092](https://github.com/NibiruChain/nibiru/pull/2092) - feat(evm): add validation for wasm multi message execution
- [#2095](https://github.com/NibiruChain/nibiru/pull/2095) - fix(evm): This
change records NIBI (ether) transfers on the `StateDB` during precompiled
contract calls using the `NibiruBankKeeper`, which is struct extension of
the `bankkeeper.BaseKeeper` that is used throughout Nibiru.
The `NibiruBankKeeper` holds a reference to the current EVM `StateDB` and records
balance changes in wei as journal changes automatically. This guarantees that
commits and reversions of the `StateDB` do not misalign with the state of the
Bank module. This code change uses the `NibiruBankKeeper` on all modules that
depend on x/bank, such as the EVM and Wasm modules.
- [#2098](https://github.com/NibiruChain/nibiru/pull/2098) - test(evm): statedb
tests for race conditions within funtoken precompile

#### Nibiru EVM | Before Audit 1 - 2024-10-18

Expand Down
2 changes: 1 addition & 1 deletion app/ante/gas_wanted_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (s *AnteTestSuite) TestGasWantedDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
stateDB := deps.NewStateDB()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is renamed from StateDB to NewStateDB everywhere to reflect that fact that it's not a getter for an existing StateDB and instead generates and returns a new pointer for a StateDB with a fresh journal

anteDec := ante.AnteDecoratorGasWanted{}

tx := tc.txSetup(&deps)
Expand Down
2 changes: 1 addition & 1 deletion app/ante/handler_opts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type AnteHandlerOptions struct {
IBCKeeper *ibckeeper.Keeper
DevGasKeeper *devgaskeeper.Keeper
DevGasBankKeeper devgasante.BankKeeper
EvmKeeper evmkeeper.Keeper
EvmKeeper *evmkeeper.Keeper
AccountKeeper authkeeper.AccountKeeper

TxCounterStoreKey types.StoreKey
Expand Down
3 changes: 1 addition & 2 deletions app/evmante/evmante_can_transfer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ func (ctd CanTransferDecorator) AnteHandle(
BaseFeeWei: baseFeeWeiPerGas,
}

stateDB := statedb.New(
stateDB := ctd.NewStateDB(
ctx,
ctd.EVMKeeper,
statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes())),
)
Unique-Divine marked this conversation as resolved.
Show resolved Hide resolved
evmInstance := ctd.EVMKeeper.NewEVM(ctx, coreMsg, cfg, evm.NewNoOpTracer(), stateDB)
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_can_transfer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func (s *TestSuite) TestCanTransferDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.CanTransferDecorator{&deps.App.AppKeepers.EvmKeeper}
stateDB := deps.NewStateDB()
anteDec := evmante.CanTransferDecorator{deps.App.AppKeepers.EvmKeeper}
tx := tc.txSetup(&deps)

if tc.ctxSetup != nil {
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_emit_event_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func (s *TestSuite) TestEthEmitEventDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewEthEmitEventDecorator(&deps.App.AppKeepers.EvmKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewEthEmitEventDecorator(deps.App.AppKeepers.EvmKeeper)

tx := tc.txSetup(&deps)
s.Require().NoError(stateDB.Commit())
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_gas_consume_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ func (s *TestSuite) TestAnteDecEthGasConsume() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
stateDB := deps.NewStateDB()
anteDec := evmante.NewAnteDecEthGasConsume(
&deps.App.AppKeepers.EvmKeeper, tc.maxGasWanted,
deps.App.AppKeepers.EvmKeeper, tc.maxGasWanted,
)

tc.beforeTxSetup(&deps, stateDB)
Expand Down
18 changes: 9 additions & 9 deletions app/evmante/evmante_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ func NewAnteHandlerEVM(
) sdk.AnteHandler {
return sdk.ChainAnteDecorators(
// outermost AnteDecorator. SetUpContext must be called first
NewEthSetUpContextDecorator(&options.EvmKeeper),
NewMempoolGasPriceDecorator(&options.EvmKeeper),
NewEthValidateBasicDecorator(&options.EvmKeeper),
NewEthSigVerificationDecorator(&options.EvmKeeper),
NewAnteDecVerifyEthAcc(&options.EvmKeeper, options.AccountKeeper),
CanTransferDecorator{&options.EvmKeeper},
NewAnteDecEthGasConsume(&options.EvmKeeper, options.MaxTxGasWanted),
NewAnteDecEthIncrementSenderSequence(&options.EvmKeeper, options.AccountKeeper),
NewEthSetUpContextDecorator(options.EvmKeeper),
NewMempoolGasPriceDecorator(options.EvmKeeper),
NewEthValidateBasicDecorator(options.EvmKeeper),
NewEthSigVerificationDecorator(options.EvmKeeper),
NewAnteDecVerifyEthAcc(options.EvmKeeper, options.AccountKeeper),
CanTransferDecorator{options.EvmKeeper},
NewAnteDecEthGasConsume(options.EvmKeeper, options.MaxTxGasWanted),
NewAnteDecEthIncrementSenderSequence(options.EvmKeeper, options.AccountKeeper),
ante.AnteDecoratorGasWanted{},
// emit eth tx hash and index at the very last ante handler.
NewEthEmitEventDecorator(&options.EvmKeeper),
NewEthEmitEventDecorator(options.EvmKeeper),
)
}
2 changes: 1 addition & 1 deletion app/evmante/evmante_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (s *TestSuite) TestAnteHandlerEVM() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
stateDB := deps.NewStateDB()

anteHandlerEVM := evmante.NewAnteHandlerEVM(
ante.AnteHandlerOptions{
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_increment_sender_seq_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func (s *TestSuite) TestAnteDecEthIncrementSenderSequence() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewAnteDecEthIncrementSenderSequence(&deps.App.EvmKeeper, deps.App.AccountKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewAnteDecEthIncrementSenderSequence(deps.App.EvmKeeper, deps.App.AccountKeeper)

if tc.beforeTxSetup != nil {
tc.beforeTxSetup(&deps, stateDB)
Expand Down
2 changes: 1 addition & 1 deletion app/evmante/evmante_mempool_fees_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (s *TestSuite) TestMempoolGasFeeDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
anteDec := evmante.NewMempoolGasPriceDecorator(&deps.App.AppKeepers.EvmKeeper)
anteDec := evmante.NewMempoolGasPriceDecorator(deps.App.AppKeepers.EvmKeeper)

tx := tc.txSetup(&deps)

Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_setup_ctx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (

func (s *TestSuite) TestEthSetupContextDecorator() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewEthSetUpContextDecorator(&deps.App.EvmKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewEthSetUpContextDecorator(deps.App.EvmKeeper)

s.Require().NoError(stateDB.Commit())
tx := evmtest.HappyCreateContractTx(&deps)
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_sigverify_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func (s *TestSuite) TestEthSigVerificationDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewEthSigVerificationDecorator(&deps.App.AppKeepers.EvmKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewEthSigVerificationDecorator(deps.App.AppKeepers.EvmKeeper)

tx := tc.txSetup(&deps)
s.Require().NoError(stateDB.Commit())
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_validate_basic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ func (s *TestSuite) TestEthValidateBasicDecorator() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewEthValidateBasicDecorator(&deps.App.AppKeepers.EvmKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewEthValidateBasicDecorator(deps.App.AppKeepers.EvmKeeper)

tx := tc.txSetup(&deps)
s.Require().NoError(stateDB.Commit())
Expand Down
4 changes: 2 additions & 2 deletions app/evmante/evmante_verify_eth_acc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func (s *TestSuite) TestAnteDecoratorVerifyEthAcc_CheckTx() {
for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := evmante.NewAnteDecVerifyEthAcc(&deps.App.AppKeepers.EvmKeeper, &deps.App.AppKeepers.AccountKeeper)
stateDB := deps.NewStateDB()
anteDec := evmante.NewAnteDecVerifyEthAcc(deps.App.AppKeepers.EvmKeeper, &deps.App.AppKeepers.AccountKeeper)

tc.beforeTxSetup(&deps, stateDB)
tx := tc.txSetup(&deps)
Expand Down
29 changes: 16 additions & 13 deletions app/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ type AppKeepers struct {
}

type privateKeepers struct {
bankBaseKeeper bankkeeper.BaseKeeper
capabilityKeeper *capabilitykeeper.Keeper
slashingKeeper slashingkeeper.Keeper
crisisKeeper crisiskeeper.Keeper
Expand Down Expand Up @@ -264,14 +263,17 @@ func (app *NibiruApp) InitKeepers(
govModuleAddr,
)

app.bankBaseKeeper = bankkeeper.NewBaseKeeper(
appCodec,
keys[banktypes.StoreKey],
app.AccountKeeper,
BlockedAddresses(),
govModuleAddr,
)
app.BankKeeper = app.bankBaseKeeper
nibiruBankKeeper := &evmkeeper.NibiruBankKeeper{
BaseKeeper: bankkeeper.NewBaseKeeper(
appCodec,
keys[banktypes.StoreKey],
app.AccountKeeper,
BlockedAddresses(),
govModuleAddr,
),
StateDB: nil,
}
app.BankKeeper = nibiruBankKeeper
app.StakingKeeper = stakingkeeper.NewKeeper(
appCodec,
keys[stakingtypes.StoreKey],
Expand Down Expand Up @@ -367,16 +369,17 @@ func (app *NibiruApp) InitKeepers(
),
)

app.EvmKeeper = evmkeeper.NewKeeper(
evmKeeper := evmkeeper.NewKeeper(
appCodec,
keys[evm.StoreKey],
tkeys[evm.TransientKey],
authtypes.NewModuleAddress(govtypes.ModuleName),
app.AccountKeeper,
app.BankKeeper,
nibiruBankKeeper,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Written this way to make sure the EVM keeper and its constructor have the stronger typed NibiruBankKeeper rather than an a bankkeeper.Keeper interface

app.StakingKeeper,
cast.ToString(appOpts.Get("evm.tracer")),
)
app.EvmKeeper = &evmKeeper

// ---------------------------------- IBC keepers

Expand Down Expand Up @@ -608,7 +611,7 @@ func (app *NibiruApp) initAppModules(
),
auth.NewAppModule(appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts, app.GetSubspace(authtypes.ModuleName)),
vesting.NewAppModule(app.AccountKeeper, app.BankKeeper),
bank.NewAppModule(appCodec, app.bankBaseKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
capability.NewAppModule(appCodec, *app.capabilityKeeper, false),
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
Expand All @@ -633,7 +636,7 @@ func (app *NibiruApp) initAppModules(
ibcfee.NewAppModule(app.ibcFeeKeeper),
ica.NewAppModule(&app.icaControllerKeeper, &app.icaHostKeeper),

evmmodule.NewAppModule(&app.EvmKeeper, app.AccountKeeper),
evmmodule.NewAppModule(app.EvmKeeper, app.AccountKeeper),

// wasm
wasm.NewAppModule(
Expand Down
2 changes: 1 addition & 1 deletion app/keepers/all_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type PublicKeepers struct {
SudoKeeper keeper.Keeper
DevGasKeeper devgaskeeper.Keeper
TokenFactoryKeeper tokenfactorykeeper.Keeper
EvmKeeper evmkeeper.Keeper
EvmKeeper *evmkeeper.Keeper

// WASM keepers
WasmKeeper wasmkeeper.Keeper
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ replace (
cosmossdk.io/api => cosmossdk.io/api v0.3.1

github.com/CosmWasm/wasmd => github.com/NibiruChain/wasmd v0.44.0-nibiru
github.com/cosmos/cosmos-sdk => github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru
github.com/cosmos/cosmos-sdk => github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru.2

github.com/cosmos/iavl => github.com/cosmos/iavl v0.20.0

Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
github.com/NibiruChain/collections v0.5.0 h1:33pXpVTe1PK/tfdZlAJF1JF7AdzGNARG+iL9G/z3X7k=
github.com/NibiruChain/collections v0.5.0/go.mod h1:43L6yjuF0BMre/mw4gqn/kUOZz1c2Y3huZ/RQfBFrOQ=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru h1:PgFpxDe+7+OzWHs4zXlml5j2i9sGq2Zpd3ndYQG29/0=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru/go.mod h1:ADjORYzUQqQv/FxDi0H0K5gW/rAk1CiDR3ZKsExfJV0=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru.2 h1:HtLNrkp0HhgxtbpdNwsasOod4uUNFpuwYpyceFtbj3E=
github.com/NibiruChain/cosmos-sdk v0.47.11-nibiru.2/go.mod h1:ADjORYzUQqQv/FxDi0H0K5gW/rAk1CiDR3ZKsExfJV0=
github.com/NibiruChain/go-ethereum v1.10.27-nibiru h1:o6lRFt57izoYwzN5cG8tnnBtJcaO3X7MjjN7PGGNCFg=
github.com/NibiruChain/go-ethereum v1.10.27-nibiru/go.mod h1:kvvL3nDceUcB+1qGUBAsVf5dW23RBR77fqxgx2PGNrQ=
github.com/NibiruChain/wasmd v0.44.0-nibiru h1:b+stNdbMFsl0+o4KedXyF83qRnEpB/jCiTGZZgv2h2U=
Expand Down
Loading
Loading