From 8d8a2490d3e0168f22a1fae064efd52ecea50213 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Mon, 21 Oct 2024 04:22:55 -0500 Subject: [PATCH 01/12] commit the cached state at the beginning of precompile runs --- x/evm/evmtest/erc20.go | 2 +- x/evm/keeper/erc20.go | 7 ++- x/evm/logs.go | 8 --- x/evm/logs_test.go | 5 +- x/evm/precompile/abci_event.go | 24 +++++++++ x/evm/precompile/funtoken.go | 41 ++++----------- x/evm/precompile/funtoken_test.go | 4 +- x/evm/precompile/oracle.go | 28 ++--------- x/evm/precompile/precompile.go | 60 ++++++++++++++++++++++ x/evm/precompile/wasm.go | 27 +++------- x/evm/statedb/interfaces.go | 2 +- x/evm/statedb/journal.go | 84 ++++++++++++++++++++++++------- x/evm/statedb/state_object.go | 2 +- x/evm/statedb/statedb.go | 64 ++++++++++++++++++++--- 14 files changed, 243 insertions(+), 115 deletions(-) create mode 100644 x/evm/precompile/abci_event.go diff --git a/x/evm/evmtest/erc20.go b/x/evm/evmtest/erc20.go index ce020036f..773005661 100644 --- a/x/evm/evmtest/erc20.go +++ b/x/evm/evmtest/erc20.go @@ -23,7 +23,7 @@ func AssertERC20BalanceEqual( ) { actualBalance, err := deps.EvmKeeper.ERC20().BalanceOf(erc20, account, deps.Ctx) assert.NoError(t, err) - assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance) + assert.Equal(t, expectedBalance.String(), actualBalance.String(), "expected %s, got %s", expectedBalance, actualBalance) } // CreateFunTokenForBankCoin: Uses the "TestDeps.Sender" account to create a diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index abecd80ca..f5e4f2123 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -213,7 +213,12 @@ func (k Keeper) CallContractWithInput( return nil, errors.Wrapf(err, "failed to load evm config") } - txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash())) + blockHash := gethcommon.BytesToHash(ctx.HeaderHash()) + // txIdx := uint(k.EvmState.BlockTxIndex.GetOr(ctx, 0)) // TxIndex + // txLogIdx := uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) // LogIndex + ctx.TxBytes() + txConfig := statedb.NewEmptyTxConfig(blockHash) + evmResp, err = k.ApplyEvmMsg( ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, ) diff --git a/x/evm/logs.go b/x/evm/logs.go index 21bc74db5..3b662964f 100644 --- a/x/evm/logs.go +++ b/x/evm/logs.go @@ -11,14 +11,6 @@ import ( "github.com/NibiruChain/nibiru/v2/eth" ) -// NewTransactionLogs creates a new NewTransactionLogs instance. -func NewTransactionLogs(hash gethcommon.Hash, logs []*Log) TransactionLogs { - return TransactionLogs{ - Hash: hash.String(), - Logs: logs, - } -} - // NewTransactionLogsFromEth creates a new NewTransactionLogs instance using []*ethtypes.Log. func NewTransactionLogsFromEth(hash gethcommon.Hash, ethlogs []*gethcore.Log) TransactionLogs { return TransactionLogs{ diff --git a/x/evm/logs_test.go b/x/evm/logs_test.go index 34cb80a12..66dc1105c 100644 --- a/x/evm/logs_test.go +++ b/x/evm/logs_test.go @@ -193,7 +193,10 @@ func TestConversionFunctions(t *testing.T) { conversionErr := conversionLogs.Validate() // create new transaction logs as copy of old valid one (and validate) - copyLogs := evm.NewTransactionLogs(common.BytesToHash([]byte("tx_hash")), txLogs.Logs) + copyLogs := evm.TransactionLogs{ + Hash: common.BytesToHash([]byte("tx_hash")).Hex(), + Logs: txLogs.Logs, + } copyErr := copyLogs.Validate() require.Nil(t, conversionErr) diff --git a/x/evm/precompile/abci_event.go b/x/evm/precompile/abci_event.go new file mode 100644 index 000000000..beae86a89 --- /dev/null +++ b/x/evm/precompile/abci_event.go @@ -0,0 +1,24 @@ +package precompile + +import ( + // "github.com/ethereum/go-ethereum/accounts/abi" + gethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + // "github.com/NibiruChain/nibiru/v2/x/evm/statedb" +) + +func MakeTopicFromStr(s string) (topic gethcommon.Hash) { + return MakeTopicFromBytes([]byte(s)) +} + +func MakeTopicFromBytes(bz []byte) (topic gethcommon.Hash) { + hash := crypto.Keccak256Hash(bz) + copy(topic[:], hash[:]) + return topic +} + +// func (p *precompileWasm) foo(stateDB *statedb.StateDB) { +// abi.MakeTopics() +// // stateDB.AddLog() +// // p.evmKeeper.State +// } diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 042544269..bcafb0c55 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -3,7 +3,6 @@ package precompile import ( "fmt" "math/big" - "reflect" "sync" "cosmossdk.io/math" @@ -12,13 +11,11 @@ import ( gethabi "github.com/ethereum/go-ethereum/accounts/abi" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" - gethparams "github.com/ethereum/go-ethereum/params" "github.com/NibiruChain/nibiru/v2/app/keepers" "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper" - "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) var _ vm.PrecompiledContract = (*precompileFunToken)(nil) @@ -34,15 +31,7 @@ func (p precompileFunToken) Address() gethcommon.Address { // RequiredGas calculates the cost of calling the precompile in gas units. func (p precompileFunToken) RequiredGas(input []byte) (gasCost uint64) { - // Since [gethparams.TxGas] is the cost per (Ethereum) transaction that does not create - // a contract, it's value can be used to derive an appropriate value for the - // precompile call. The FunToken precompile performs 3 operations, labeled 1-3 - // below: - // 0 | Call the precompile (already counted in gas calculation) - // 1 | Send ERC20 to EVM. - // 2 | Convert ERC20 to coin - // 3 | Send coin to recipient. - return gethparams.TxGas * 2 + return RequiredGas(input, embeds.SmartContract_FunToken.ABI) } const ( @@ -55,27 +44,13 @@ type PrecompileMethod string func (p precompileFunToken) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - // This is a `defer` pattern to add behavior that runs in the case that the error is - // non-nil, creating a concise way to add extra information. - defer func() { - if err != nil { - precompileType := reflect.TypeOf(p).Name() - err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) - } - }() - - // 1 | Get context from StateDB - stateDB, ok := evm.StateDB.(*statedb.StateDB) - if !ok { - err = fmt.Errorf("failed to load the sdk.Context from the EVM StateDB") - return - } - ctx := stateDB.GetContext() - - method, args, err := DecomposeInput(embeds.SmartContract_FunToken.ABI, contract.Input) + defer ErrPrecompileRun(err, p)() + res, err := OnRunStart(evm, contract, embeds.SmartContract_FunToken.ABI) if err != nil { return nil, err } + method, args := res.Method, res.Args + ctx := res.Ctx switch PrecompileMethod(method.Name) { case FunTokenMethod_BankSend: @@ -87,7 +62,11 @@ func (p precompileFunToken) Run( return } - return + if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { + return nil, err + } + res.WriteCtx() + return bz, nil } func PrecompileFunToken(keepers keepers.PublicKeepers) vm.PrecompiledContract { diff --git a/x/evm/precompile/funtoken_test.go b/x/evm/precompile/funtoken_test.go index 64be0360f..9aad9feb3 100644 --- a/x/evm/precompile/funtoken_test.go +++ b/x/evm/precompile/funtoken_test.go @@ -133,7 +133,7 @@ func (s *FuntokenSuite) TestHappyPath() { evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, deps.Sender.EthAddr, big.NewInt(69_000)) evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(0)) - s.Equal(sdk.NewInt(420), - deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, funtoken.BankDenom).Amount, + s.Equal(sdk.NewInt(420).String(), + deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, funtoken.BankDenom).Amount.String(), ) } diff --git a/x/evm/precompile/oracle.go b/x/evm/precompile/oracle.go index 7c3a57af5..568c0d4fb 100644 --- a/x/evm/precompile/oracle.go +++ b/x/evm/precompile/oracle.go @@ -2,18 +2,15 @@ package precompile import ( "fmt" - "reflect" sdk "github.com/cosmos/cosmos-sdk/types" gethabi "github.com/ethereum/go-ethereum/accounts/abi" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" - gethparams "github.com/ethereum/go-ethereum/params" "github.com/NibiruChain/nibiru/v2/app/keepers" "github.com/NibiruChain/nibiru/v2/x/common/asset" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" - "github.com/NibiruChain/nibiru/v2/x/evm/statedb" oraclekeeper "github.com/NibiruChain/nibiru/v2/x/oracle/keeper" ) @@ -27,9 +24,7 @@ func (p precompileOracle) Address() gethcommon.Address { } func (p precompileOracle) RequiredGas(input []byte) (gasPrice uint64) { - // Since [gethparams.TxGas] is the cost per (Ethereum) transaction that does not create - // a contract, it's value can be used to derive an appropriate value for the precompile call. - return gethparams.TxGas + return RequiredGas(input, embeds.SmartContract_Oracle.ABI) } const ( @@ -42,27 +37,12 @@ type OracleMethod string func (p precompileOracle) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - // This is a `defer` pattern to add behavior that runs in the case that the error is - // non-nil, creating a concise way to add extra information. - defer func() { - if err != nil { - precompileType := reflect.TypeOf(p).Name() - err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) - } - }() - - // 1 | Get context from StateDB - stateDB, ok := evm.StateDB.(*statedb.StateDB) - if !ok { - err = fmt.Errorf("failed to load the sdk.Context from the EVM StateDB") - return - } - ctx := stateDB.GetContext() - - method, args, err := DecomposeInput(embeds.SmartContract_Oracle.ABI, contract.Input) + defer ErrPrecompileRun(err, p)() + res, err := OnRunStart(evm, contract, embeds.SmartContract_Oracle.ABI) if err != nil { return nil, err } + method, args, ctx := res.Method, res.Args, res.Ctx switch OracleMethod(method.Name) { case OracleMethod_QueryExchangeRate: diff --git a/x/evm/precompile/precompile.go b/x/evm/precompile/precompile.go index 38a8744c1..7153dd575 100644 --- a/x/evm/precompile/precompile.go +++ b/x/evm/precompile/precompile.go @@ -16,6 +16,7 @@ package precompile import ( "bytes" "fmt" + "reflect" "github.com/NibiruChain/collections" store "github.com/cosmos/cosmos-sdk/store/types" @@ -24,7 +25,10 @@ import ( "github.com/ethereum/go-ethereum/core/vm" gethparams "github.com/ethereum/go-ethereum/params" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/NibiruChain/nibiru/v2/app/keepers" + "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) // InitPrecompiles initializes and returns a map of precompiled contracts for the EVM. @@ -130,3 +134,59 @@ func RequiredGas(input []byte, abi *gethabi.ABI) uint64 { argsBzLen := uint64(len(input[4:])) return (costPerByte * argsBzLen) + costFlat } + +// This is a `defer` pattern to add behavior that runs in the case that the error is +// non-nil, creating a concise way to add extra information. +func ErrPrecompileRun(err error, p vm.PrecompiledContract) func() { + return func() { + if err != nil { + precompileType := reflect.TypeOf(p).Name() + err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) + } + } +} + +type PrecompileStartResult struct { + Args []any + Ctx sdk.Context + WriteCtx func() + Method *gethabi.Method + SnapshotBeforeRun statedb.PrecompileSnapshotBeforeRun + StateDB *statedb.StateDB +} + +func OnRunStart( + evm *vm.EVM, contract *vm.Contract, abi *gethabi.ABI, +) (res PrecompileStartResult, err error) { + method, args, err := DecomposeInput(abi, contract.Input) + if err != nil { + return res, err + } + + stateDB, ok := evm.StateDB.(*statedb.StateDB) + if !ok { + err = fmt.Errorf("failed to load the sdk.Context from the EVM StateDB") + return + } + cacheCtx, writeCacheCtx, stateDBSnapshot := stateDB.CacheCtxForPrecompile() + if err = stateDB.CommitContext(cacheCtx); err != nil { + return res, fmt.Errorf("error committing dirty journal entries for the precompile call to the cache ctx: %w", err) + } + + return PrecompileStartResult{ + Args: args, + Ctx: cacheCtx, + WriteCtx: writeCacheCtx, + Method: method, + SnapshotBeforeRun: stateDBSnapshot, + StateDB: stateDB, + }, nil +} + +func OnRunEnd( + stateDB *statedb.StateDB, + snapshot statedb.PrecompileSnapshotBeforeRun, + precompileAddr gethcommon.Address, +) error { + return stateDB.SavePrecompileSnapshotToJournal(precompileAddr, snapshot) +} diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index 091999ee3..ad4484f29 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -2,7 +2,6 @@ package precompile import ( "fmt" - "reflect" sdk "github.com/cosmos/cosmos-sdk/types" @@ -14,8 +13,6 @@ import ( gethabi "github.com/ethereum/go-ethereum/accounts/abi" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" - - "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) var _ vm.PrecompiledContract = (*precompileWasm)(nil) @@ -75,26 +72,12 @@ func (p precompileWasm) RequiredGas(input []byte) (gasCost uint64) { func (p precompileWasm) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - // This is a `defer` pattern to add behavior that runs in the case that the error is - // non-nil, creating a concise way to add extra information. - defer func() { - if err != nil { - precompileType := reflect.TypeOf(p).Name() - err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) - } - }() - - method, args, err := DecomposeInput(embeds.SmartContract_Wasm.ABI, contract.Input) + defer ErrPrecompileRun(err, p)() + res, err := OnRunStart(evm, contract, embeds.SmartContract_Wasm.ABI) if err != nil { return nil, err } - - stateDB, ok := evm.StateDB.(*statedb.StateDB) - if !ok { - err = fmt.Errorf("failed to load the sdk.Context from the EVM StateDB") - return - } - ctx := stateDB.GetContext() + method, args, ctx := res.Method, res.Args, res.Ctx switch PrecompileMethod(method.Name) { case WasmMethod_execute: @@ -114,6 +97,10 @@ func (p precompileWasm) Run( return } + if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { + return nil, err + } + res.WriteCtx() return } diff --git a/x/evm/statedb/interfaces.go b/x/evm/statedb/interfaces.go index 4ef8b2862..a4c1c3b59 100644 --- a/x/evm/statedb/interfaces.go +++ b/x/evm/statedb/interfaces.go @@ -14,7 +14,7 @@ import ( // stateful precompiled contracts. type ExtStateDB interface { vm.StateDB - AppendJournalEntry(JournalEntry) + AppendJournalEntry(JournalChange) } // Keeper provide underlying storage of StateDB diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index 14bb7b1df..c4a02a950 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -18,15 +18,18 @@ package statedb import ( "bytes" + "fmt" "math/big" "sort" + store "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" ) -// JournalEntry is a modification entry in the state change journal that can be +// JournalChange is a modification entry in the state change journal that can be // Reverted on demand. -type JournalEntry interface { +type JournalChange interface { // Revert undoes the changes introduced by this journal entry. Revert(*StateDB) @@ -38,7 +41,7 @@ type JournalEntry interface { // commit. These are tracked to be able to be reverted in the case of an execution // exception or request for reversal. type journal struct { - entries []JournalEntry // Current changes tracked by the journal + entries []JournalChange // Current changes tracked by the journal dirties map[common.Address]int // Dirty accounts and the number of changes } @@ -62,7 +65,7 @@ func (j *journal) sortedDirties() []common.Address { } // append inserts a new modification entry to the end of the change journal. -func (j *journal) append(entry JournalEntry) { +func (j *journal) append(entry JournalChange) { j.entries = append(j.entries, entry) if addr := entry.Dirtied(); addr != nil { j.dirties[*addr]++ @@ -92,10 +95,14 @@ func (j *journal) length() int { } type ( - // Changes to the account trie. + // createObjectChange: [JournalChange] implementation for when + // a new account (called an "object" in this context) is created in state. createObjectChange struct { account *common.Address } + // resetObjectChange: [JournalChange] implementation for when + // a backup for a state object is needed in order to revert state changes + // that overwrite an existing object. resetObjectChange struct { prev *stateObject } @@ -105,11 +112,12 @@ type ( prevbalance *big.Int } - // Changes to individual accounts. + // balanceChange: [JournalChange] for an update to an account's wei balance balanceChange struct { account *common.Address - prev *big.Int + prevWei *big.Int } + // nonceChange: [JournalChange] for an update to an account's nonce. nonceChange struct { account *common.Address prev uint64 @@ -133,12 +141,55 @@ type ( accessListAddAccountChange struct { address *common.Address } + // accessListAddSlotChange: [JournalChange] implementations for accessListAddSlotChange struct { address *common.Address slot *common.Hash } ) +// ------------------------------------------------------ +// PrecompileSnapshotBeforeRun + +var _ JournalChange = PrecompileSnapshotBeforeRun{} + +type PrecompileSnapshotBeforeRun struct { + MultiStore store.CacheMultiStore + Events sdk.Events +} + +func (ch PrecompileSnapshotBeforeRun) Revert(s *StateDB) { + // TODO: Revert PrecompileSnapshotBeforeRun + // Restore the multistore recorded in the journal entry + + fmt.Printf("\"Revert was called.\"\n") + + s.cacheCtx = s.cacheCtx.WithMultiStore(ch.MultiStore) + // Rewrite the `writeCacheCtxFn` using the same logic as sdk.Context.CacheCtx + s.writeCacheCtxFn = func() { + s.ctx.EventManager().EmitEvents(ch.Events) + ch.MultiStore.Write() + } +} + +func (ch PrecompileSnapshotBeforeRun) Dirtied() *common.Address { + return nil +} + +var ( + _ JournalChange = accessListAddSlotChange{} + _ JournalChange = createObjectChange{} + _ JournalChange = resetObjectChange{} + _ JournalChange = suicideChange{} + _ JournalChange = balanceChange{} + _ JournalChange = nonceChange{} + _ JournalChange = storageChange{} + _ JournalChange = codeChange{} + _ JournalChange = refundChange{} + _ JournalChange = addLogChange{} + _ JournalChange = accessListAddAccountChange{} +) + func (ch createObjectChange) Revert(s *StateDB) { delete(s.stateObjects, *ch.account) } @@ -168,7 +219,7 @@ func (ch suicideChange) Dirtied() *common.Address { } func (ch balanceChange) Revert(s *StateDB) { - s.getStateObject(*ch.account).setBalance(ch.prev) + s.getStateObject(*ch.account).setBalance(ch.prevWei) } func (ch balanceChange) Dirtied() *common.Address { @@ -215,16 +266,15 @@ func (ch addLogChange) Dirtied() *common.Address { return nil } +// When an (address, slot) combination is added, it always results in two +// journal entries if the address is not already present: +// 1. `accessListAddAccountChange`: a journal change for the address +// 2. `accessListAddSlotChange`: a journal change for the (address, slot) +// combination. +// +// Thus, when reverting, we can safely delete the address, as no storage slots +// remain once the address entry is reverted. func (ch accessListAddAccountChange) Revert(s *StateDB) { - /* - One important invariant here, is that whenever a (addr, slot) is added, if the - addr is not already present, the add causes two journal entries: - - one for the address, - - one for the (address,slot) - Therefore, when unrolling the change, we can always blindly delete the - (addr) at this point, since no storage adds can remain when come upon - a single (addr) change. - */ s.accessList.DeleteAddress(*ch.address) } diff --git a/x/evm/statedb/state_object.go b/x/evm/statedb/state_object.go index bebbf7b40..e357de7d1 100644 --- a/x/evm/statedb/state_object.go +++ b/x/evm/statedb/state_object.go @@ -172,7 +172,7 @@ func (s *stateObject) SubBalance(amount *big.Int) { func (s *stateObject) SetBalance(amount *big.Int) { s.db.journal.append(balanceChange{ account: &s.address, - prev: new(big.Int).Set(s.account.BalanceWei), + prevWei: new(big.Int).Set(s.account.BalanceWei), }) s.setBalance(amount) } diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index 27b81a3ce..b917845a7 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -30,7 +30,18 @@ var _ vm.StateDB = &StateDB{} // * Accounts type StateDB struct { keeper Keeper - ctx sdk.Context + // ctx is the persistent context used for official `StateDB.Commit` calls. + ctx sdk.Context + // cacheCtx: An sdk.Context produced from the [StateDB.ctx] with the + // multi-store cached and a new event manager. The cached context + // (`cacheCtx`) is written to the persistent context (`ctx`) when + // `writeCacheCtx` is called. + cacheCtx sdk.Context + // Events are automatically emitted on the parent context's + // EventManager when the caller executes the [writeCacheCtxFn]. + writeCacheCtxFn func() + // THe number of precompiled contract calls within the current transaction + precompileSnapshotsCount uint8 // Journal of state modifications. This is the backbone of // Snapshot and RevertToSnapshot. @@ -449,20 +460,21 @@ func errorf(format string, args ...any) error { return fmt.Errorf("StateDB error: "+format, args...) } -// Commit writes the dirty states to keeper -// the StateDB object should be discarded after committed. -func (s *StateDB) Commit() error { +// CommitContext writes the dirty journal state changes to the EVM Keeper. The +// StateDB object cannot be reused after [CommitContext] has completed. A new +// object needs to be created from the EVM. +func (s *StateDB) CommitContext(ctx sdk.Context) error { for _, addr := range s.journal.sortedDirties() { obj := s.stateObjects[addr] if obj.suicided { - if err := s.keeper.DeleteAccount(s.ctx, obj.Address()); err != nil { + if err := s.keeper.DeleteAccount(ctx, obj.Address()); err != nil { return errorf("failed to delete account: %w") } } else { if obj.code != nil && obj.dirtyCode { - s.keeper.SetCode(s.ctx, obj.CodeHash(), obj.code) + s.keeper.SetCode(ctx, obj.CodeHash(), obj.code) } - if err := s.keeper.SetAccount(s.ctx, obj.Address(), obj.account.ToNative()); err != nil { + if err := s.keeper.SetAccount(ctx, obj.Address(), obj.account.ToNative()); err != nil { return errorf("failed to set account: %w") } for _, key := range obj.dirtyStorage.SortedKeys() { @@ -471,13 +483,21 @@ func (s *StateDB) Commit() error { if value == obj.originStorage[key] { continue } - s.keeper.SetState(s.ctx, obj.Address(), key, value.Bytes()) + s.keeper.SetState(ctx, obj.Address(), key, value.Bytes()) } } } return nil } +// Commit writes the dirty journal state changes to the EVM Keeper. The +// StateDB object cannot be reused after [CommitContext] has completed. A new +// object needs to be created from the EVM. +func (s *StateDB) Commit() error { + ctx := s.ctx + return s.CommitContext(ctx) +} + // StateObjects: Returns a copy of the [StateDB.stateObjects] map. func (s *StateDB) StateObjects() map[common.Address]*stateObject { copyOfMap := make(map[common.Address]*stateObject) @@ -486,3 +506,31 @@ func (s *StateDB) StateObjects() map[common.Address]*stateObject { } return copyOfMap } + +func (s *StateDB) CacheCtxForPrecompile() ( + sdk.Context, func(), PrecompileSnapshotBeforeRun, +) { + if s.writeCacheCtxFn == nil { + s.cacheCtx, s.writeCacheCtxFn = s.ctx.CacheContext() + } + return s.cacheCtx, s.writeCacheCtxFn, PrecompileSnapshotBeforeRun{ + MultiStore: s.cacheCtx.MultiStore().CacheMultiStore(), + Events: s.cacheCtx.EventManager().Events(), + } +} + +func (s *StateDB) SavePrecompileSnapshotToJournal( + precompileAddr common.Address, + snapshot PrecompileSnapshotBeforeRun, +) error { + fmt.Println("SavePrecompileSnapshotToJournal was called") + obj := s.getOrNewStateObject(precompileAddr) + obj.db.journal.append(snapshot) + s.precompileSnapshotsCount++ + if s.precompileSnapshotsCount > maxPrecompileCalls { + return fmt.Errorf("exceeded maximum allowed number of precompiled contract calls in one transaction (%d)", maxPrecompileCalls) + } + return nil +} + +const maxPrecompileCalls uint8 = 10 From 06f0fa2c6a452f8e2cdd715c562a7f1d2e889234 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Mon, 21 Oct 2024 04:42:05 -0500 Subject: [PATCH 02/12] chore changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45c9f61da..1f3c02c33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Use effective gas price in RefundGas and make sure that units are properly reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059) and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724). +- [#2xxx](https://github.com/NibiruChain/nibiru/pull/2xxx) - fix(evm-precompiles): It is possible for the `Run` function of a custom rpecompile to retrieve stale state because EVM state changes can occur before the precompile is called that are recorded as entries in the StateDB journal for the transaction without being reflected in the `sdk.Context`. This pull request makes sure that state is committed as expected. #### Dapp modules: perp, spot, oracle, etc From 55f75c1aa85dce534be48bdec616d3d737b019c9 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Mon, 21 Oct 2024 21:37:29 -0500 Subject: [PATCH 03/12] fix funtoken.sol precompile and test --- x/evm/evmtest/test_deps.go | 6 ++ x/evm/evmtest/tx.go | 86 +++++++++++++++++++----- x/evm/keeper/erc20.go | 5 +- x/evm/keeper/funtoken_from_coin_test.go | 2 +- x/evm/keeper/funtoken_from_erc20_test.go | 15 +++-- x/evm/precompile/abci_event.go | 24 ------- x/evm/precompile/funtoken.go | 5 +- x/evm/precompile/funtoken_test.go | 16 +++-- x/evm/precompile/wasm.go | 3 + x/evm/statedb/journal.go | 2 +- x/evm/statedb/statedb.go | 1 - 11 files changed, 105 insertions(+), 60 deletions(-) delete mode 100644 x/evm/precompile/abci_event.go diff --git a/x/evm/evmtest/test_deps.go b/x/evm/evmtest/test_deps.go index 9c6015933..1810b1c8f 100644 --- a/x/evm/evmtest/test_deps.go +++ b/x/evm/evmtest/test_deps.go @@ -1,6 +1,8 @@ package evmtest import ( + "context" + sdk "github.com/cosmos/cosmos-sdk/types" gethcommon "github.com/ethereum/go-ethereum/common" @@ -54,3 +56,7 @@ func (deps TestDeps) StateDB() *statedb.StateDB { func (deps *TestDeps) GethSigner() gethcore.Signer { return gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx)) } + +func (deps TestDeps) GoCtx() context.Context { + return sdk.WrapSDKContext(deps.Ctx) +} diff --git a/x/evm/evmtest/tx.go b/x/evm/evmtest/tx.go index 107a15593..01e9a97ed 100644 --- a/x/evm/evmtest/tx.go +++ b/x/evm/evmtest/tx.go @@ -20,8 +20,11 @@ import ( srvconfig "github.com/NibiruChain/nibiru/v2/app/server/config" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" + "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) type GethTxType = uint8 @@ -123,7 +126,9 @@ func ExecuteNibiTransfer(deps *TestDeps, t *testing.T) *evm.MsgEthereumTx { To: &recipient, Nonce: (*hexutil.Uint64)(&nonce), } - ethTxMsg, err := GenerateAndSignEthTxMsg(txArgs, deps) + ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender) + require.NoError(t, err) + err = ethTxMsg.Sign(gethSigner, krSigner) require.NoError(t, err) resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg) @@ -153,18 +158,20 @@ func DeployContract( bytecodeForCall := append(contract.Bytecode, packedArgs...) nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr) - msgEthTx, err := GenerateAndSignEthTxMsg( + ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner( evm.JsonTxArgs{ Nonce: (*hexutil.Uint64)(&nonce), Input: (*hexutil.Bytes)(&bytecodeForCall), From: &deps.Sender.EthAddr, - }, deps, + }, deps, deps.Sender, ) if err != nil { return nil, errors.Wrap(err, "failed to generate and sign eth tx msg") + } else if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil { + return nil, errors.Wrap(err, "failed to generate and sign eth tx msg") } - resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), msgEthTx) + resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg) if err != nil { return nil, errors.Wrap(err, "failed to execute ethereum tx") } @@ -174,7 +181,7 @@ func DeployContract( return &DeployContractResult{ TxResp: resp, - EthTxMsg: msgEthTx, + EthTxMsg: ethTxMsg, ContractData: contract, Nonce: nonce, ContractAddr: crypto.CreateAddress(deps.Sender.EthAddr, nonce), @@ -210,23 +217,70 @@ func DeployAndExecuteERC20Transfer( Nonce: (*hexutil.Uint64)(&nonce), Data: (*hexutil.Bytes)(&input), } - erc20Transfer, err = GenerateAndSignEthTxMsg(txArgs, deps) + erc20Transfer, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender) + require.NoError(t, err) + err = erc20Transfer.Sign(gethSigner, krSigner) require.NoError(t, err) - resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), erc20Transfer) + resp, err := deps.App.EvmKeeper.EthereumTx(deps.GoCtx(), erc20Transfer) require.NoError(t, err) require.Empty(t, resp.VmError) return erc20Transfer, predecessors } -// GenerateAndSignEthTxMsg estimates gas, sets gas limit and sings the tx -func GenerateAndSignEthTxMsg( - jsonTxArgs evm.JsonTxArgs, deps *TestDeps, -) (*evm.MsgEthereumTx, error) { +func CallContractTx( + deps *TestDeps, + contractAddr gethcommon.Address, + input []byte, + sender EthPrivKeyAcc, +) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) { + nonce := deps.StateDB().GetNonce(sender.EthAddr) + ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{ + From: &sender.EthAddr, + To: &contractAddr, + Nonce: (*hexutil.Uint64)(&nonce), + Data: (*hexutil.Bytes)(&input), + }, deps, sender) + if err != nil { + err = fmt.Errorf("CallContract error during tx generation: %w", err) + return + } + + txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash)) + stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig) + err = stateDB.Commit() + if err != nil { + return + } + + err = ethTxMsg.Sign(gethSigner, krSigner) + if err != nil { + err = fmt.Errorf("CallContract error during signature: %w", err) + return + } + + resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg) + return ethTxMsg, resp, err +} + +// GenerateEthTxMsgAndSigner estimates gas, sets gas limit and returns signer for +// the tx. +// +// Usage: +// +// ```go +// evmTxMsg, gethSigner, krSigner, _ := GenerateEthTxMsgAndSigner( +// jsonTxArgs, &deps, sender, +// ) +// err := evmTxMsg.Sign(gethSigner, sender.KeyringSigner) +// ``` +func GenerateEthTxMsgAndSigner( + jsonTxArgs evm.JsonTxArgs, deps *TestDeps, sender EthPrivKeyAcc, +) (evmTxMsg *evm.MsgEthereumTx, gethSigner gethcore.Signer, krSigner keyring.Signer, err error) { estimateArgs, err := json.Marshal(&jsonTxArgs) if err != nil { - return nil, err + return } res, err := deps.App.EvmKeeper.EstimateGas( sdk.WrapSDKContext(deps.Ctx), @@ -238,13 +292,13 @@ func GenerateAndSignEthTxMsg( }, ) if err != nil { - return nil, err + return } jsonTxArgs.Gas = (*hexutil.Uint64)(&res.Gas) - msgEthTx := jsonTxArgs.ToMsgEthTx() - gethSigner := gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx)) - return msgEthTx, msgEthTx.Sign(gethSigner, deps.Sender.KeyringSigner) + evmTxMsg = jsonTxArgs.ToMsgEthTx() + gethSigner = gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx)) + return evmTxMsg, gethSigner, sender.KeyringSigner, nil } func TransferWei( diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index f5e4f2123..4196bc683 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -214,10 +214,9 @@ func (k Keeper) CallContractWithInput( } blockHash := gethcommon.BytesToHash(ctx.HeaderHash()) - // txIdx := uint(k.EvmState.BlockTxIndex.GetOr(ctx, 0)) // TxIndex - // txLogIdx := uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) // LogIndex - ctx.TxBytes() txConfig := statedb.NewEmptyTxConfig(blockHash) + txConfig.TxIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) + txConfig.LogIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) evmResp, err = k.ApplyEvmMsg( ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, diff --git a/x/evm/keeper/funtoken_from_coin_test.go b/x/evm/keeper/funtoken_from_coin_test.go index cb9b87ffb..1f5e3a85a 100644 --- a/x/evm/keeper/funtoken_from_coin_test.go +++ b/x/evm/keeper/funtoken_from_coin_test.go @@ -282,7 +282,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { // Check 3: erc-20 balance balance, err = deps.EvmKeeper.ERC20().BalanceOf(funTokenErc20Addr.Address, alice.EthAddr, deps.Ctx) s.Require().NoError(err) - s.Require().Zero(balance.Cmp(big.NewInt(0))) + s.Require().Equal("0", balance.String()) s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds") _, err = deps.EvmKeeper.CallContract( diff --git a/x/evm/keeper/funtoken_from_erc20_test.go b/x/evm/keeper/funtoken_from_erc20_test.go index 3eaf1462c..e6341bb22 100644 --- a/x/evm/keeper/funtoken_from_erc20_test.go +++ b/x/evm/keeper/funtoken_from_erc20_test.go @@ -160,7 +160,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() { s.ErrorContains(err, "either the \"from_erc20\" or \"from_bank_denom\" must be set") } -func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() { +func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() { deps := evmtest.NewTestDeps() s.T().Log("Deploy ERC20") @@ -210,7 +210,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() { randomAcc := testutil.AccAddress() - s.T().Log("send erc20 tokens to cosmos") + s.T().Log("send erc20 tokens to Bank") _, err = deps.EvmKeeper.CallContract( deps.Ctx, embeds.SmartContract_FunToken.ABI, @@ -231,8 +231,8 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() { deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount, ) - s.T().Log("sad: send too many erc20 tokens to cosmos") - _, err = deps.EvmKeeper.CallContract( + s.T().Log("sad: send too many erc20 tokens to Bank") + evmResp, err := deps.EvmKeeper.CallContract( deps.Ctx, embeds.SmartContract_FunToken.ABI, deps.Sender.EthAddr, @@ -243,9 +243,10 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() { big.NewInt(70_000), randomAcc.String(), ) - s.Require().Error(err) + s.T().Log("check balances") + s.Require().Error(err, evmResp.String()) - s.T().Log("send cosmos tokens back to erc20") + s.T().Log("send Bank tokens back to erc20") _, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx), &evm.MsgConvertCoinToEvm{ ToEthAddr: eth.EIP55Addr{ @@ -264,7 +265,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() { deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount.Equal(sdk.NewInt(0)), ) - s.T().Log("sad: send too many cosmos tokens back to erc20") + s.T().Log("sad: send too many Bank tokens back to erc20") _, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx), &evm.MsgConvertCoinToEvm{ ToEthAddr: eth.EIP55Addr{ diff --git a/x/evm/precompile/abci_event.go b/x/evm/precompile/abci_event.go deleted file mode 100644 index beae86a89..000000000 --- a/x/evm/precompile/abci_event.go +++ /dev/null @@ -1,24 +0,0 @@ -package precompile - -import ( - // "github.com/ethereum/go-ethereum/accounts/abi" - gethcommon "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/crypto" - // "github.com/NibiruChain/nibiru/v2/x/evm/statedb" -) - -func MakeTopicFromStr(s string) (topic gethcommon.Hash) { - return MakeTopicFromBytes([]byte(s)) -} - -func MakeTopicFromBytes(bz []byte) (topic gethcommon.Hash) { - hash := crypto.Keccak256Hash(bz) - copy(topic[:], hash[:]) - return topic -} - -// func (p *precompileWasm) foo(stateDB *statedb.StateDB) { -// abi.MakeTopics() -// // stateDB.AddLog() -// // p.evmKeeper.State -// } diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index bcafb0c55..03b08a85c 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -45,6 +45,7 @@ func (p precompileFunToken) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { defer ErrPrecompileRun(err, p)() + res, err := OnRunStart(evm, contract, embeds.SmartContract_FunToken.ABI) if err != nil { return nil, err @@ -61,7 +62,9 @@ func (p precompileFunToken) Run( err = fmt.Errorf("invalid method called with name \"%s\"", method.Name) return } - + if err != nil { + return nil, err + } if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { return nil, err } diff --git a/x/evm/precompile/funtoken_test.go b/x/evm/precompile/funtoken_test.go index 9aad9feb3..62512968f 100644 --- a/x/evm/precompile/funtoken_test.go +++ b/x/evm/precompile/funtoken_test.go @@ -5,7 +5,7 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/ethereum/go-ethereum/common" + gethcommon "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/suite" "github.com/NibiruChain/nibiru/v2/eth" @@ -49,19 +49,19 @@ func (s *FuntokenSuite) TestFailToPackABI() { { name: "wrong type for amount", methodName: string(precompile.FunTokenMethod_BankSend), - callArgs: []any{common.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), "foo", testutil.AccAddress().String()}, + callArgs: []any{gethcommon.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), "foo", testutil.AccAddress().String()}, wantError: "abi: cannot use string as type ptr as argument", }, { name: "wrong type for recipient", methodName: string(precompile.FunTokenMethod_BankSend), - callArgs: []any{common.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), big.NewInt(1), 111}, + callArgs: []any{gethcommon.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), big.NewInt(1), 111}, wantError: "abi: cannot use int as type string as argument", }, { name: "invalid method name", methodName: "foo", - callArgs: []any{common.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), big.NewInt(1), testutil.AccAddress().String()}, + callArgs: []any{gethcommon.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6"), big.NewInt(1), testutil.AccAddress().String()}, wantError: "method 'foo' not found", }, } @@ -126,10 +126,14 @@ func (s *FuntokenSuite) TestHappyPath() { input, err := embeds.SmartContract_FunToken.ABI.Pack(string(precompile.FunTokenMethod_BankSend), callArgs...) s.NoError(err) - _, err = deps.EvmKeeper.CallContractWithInput( - deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_FunToken, true, input, + _, resp, err := evmtest.CallContractTx( + &deps, + precompile.PrecompileAddr_FunToken, + input, + deps.Sender, ) s.Require().NoError(err) + s.Require().Empty(resp.VmError) evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, deps.Sender.EthAddr, big.NewInt(69_000)) evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(0)) diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index ad4484f29..583b60014 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -96,6 +96,9 @@ func (p precompileWasm) Run( err = fmt.Errorf("invalid method called with name \"%s\"", method.Name) return } + if err != nil { + return nil, err + } if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { return nil, err diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index c4a02a950..d8b15328e 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -162,7 +162,7 @@ func (ch PrecompileSnapshotBeforeRun) Revert(s *StateDB) { // TODO: Revert PrecompileSnapshotBeforeRun // Restore the multistore recorded in the journal entry - fmt.Printf("\"Revert was called.\"\n") + fmt.Printf("TODO: UD-DEBUG: PrecompileSnapshotBeforeRun.Revert called\n") s.cacheCtx = s.cacheCtx.WithMultiStore(ch.MultiStore) // Rewrite the `writeCacheCtxFn` using the same logic as sdk.Context.CacheCtx diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index b917845a7..b08eef272 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -523,7 +523,6 @@ func (s *StateDB) SavePrecompileSnapshotToJournal( precompileAddr common.Address, snapshot PrecompileSnapshotBeforeRun, ) error { - fmt.Println("SavePrecompileSnapshotToJournal was called") obj := s.getOrNewStateObject(precompileAddr) obj.db.journal.append(snapshot) s.precompileSnapshotsCount++ From 516aece3157875c6196973c3500ec7822963a425 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Mon, 21 Oct 2024 22:41:34 -0500 Subject: [PATCH 04/12] chore: changelog + small cleanup --- CHANGELOG.md | 40 ++++++++++++++++++++++-------------- x/evm/precompile/funtoken.go | 11 ++++++---- x/evm/precompile/wasm.go | 8 ++++++-- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f3c02c33..acf1100c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,17 +40,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### State Machine Breaking +### Nibiru EVM -#### For next mainnet version +#### Nibiru EVM | Before Audit 2 [Nov, 2024] -- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings. -- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them -- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types -- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines -- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions +The codebase went through a third-party [Code4rena +Zenith](https://code4rena.com/zenith) Audit, running from 2024-10-07 until +2024-11-01 and including both a primary review period and mitigation/remission +period. This section describes code changes that occured after that audit in +preparation for a second audit starting in November 2024. + +- [#2074](https://github.com/NibiruChain/nibiru/pull/2074) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation. The bank metadata for a new FunToken mapping ties a connection between the Bank Coin's `DenomUnit` and the ERC20 contract metadata like the name, decimals, and symbol. This change brings parity between EVM wallets, such as MetaMask, and Interchain wallets like Keplr and Leap. +- [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees): +Use effective gas price in RefundGas and make sure that units are properly +reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059) +and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724). +- [#2086](https://github.com/NibiruChain/nibiru/pull/2086) - fix(evm-precompiles): It is possible for the `Run` function of a custom precompile to retrieve stale state because EVM state changes can occur before the precompile is called that are recorded as entries in the StateDB journal for the transaction without being reflected in the `sdk.Context`. This pull request makes sure that state is committed as expected. -#### Nibiru EVM +#### Nibiru EVM | Before Audit 1 - 2024-10-18 - [#1837](https://github.com/NibiruChain/nibiru/pull/1837) - feat(eth): protos, eth types, and evm module types - [#1838](https://github.com/NibiruChain/nibiru/pull/1838) - feat(eth): Go-ethereum, crypto, encoding, and unit tests for evm/types @@ -70,7 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#1909](https://github.com/NibiruChain/nibiru/pull/1909) - chore(evm): set is_london true by default and removed from config - [#1911](https://github.com/NibiruChain/nibiru/pull/1911) - chore(evm): simplified config by removing old eth forks - [#1912](https://github.com/NibiruChain/nibiru/pull/1912) - test(evm): unit tests for evm_ante -- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code. +- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler - [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code. - [#1922](https://github.com/NibiruChain/nibiru/pull/1922) - feat(evm): tracer option is read from the config. - [#1936](https://github.com/NibiruChain/nibiru/pull/1936) - feat(evm): EVM fungible token protobufs and encoding tests @@ -128,13 +135,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#2060](https://github.com/NibiruChain/nibiru/pull/2060) - fix(evm-precompiles): add assertNumArgs validation - [#2056](https://github.com/NibiruChain/nibiru/pull/2056) - feat(evm): add oracle precompile - [#2065](https://github.com/NibiruChain/nibiru/pull/2065) - refactor(evm)!: Refactor out dead code from the evm.Params -- [#2073](https://github.com/NibiruChain/nibiru/pull/2073) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation -- [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees): -Use effective gas price in RefundGas and make sure that units are properly -reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059) -and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724). -- [#2xxx](https://github.com/NibiruChain/nibiru/pull/2xxx) - fix(evm-precompiles): It is possible for the `Run` function of a custom rpecompile to retrieve stale state because EVM state changes can occur before the precompile is called that are recorded as entries in the StateDB journal for the transaction without being reflected in the `sdk.Context`. This pull request makes sure that state is committed as expected. +### State Machine Breaking (Other) + +#### For next mainnet version + +- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings. +- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them +- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types +- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines +- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions #### Dapp modules: perp, spot, oracle, etc diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 03b08a85c..ee5f82dc5 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -29,9 +29,13 @@ func (p precompileFunToken) Address() gethcommon.Address { return PrecompileAddr_FunToken } +func (p precompileFunToken) ABI() *gethabi.ABI { + return embeds.SmartContract_FunToken.ABI +} + // RequiredGas calculates the cost of calling the precompile in gas units. func (p precompileFunToken) RequiredGas(input []byte) (gasCost uint64) { - return RequiredGas(input, embeds.SmartContract_FunToken.ABI) + return RequiredGas(input, p.ABI()) } const ( @@ -46,12 +50,11 @@ func (p precompileFunToken) Run( ) (bz []byte, err error) { defer ErrPrecompileRun(err, p)() - res, err := OnRunStart(evm, contract, embeds.SmartContract_FunToken.ABI) + res, err := OnRunStart(evm, contract, p.ABI()) if err != nil { return nil, err } - method, args := res.Method, res.Args - ctx := res.Ctx + method, args, ctx := res.Method, res.Args, res.Ctx switch PrecompileMethod(method.Name) { case FunTokenMethod_BankSend: diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index 583b60014..8103056c3 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -63,9 +63,13 @@ func (p precompileWasm) Address() gethcommon.Address { return PrecompileAddr_Wasm } +func (p precompileWasm) ABI() *gethabi.ABI { + return embeds.SmartContract_Wasm.ABI +} + // RequiredGas calculates the cost of calling the precompile in gas units. func (p precompileWasm) RequiredGas(input []byte) (gasCost uint64) { - return RequiredGas(input, embeds.SmartContract_Wasm.ABI) + return RequiredGas(input, p.ABI()) } // Run runs the precompiled contract @@ -73,7 +77,7 @@ func (p precompileWasm) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { defer ErrPrecompileRun(err, p)() - res, err := OnRunStart(evm, contract, embeds.SmartContract_Wasm.ABI) + res, err := OnRunStart(evm, contract, p.ABI()) if err != nil { return nil, err } From 0374dfd04497c7ecc096e9d33420b6b0c7978e7b Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Tue, 22 Oct 2024 00:02:45 -0500 Subject: [PATCH 05/12] docs and code organization --- x/evm/precompile/errors.go | 13 ++ x/evm/precompile/oracle.go | 8 +- x/evm/precompile/oracle_test.go | 6 +- x/evm/precompile/precompile.go | 88 ++++++++++--- x/evm/precompile/wasm.go | 76 +++++------ x/evm/statedb/journal.go | 221 +++++++++++++++++++++----------- x/evm/statedb/statedb.go | 13 +- 7 files changed, 278 insertions(+), 147 deletions(-) diff --git a/x/evm/precompile/errors.go b/x/evm/precompile/errors.go index f22ed9f7e..1c7dabed5 100644 --- a/x/evm/precompile/errors.go +++ b/x/evm/precompile/errors.go @@ -3,11 +3,24 @@ package precompile import ( "errors" "fmt" + "reflect" gethabi "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/core/vm" ) +// ErrPrecompileRun is error function intended for use in a `defer` pattern, +// which modifies the input error in the event that its value becomes non-nil. +// This creates a concise way to prepend extra information to the original error. +func ErrPrecompileRun(err error, p vm.PrecompiledContract) func() { + return func() { + if err != nil { + precompileType := reflect.TypeOf(p).Name() + err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) + } + } +} + // Error short-hand for type validation func ErrArgTypeValidation(solidityHint string, arg any) error { return fmt.Errorf("type validation failed for (%s) argument: %s", solidityHint, arg) diff --git a/x/evm/precompile/oracle.go b/x/evm/precompile/oracle.go index 568c0d4fb..65212ce11 100644 --- a/x/evm/precompile/oracle.go +++ b/x/evm/precompile/oracle.go @@ -28,11 +28,9 @@ func (p precompileOracle) RequiredGas(input []byte) (gasPrice uint64) { } const ( - OracleMethod_QueryExchangeRate OracleMethod = "queryExchangeRate" + OracleMethod_queryExchangeRate PrecompileMethod = "queryExchangeRate" ) -type OracleMethod string - // Run runs the precompiled contract func (p precompileOracle) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, @@ -44,8 +42,8 @@ func (p precompileOracle) Run( } method, args, ctx := res.Method, res.Args, res.Ctx - switch OracleMethod(method.Name) { - case OracleMethod_QueryExchangeRate: + switch PrecompileMethod(method.Name) { + case OracleMethod_queryExchangeRate: bz, err = p.queryExchangeRate(ctx, method, args, readonly) default: err = fmt.Errorf("invalid method called with name \"%s\"", method.Name) diff --git a/x/evm/precompile/oracle_test.go b/x/evm/precompile/oracle_test.go index 4d8d0116e..0752016be 100644 --- a/x/evm/precompile/oracle_test.go +++ b/x/evm/precompile/oracle_test.go @@ -21,13 +21,13 @@ func (s *OracleSuite) TestOracle_FailToPackABI() { }{ { name: "wrong amount of call args", - methodName: string(precompile.OracleMethod_QueryExchangeRate), + methodName: string(precompile.OracleMethod_queryExchangeRate), callArgs: []any{"nonsense", "args here", "to see if", "precompile is", "called"}, wantError: "argument count mismatch: got 5 for 1", }, { name: "wrong type for pair", - methodName: string(precompile.OracleMethod_QueryExchangeRate), + methodName: string(precompile.OracleMethod_queryExchangeRate), callArgs: []any{common.HexToAddress("0x7D4B7B8CA7E1a24928Bb96D59249c7a5bd1DfBe6")}, wantError: "abi: cannot use array as type string as argument", }, @@ -64,7 +64,7 @@ func (s *OracleSuite) TestOracle_HappyPath() { s.NoError(err) // Check the response - out, err := embeds.SmartContract_Oracle.ABI.Unpack(string(precompile.OracleMethod_QueryExchangeRate), resp.Ret) + out, err := embeds.SmartContract_Oracle.ABI.Unpack(string(precompile.OracleMethod_queryExchangeRate), resp.Ret) s.NoError(err) // Check the response diff --git a/x/evm/precompile/precompile.go b/x/evm/precompile/precompile.go index 7153dd575..43cc4e0f0 100644 --- a/x/evm/precompile/precompile.go +++ b/x/evm/precompile/precompile.go @@ -16,7 +16,6 @@ package precompile import ( "bytes" "fmt" - "reflect" "github.com/NibiruChain/collections" store "github.com/cosmos/cosmos-sdk/store/types" @@ -135,29 +134,58 @@ func RequiredGas(input []byte, abi *gethabi.ABI) uint64 { return (costPerByte * argsBzLen) + costFlat } -// This is a `defer` pattern to add behavior that runs in the case that the error is -// non-nil, creating a concise way to add extra information. -func ErrPrecompileRun(err error, p vm.PrecompiledContract) func() { - return func() { - if err != nil { - precompileType := reflect.TypeOf(p).Name() - err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) - } - } -} +type OnRunStartResult struct { + // Args contains the decoded (ABI unpacked) arguments passed to the contract + // as input. + Args []any + + // Ctx is a cached SDK context that allows isolated state + // operations to occur that can be reverted by the EVM's [statedb.StateDB]. + Ctx sdk.Context -type PrecompileStartResult struct { - Args []any - Ctx sdk.Context - WriteCtx func() - Method *gethabi.Method + // WriteCtx commits the cached context changes to the parent context. + WriteCtx func() + + // Method is the ABI method for the precompiled contract call. + Method *gethabi.Method + + // SnapshotBeforeRun captures the state before precompile execution to enable + // proper state reversal if the call fails or if [statedb.JournalChange] + // is reverted in general. SnapshotBeforeRun statedb.PrecompileSnapshotBeforeRun StateDB *statedb.StateDB } +// OnRunStart prepares the execution environment for a precompiled contract call. +// It handles decoding the input data according the to contract ABI, creates an +// isolated cache context for state changes, and sets up a snapshot for potential +// EVM "reverts". +// +// Args: +// - evm: Instance of the EVM executing the contract +// - contract: Precompiled contract being called +// - abi: [gethabi.ABI] defining the contract's invokable methods. +// +// Example Usage: +// +// ```go +// func (p newPrecompile) Run( +// evm *vm.EVM, contract *vm.Contract, readonly bool +// ) (bz []byte, err error) { +// res, err := OnRunStart(evm, contract, p.ABI()) +// if err != nil { +// return nil, err +// } +// // ... +// // Use res.Ctx for state changes +// // Use res.WriteCtx to commit changes to the multistore +// // after successful execution +// res.WriteCtx() +// } +// ``` func OnRunStart( evm *vm.EVM, contract *vm.Contract, abi *gethabi.ABI, -) (res PrecompileStartResult, err error) { +) (res OnRunStartResult, err error) { method, args, err := DecomposeInput(abi, contract.Input) if err != nil { return res, err @@ -173,7 +201,7 @@ func OnRunStart( return res, fmt.Errorf("error committing dirty journal entries for the precompile call to the cache ctx: %w", err) } - return PrecompileStartResult{ + return OnRunStartResult{ Args: args, Ctx: cacheCtx, WriteCtx: writeCacheCtx, @@ -183,6 +211,18 @@ func OnRunStart( }, nil } +// OnRunEnd finalizes a precompile execution by saving its state snapshot to the +// journal. This ensures that any state changes can be properly reverted if needed. +// +// Args: +// - stateDB: The EVM state database +// - snapshot: The state snapshot taken before the precompile executed +// - precompileAddr: The address of the precompiled contract +// +// The snapshot is critical for maintaining state consistency when: +// - The operation gets reverted ([statedb.JournalChange] Revert). +// - The precompile modifies state in other modules (e.g., bank, wasm) +// - Multiple precompiles are called within a single transaction func OnRunEnd( stateDB *statedb.StateDB, snapshot statedb.PrecompileSnapshotBeforeRun, @@ -190,3 +230,15 @@ func OnRunEnd( ) error { return stateDB.SavePrecompileSnapshotToJournal(precompileAddr, snapshot) } + +var precompileMethodIsTxMap map[PrecompileMethod]bool = map[PrecompileMethod]bool{ + WasmMethod_execute: true, + WasmMethod_instantiate: true, + WasmMethod_executeMulti: true, + WasmMethod_query: false, + WasmMethod_queryRaw: false, + + FunTokenMethod_BankSend: true, + + OracleMethod_queryExchangeRate: false, +} diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index 8103056c3..3a88503d4 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -29,49 +29,6 @@ const ( WasmMethod_queryRaw PrecompileMethod = "queryRaw" ) -var precompileMethodIsTxMap map[PrecompileMethod]bool = map[PrecompileMethod]bool{ - WasmMethod_execute: true, - WasmMethod_instantiate: true, - WasmMethod_executeMulti: true, - WasmMethod_query: false, - WasmMethod_queryRaw: false, - - FunTokenMethod_BankSend: true, -} - -// Wasm: A struct embedding keepers for read and write operations in Wasm, such -// as execute, query, and instantiate. -type Wasm struct { - *wasmkeeper.PermissionedKeeper - wasmkeeper.Keeper -} - -func PrecompileWasm(keepers keepers.PublicKeepers) vm.PrecompiledContract { - return precompileWasm{ - Wasm: Wasm{ - wasmkeeper.NewDefaultPermissionKeeper(keepers.WasmKeeper), - keepers.WasmKeeper, - }, - } -} - -type precompileWasm struct { - Wasm Wasm -} - -func (p precompileWasm) Address() gethcommon.Address { - return PrecompileAddr_Wasm -} - -func (p precompileWasm) ABI() *gethabi.ABI { - return embeds.SmartContract_Wasm.ABI -} - -// RequiredGas calculates the cost of calling the precompile in gas units. -func (p precompileWasm) RequiredGas(input []byte) (gasCost uint64) { - return RequiredGas(input, p.ABI()) -} - // Run runs the precompiled contract func (p precompileWasm) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, @@ -111,6 +68,39 @@ func (p precompileWasm) Run( return } +type precompileWasm struct { + Wasm Wasm +} + +func (p precompileWasm) Address() gethcommon.Address { + return PrecompileAddr_Wasm +} + +func (p precompileWasm) ABI() *gethabi.ABI { + return embeds.SmartContract_Wasm.ABI +} + +// RequiredGas calculates the cost of calling the precompile in gas units. +func (p precompileWasm) RequiredGas(input []byte) (gasCost uint64) { + return RequiredGas(input, p.ABI()) +} + +// Wasm: A struct embedding keepers for read and write operations in Wasm, such +// as execute, query, and instantiate. +type Wasm struct { + *wasmkeeper.PermissionedKeeper + wasmkeeper.Keeper +} + +func PrecompileWasm(keepers keepers.PublicKeepers) vm.PrecompiledContract { + return precompileWasm{ + Wasm: Wasm{ + wasmkeeper.NewDefaultPermissionKeeper(keepers.WasmKeeper), + keepers.WasmKeeper, + }, + } +} + // execute invokes a Wasm contract's "ExecuteMsg", which corresponds to // "wasm/types/MsgExecuteContract". This enables arbitrary smart contract // execution using the Wasm VM from the EVM. diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index d8b15328e..ab72839c6 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -18,7 +18,6 @@ package statedb import ( "bytes" - "fmt" "math/big" "sort" @@ -27,8 +26,8 @@ import ( "github.com/ethereum/go-ethereum/common" ) -// JournalChange is a modification entry in the state change journal that can be -// Reverted on demand. +// JournalChange, also called a "journal entry", is a modification entry in the +// state change journal that can be reverted on demand. type JournalChange interface { // Revert undoes the changes introduced by this journal entry. Revert(*StateDB) @@ -94,76 +93,32 @@ func (j *journal) length() int { return len(j.entries) } -type ( - // createObjectChange: [JournalChange] implementation for when - // a new account (called an "object" in this context) is created in state. - createObjectChange struct { - account *common.Address - } - // resetObjectChange: [JournalChange] implementation for when - // a backup for a state object is needed in order to revert state changes - // that overwrite an existing object. - resetObjectChange struct { - prev *stateObject - } - suicideChange struct { - account *common.Address - prev bool // whether account had already suicided - prevbalance *big.Int - } - - // balanceChange: [JournalChange] for an update to an account's wei balance - balanceChange struct { - account *common.Address - prevWei *big.Int - } - // nonceChange: [JournalChange] for an update to an account's nonce. - nonceChange struct { - account *common.Address - prev uint64 - } - storageChange struct { - account *common.Address - key, prevalue common.Hash - } - codeChange struct { - account *common.Address - prevcode, prevhash []byte - } - - // Changes to other state values. - refundChange struct { - prev uint64 - } - addLogChange struct{} - - // Changes to the access list - accessListAddAccountChange struct { - address *common.Address - } - // accessListAddSlotChange: [JournalChange] implementations for - accessListAddSlotChange struct { - address *common.Address - slot *common.Hash - } -) - // ------------------------------------------------------ // PrecompileSnapshotBeforeRun -var _ JournalChange = PrecompileSnapshotBeforeRun{} - +// PrecompileSnapshotBeforeRun: Precompiles can alter persistent storage of other +// modules. These changes to persistent storage are not reverted by a `Revert` of +// [JournalChange] by default, as it generally manages only changes to accounts +// and Bank balances for ether (NIBI). +// +// As a workaround to make state changes from precompiles reversible, we store +// [PrecompileSnapshotBeforeRun] snapshots that sync and record the prior state +// of the other modules, allowing precompile calls to truly be reverted. +// +// As a simple example, suppose that a transaction calls a precompile. +// 1. If the precompile changes the state in the Bank Module or Wasm module +// 2. The call gets reverted (`revert()` in Solidity), which shoud restore the +// state to a in-memory snapshot recorded on the StateDB journal. +// 3. This could cause a problem where changes to the rest of the blockchain state +// are still in effect following the reversion in the EVM state DB. type PrecompileSnapshotBeforeRun struct { MultiStore store.CacheMultiStore Events sdk.Events } -func (ch PrecompileSnapshotBeforeRun) Revert(s *StateDB) { - // TODO: Revert PrecompileSnapshotBeforeRun - // Restore the multistore recorded in the journal entry - - fmt.Printf("TODO: UD-DEBUG: PrecompileSnapshotBeforeRun.Revert called\n") +var _ JournalChange = PrecompileSnapshotBeforeRun{} +func (ch PrecompileSnapshotBeforeRun) Revert(s *StateDB) { s.cacheCtx = s.cacheCtx.WithMultiStore(ch.MultiStore) // Rewrite the `writeCacheCtxFn` using the same logic as sdk.Context.CacheCtx s.writeCacheCtxFn = func() { @@ -176,19 +131,16 @@ func (ch PrecompileSnapshotBeforeRun) Dirtied() *common.Address { return nil } -var ( - _ JournalChange = accessListAddSlotChange{} - _ JournalChange = createObjectChange{} - _ JournalChange = resetObjectChange{} - _ JournalChange = suicideChange{} - _ JournalChange = balanceChange{} - _ JournalChange = nonceChange{} - _ JournalChange = storageChange{} - _ JournalChange = codeChange{} - _ JournalChange = refundChange{} - _ JournalChange = addLogChange{} - _ JournalChange = accessListAddAccountChange{} -) +// ------------------------------------------------------ +// createObjectChange + +// createObjectChange: [JournalChange] implementation for when +// a new account (called an "object" in this context) is created in state. +type createObjectChange struct { + account *common.Address +} + +var _ JournalChange = createObjectChange{} func (ch createObjectChange) Revert(s *StateDB) { delete(s.stateObjects, *ch.account) @@ -198,6 +150,18 @@ func (ch createObjectChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// resetObjectChange + +// resetObjectChange: [JournalChange] for an account that needs its +// original state reset. This is used when an account's state is being replaced +// and we need to revert to the previous version. +type resetObjectChange struct { + prev *stateObject +} + +var _ JournalChange = resetObjectChange{} + func (ch resetObjectChange) Revert(s *StateDB) { s.setStateObject(ch.prev) } @@ -206,6 +170,17 @@ func (ch resetObjectChange) Dirtied() *common.Address { return nil } +// ------------------------------------------------------ +// suicideChange + +type suicideChange struct { + account *common.Address + prev bool // whether account had already suicided + prevbalance *big.Int +} + +var _ JournalChange = suicideChange{} + func (ch suicideChange) Revert(s *StateDB) { obj := s.getStateObject(*ch.account) if obj != nil { @@ -218,6 +193,17 @@ func (ch suicideChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// balanceChange + +// balanceChange: [JournalChange] for an update to the wei balance of an account. +type balanceChange struct { + account *common.Address + prevWei *big.Int +} + +var _ JournalChange = balanceChange{} + func (ch balanceChange) Revert(s *StateDB) { s.getStateObject(*ch.account).setBalance(ch.prevWei) } @@ -226,6 +212,18 @@ func (ch balanceChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// nonceChange + +// nonceChange: [JournalChange] for an update to the nonce of an account. +// The nonce is a counter of the number of transactions sent from an account. +type nonceChange struct { + account *common.Address + prev uint64 +} + +var _ JournalChange = nonceChange{} + func (ch nonceChange) Revert(s *StateDB) { s.getStateObject(*ch.account).setNonce(ch.prev) } @@ -234,6 +232,19 @@ func (ch nonceChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// codeChange + +// codeChange: [JournalChange] for an update to an account's code (smart contract +// bytecode). The previous code and hash for the code are stored to enable +// reversion. +type codeChange struct { + account *common.Address + prevcode, prevhash []byte +} + +var _ JournalChange = codeChange{} + func (ch codeChange) Revert(s *StateDB) { s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode) } @@ -242,6 +253,18 @@ func (ch codeChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// storageChange + +// storageChange: [JournalChange] for the modification of a single key and value +// within a contract's storage. +type storageChange struct { + account *common.Address + key, prevalue common.Hash +} + +var _ JournalChange = storageChange{} + func (ch storageChange) Revert(s *StateDB) { s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } @@ -250,6 +273,17 @@ func (ch storageChange) Dirtied() *common.Address { return ch.account } +// ------------------------------------------------------ +// refundChange + +// refundChange: [JournalChange] for the global gas refund counter. +// This tracks changes to the gas refund value during contract execution. +type refundChange struct { + prev uint64 +} + +var _ JournalChange = refundChange{} + func (ch refundChange) Revert(s *StateDB) { s.refund = ch.prev } @@ -258,6 +292,15 @@ func (ch refundChange) Dirtied() *common.Address { return nil } +// ------------------------------------------------------ +// addLogChange + +// addLogChange represents [JournalChange] for a new log addition. +// When reverted, it removes the last log from the accumulated logs list. +type addLogChange struct{} + +var _ JournalChange = addLogChange{} + func (ch addLogChange) Revert(s *StateDB) { s.logs = s.logs[:len(s.logs)-1] } @@ -266,6 +309,16 @@ func (ch addLogChange) Dirtied() *common.Address { return nil } +// ------------------------------------------------------ +// accessListAddAccountChange + +// accessListAddAccountChange represents [JournalChange] for when an address +// is added to the access list. Access lists track warm storage slots for +// gas cost calculations. +type accessListAddAccountChange struct { + address *common.Address +} + // When an (address, slot) combination is added, it always results in two // journal entries if the address is not already present: // 1. `accessListAddAccountChange`: a journal change for the address @@ -282,6 +335,20 @@ func (ch accessListAddAccountChange) Dirtied() *common.Address { return nil } +// ------------------------------------------------------ +// accessListAddSlotChange + +// accessListAddSlotChange: [JournalChange] implementations for +type accessListAddSlotChange struct { + address *common.Address + slot *common.Hash +} + +// accessListAddSlotChange represents a [JournalChange] for when a storage slot +// is added to an address's access list entry. This tracks individual storage +// slots that have been accessed. +var _ JournalChange = accessListAddSlotChange{} + func (ch accessListAddSlotChange) Revert(s *StateDB) { s.accessList.DeleteSlot(*ch.address, *ch.slot) } diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index b08eef272..5ae1586a6 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -32,15 +32,18 @@ type StateDB struct { keeper Keeper // ctx is the persistent context used for official `StateDB.Commit` calls. ctx sdk.Context + // cacheCtx: An sdk.Context produced from the [StateDB.ctx] with the // multi-store cached and a new event manager. The cached context // (`cacheCtx`) is written to the persistent context (`ctx`) when // `writeCacheCtx` is called. cacheCtx sdk.Context + // Events are automatically emitted on the parent context's // EventManager when the caller executes the [writeCacheCtxFn]. writeCacheCtxFn func() - // THe number of precompiled contract calls within the current transaction + + // The number of precompiled contract calls within the current transaction precompileSnapshotsCount uint8 // Journal of state modifications. This is the backbone of @@ -519,6 +522,14 @@ func (s *StateDB) CacheCtxForPrecompile() ( } } +// SavePrecompileSnapshotToJournal adds a snapshot of the commit multistore +// ([PrecompileSnapshotBeforeRun]) to the [StateDB] journal at the end of +// successful invocation of a precompiled contract. This is necessary to revert +// intermediate states where an EVM contract augments the multistore with a +// precompile and an inconsistency occurs between the EVM module and other +// modules. +// +// See [PrecompileSnapshotBeforeRun] for more info. func (s *StateDB) SavePrecompileSnapshotToJournal( precompileAddr common.Address, snapshot PrecompileSnapshotBeforeRun, From 8d7fcd370a909b427741e53a01d5256d199b80d0 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Tue, 22 Oct 2024 01:13:39 -0500 Subject: [PATCH 06/12] fix defer sage with ErrPrecompileRun --- x/evm/precompile/errors.go | 11 +++++------ x/evm/precompile/funtoken.go | 4 +++- x/evm/precompile/oracle.go | 4 +++- x/evm/precompile/wasm.go | 4 +++- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/x/evm/precompile/errors.go b/x/evm/precompile/errors.go index 1c7dabed5..a95989b71 100644 --- a/x/evm/precompile/errors.go +++ b/x/evm/precompile/errors.go @@ -12,13 +12,12 @@ import ( // ErrPrecompileRun is error function intended for use in a `defer` pattern, // which modifies the input error in the event that its value becomes non-nil. // This creates a concise way to prepend extra information to the original error. -func ErrPrecompileRun(err error, p vm.PrecompiledContract) func() { - return func() { - if err != nil { - precompileType := reflect.TypeOf(p).Name() - err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) - } +func ErrPrecompileRun(err error, p vm.PrecompiledContract) error { + if err != nil { + precompileType := reflect.TypeOf(p).Name() + err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err) } + return err } // Error short-hand for type validation diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index ee5f82dc5..2c7d13924 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -48,7 +48,9 @@ type PrecompileMethod string func (p precompileFunToken) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - defer ErrPrecompileRun(err, p)() + defer func() { + err = ErrPrecompileRun(err, p) + }() res, err := OnRunStart(evm, contract, p.ABI()) if err != nil { diff --git a/x/evm/precompile/oracle.go b/x/evm/precompile/oracle.go index 65212ce11..59a375707 100644 --- a/x/evm/precompile/oracle.go +++ b/x/evm/precompile/oracle.go @@ -35,7 +35,9 @@ const ( func (p precompileOracle) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - defer ErrPrecompileRun(err, p)() + defer func() { + err = ErrPrecompileRun(err, p) + }() res, err := OnRunStart(evm, contract, embeds.SmartContract_Oracle.ABI) if err != nil { return nil, err diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index 3a88503d4..bba973bb1 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -33,7 +33,9 @@ const ( func (p precompileWasm) Run( evm *vm.EVM, contract *vm.Contract, readonly bool, ) (bz []byte, err error) { - defer ErrPrecompileRun(err, p)() + defer func() { + err = ErrPrecompileRun(err, p) + }() res, err := OnRunStart(evm, contract, p.ABI()) if err != nil { return nil, err From eb87c3ea041c1262d9d6fedd20f82eafc504da95 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Tue, 22 Oct 2024 03:23:14 -0500 Subject: [PATCH 07/12] a couple more tests --- x/evm/errors.go | 12 ++++- x/evm/evmtest/erc20.go | 6 +++ x/evm/evmtest/tx.go | 20 +++----- x/evm/evmtest/tx_test.go | 90 +++++++++++++++++++++++++++++++++ x/evm/keeper/grpc_query.go | 17 +++---- x/evm/keeper/grpc_query_test.go | 4 +- 6 files changed, 124 insertions(+), 25 deletions(-) create mode 100644 x/evm/evmtest/tx_test.go diff --git a/x/evm/errors.go b/x/evm/errors.go index a32facdcb..9fc6722ac 100644 --- a/x/evm/errors.go +++ b/x/evm/errors.go @@ -74,9 +74,17 @@ var ( func NewExecErrorWithReason(revertReason []byte) *RevertError { result := common.CopyBytes(revertReason) reason, errUnpack := abi.UnpackRevert(result) - err := errors.New("execution reverted") + + var err error + errPrefix := "execution reverted" if errUnpack == nil { - err = fmt.Errorf("execution reverted: %v", reason) + reasonStr := reason + err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr) + } else if string(result) != "" { + reasonStr := string(result) + err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr) + } else { + err = errors.New(errPrefix) } return &RevertError{ error: err, diff --git a/x/evm/evmtest/erc20.go b/x/evm/evmtest/erc20.go index 773005661..d8798f71d 100644 --- a/x/evm/evmtest/erc20.go +++ b/x/evm/evmtest/erc20.go @@ -99,3 +99,9 @@ func AssertBankBalanceEqual( actualBalance := deps.App.BankKeeper.GetBalance(deps.Ctx, bech32Addr, denom).Amount.BigInt() assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance) } + +// BigPow multiplies "amount" by 10 to the "pow10Exp". +func BigPow(amount *big.Int, pow10Exp uint8) (powAmount *big.Int) { + pow10 := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(pow10Exp)), nil) + return new(big.Int).Mul(amount, pow10) +} diff --git a/x/evm/evmtest/tx.go b/x/evm/evmtest/tx.go index 01e9a97ed..dde679851 100644 --- a/x/evm/evmtest/tx.go +++ b/x/evm/evmtest/tx.go @@ -24,7 +24,6 @@ import ( "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" - "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) type GethTxType = uint8 @@ -191,7 +190,11 @@ func DeployContract( // DeployAndExecuteERC20Transfer deploys contract, executes transfer and returns tx hash func DeployAndExecuteERC20Transfer( deps *TestDeps, t *testing.T, -) (erc20Transfer *evm.MsgEthereumTx, predecessors []*evm.MsgEthereumTx) { +) ( + erc20Transfer *evm.MsgEthereumTx, + predecessors []*evm.MsgEthereumTx, + contractAddr gethcommon.Address, +) { // TX 1: Deploy ERC-20 contract deployResp, err := DeployContract(deps, embeds.SmartContract_TestERC20) require.NoError(t, err) @@ -199,7 +202,7 @@ func DeployAndExecuteERC20Transfer( nonce := deployResp.Nonce // Contract address is deterministic - contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce) + contractAddr = crypto.CreateAddress(deps.Sender.EthAddr, nonce) deps.App.Commit() predecessors = []*evm.MsgEthereumTx{ deployResp.EthTxMsg, @@ -213,7 +216,7 @@ func DeployAndExecuteERC20Transfer( nonce = deps.StateDB().GetNonce(deps.Sender.EthAddr) txArgs := evm.JsonTxArgs{ From: &deps.Sender.EthAddr, - To: &contractAddress, + To: &contractAddr, Nonce: (*hexutil.Uint64)(&nonce), Data: (*hexutil.Bytes)(&input), } @@ -226,7 +229,7 @@ func DeployAndExecuteERC20Transfer( require.NoError(t, err) require.Empty(t, resp.VmError) - return erc20Transfer, predecessors + return erc20Transfer, predecessors, contractAddr } func CallContractTx( @@ -247,13 +250,6 @@ func CallContractTx( return } - txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash)) - stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig) - err = stateDB.Commit() - if err != nil { - return - } - err = ethTxMsg.Sign(gethSigner, krSigner) if err != nil { err = fmt.Errorf("CallContract error during signature: %w", err) diff --git a/x/evm/evmtest/tx_test.go b/x/evm/evmtest/tx_test.go new file mode 100644 index 000000000..e9cc956c1 --- /dev/null +++ b/x/evm/evmtest/tx_test.go @@ -0,0 +1,90 @@ +// Copyright (c) 2023-2024 Nibi, Inc. +package evmtest_test + +import ( + "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/crypto" + + "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" + "github.com/NibiruChain/nibiru/v2/x/evm" + "github.com/NibiruChain/nibiru/v2/x/evm/embeds" + "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" +) + +func (s *Suite) TestCallContractTx() { + deps := evmtest.NewTestDeps() + + s.T().Log("Deploy some ERC20") + deployArgs := []any{"name", "SYMBOL", uint8(18)} + deployResp, err := evmtest.DeployContract( + &deps, + embeds.SmartContract_ERC20Minter, + deployArgs..., + ) + s.Require().NoError(err, deployResp) + contractAddr := crypto.CreateAddress(deps.Sender.EthAddr, deployResp.Nonce) + gotContractAddr := deployResp.ContractAddr + s.Require().Equal(contractAddr, gotContractAddr) + + s.T().Log("expect zero balance") + { + wantBal := big.NewInt(0) + evmtest.AssertERC20BalanceEqual( + s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal, + ) + } + + abi := deployResp.ContractData.ABI + s.T().Log("mint some tokens") + { + amount := big.NewInt(69_420) + to := deps.Sender.EthAddr + callArgs := []any{to, amount} + input, err := abi.Pack( + "mint", callArgs..., + ) + s.Require().NoError(err) + _, resp, err := evmtest.CallContractTx( + &deps, + contractAddr, + input, + deps.Sender, + ) + s.Require().NoError(err) + s.Require().Empty(resp.VmError) + } + + s.T().Log("expect nonzero balance") + { + wantBal := big.NewInt(69_420) + evmtest.AssertERC20BalanceEqual( + s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal, + ) + } +} + +func (s *Suite) TestTransferWei() { + deps := evmtest.NewTestDeps() + + s.Require().NoError(testapp.FundAccount( + deps.App.BankKeeper, + deps.Ctx, + deps.Sender.NibiruAddr, + sdk.NewCoins(sdk.NewCoin(evm.EVMBankDenom, sdk.NewInt(69_420))), + )) + + randomAcc := evmtest.NewEthPrivAcc() + to := randomAcc.EthAddr + err := evmtest.TransferWei(&deps, to, evm.NativeToWei(big.NewInt(420))) + s.Require().NoError(err) + + evmtest.AssertBankBalanceEqual( + s.T(), deps, evm.EVMBankDenom, deps.Sender.EthAddr, big.NewInt(69_000), + ) + + s.Run("DeployAndExecuteERC20Transfer", func() { + evmtest.DeployAndExecuteERC20Transfer(&deps, s.T()) + }) +} diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index 1ae3c19be..1b2ed1c4d 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -663,15 +663,14 @@ func (k Keeper) TraceBlock( contextHeight = 1 } - ctx := sdk.UnwrapSDKContext(goCtx) - ctx = ctx.WithBlockHeight(contextHeight) - ctx = ctx.WithBlockTime(req.BlockTime) - ctx = ctx.WithHeaderHash(gethcommon.Hex2Bytes(req.BlockHash)) - - // to get the base fee we only need the block max gas in the consensus params - ctx = ctx.WithConsensusParams(&cmtproto.ConsensusParams{ - Block: &cmtproto.BlockParams{MaxGas: req.BlockMaxGas}, - }) + ctx := sdk.UnwrapSDKContext(goCtx). + WithBlockHeight(contextHeight). + WithBlockTime(req.BlockTime). + WithHeaderHash(gethcommon.Hex2Bytes(req.BlockHash)). + // to get the base fee we only need the block max gas in the consensus params + WithConsensusParams(&cmtproto.ConsensusParams{ + Block: &cmtproto.BlockParams{MaxGas: req.BlockMaxGas}, + }) chainID := k.EthChainID(ctx) diff --git a/x/evm/keeper/grpc_query_test.go b/x/evm/keeper/grpc_query_test.go index 46ce456d2..fa1d9ca2b 100644 --- a/x/evm/keeper/grpc_query_test.go +++ b/x/evm/keeper/grpc_query_test.go @@ -791,7 +791,7 @@ func (s *Suite) TestTraceTx() { { name: "happy: trace erc-20 transfer tx", scenario: func(deps *evmtest.TestDeps) (req In, wantResp Out) { - txMsg, predecessors := evmtest.DeployAndExecuteERC20Transfer(deps, s.T()) + txMsg, predecessors, _ := evmtest.DeployAndExecuteERC20Transfer(deps, s.T()) req = &evm.QueryTraceTxRequest{ Msg: txMsg, @@ -870,7 +870,7 @@ func (s *Suite) TestTraceBlock() { name: "happy: trace erc-20 transfer tx", setup: nil, scenario: func(deps *evmtest.TestDeps) (req In, wantResp Out) { - txMsg, _ := evmtest.DeployAndExecuteERC20Transfer(deps, s.T()) + txMsg, _, _ := evmtest.DeployAndExecuteERC20Transfer(deps, s.T()) req = &evm.QueryTraceBlockRequest{ Txs: []*evm.MsgEthereumTx{ txMsg, From 7cc070baf158fb7c9041946335f4d9b6834db263 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Wed, 23 Oct 2024 13:32:55 -0500 Subject: [PATCH 08/12] revert cacheCtx since Revert fails txs automatically --- x/evm/keeper/erc20.go | 30 +-- x/evm/keeper/funtoken_from_coin.go | 2 +- x/evm/keeper/grpc_query.go | 8 +- x/evm/keeper/msg_server.go | 49 ++--- x/evm/precompile/funtoken.go | 19 +- x/evm/precompile/funtoken_test.go | 2 +- x/evm/precompile/oracle_test.go | 2 +- x/evm/precompile/precompile.go | 50 +---- x/evm/precompile/test/export.go | 316 +++++++++++++++++++++++++++++ x/evm/precompile/wasm.go | 46 ++--- x/evm/precompile/wasm_test.go | 311 ++-------------------------- x/evm/statedb/journal.go | 44 +--- x/evm/statedb/journal_test.go | 88 ++++++++ x/evm/statedb/state_object.go | 8 +- x/evm/statedb/statedb.go | 83 ++------ 15 files changed, 527 insertions(+), 531 deletions(-) create mode 100644 x/evm/precompile/test/export.go create mode 100644 x/evm/statedb/journal_test.go diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index 4196bc683..280630e82 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -12,6 +12,7 @@ import ( gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" gethcore "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" serverconfig "github.com/NibiruChain/nibiru/v2/app/server/config" "github.com/NibiruChain/nibiru/v2/x/evm" @@ -57,7 +58,8 @@ func (e erc20Calls) Mint( if err != nil { return nil, fmt.Errorf("failed to pack ABI args: %w", err) } - return e.CallContractWithInput(ctx, from, &contract, true, input) + evmResp, _, err = e.CallContractWithInput(ctx, from, &contract, true, input) + return } /* @@ -78,7 +80,7 @@ func (e erc20Calls) Transfer( if err != nil { return false, fmt.Errorf("failed to pack ABI args: %w", err) } - resp, err := e.CallContractWithInput(ctx, from, &contract, true, input) + resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input) if err != nil { return false, err } @@ -118,7 +120,8 @@ func (e erc20Calls) Burn( return } commit := true - return e.CallContractWithInput(ctx, from, &contract, commit, input) + evmResp, _, err = e.CallContractWithInput(ctx, from, &contract, commit, input) + return } // CallContract invokes a smart contract on the method specified by [methodName] @@ -149,7 +152,8 @@ func (k Keeper) CallContract( if err != nil { return nil, fmt.Errorf("failed to pack ABI args: %w", err) } - return k.CallContractWithInput(ctx, fromAcc, contract, commit, contractInput) + evmResp, _, err = k.CallContractWithInput(ctx, fromAcc, contract, commit, contractInput) + return evmResp, err } // CallContractWithInput invokes a smart contract with the given [contractInput]. @@ -170,7 +174,7 @@ func (k Keeper) CallContractWithInput( contract *gethcommon.Address, commit bool, contractInput []byte, -) (evmResp *evm.MsgEthereumTxResponse, err error) { +) (evmResp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) { // This is a `defer` pattern to add behavior that runs in the case that the error is // non-nil, creating a concise way to add extra information. defer func() { @@ -185,7 +189,7 @@ func (k Keeper) CallContractWithInput( commit, gasLimit, &fromAcc, contract, contractInput, k, ctx, ) if err != nil { - return nil, err + return } unusedBigInt := big.NewInt(0) @@ -210,26 +214,28 @@ func (k Keeper) CallContractWithInput( k.EthChainID(ctx), ) if err != nil { - return nil, errors.Wrapf(err, "failed to load evm config") + err = errors.Wrapf(err, "failed to load evm config") + return } blockHash := gethcommon.BytesToHash(ctx.HeaderHash()) txConfig := statedb.NewEmptyTxConfig(blockHash) txConfig.TxIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) txConfig.LogIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) - - evmResp, err = k.ApplyEvmMsg( + evmResp, evmObj, err = k.ApplyEvmMsg( ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, ) if err != nil { - return nil, errors.Wrapf(err, "failed to apply EVM message") + err = errors.Wrapf(err, "failed to apply EVM message") + return } if evmResp.Failed() { - return nil, errors.Wrapf(err, "EVM execution failed: %s", evmResp.VmError) + err = errors.Wrapf(err, "EVM execution failed: %s", evmResp.VmError) + return } - return evmResp, err + return evmResp, evmObj, err } // computeCommitGasLimit: If the transition is meant to mutate state, this diff --git a/x/evm/keeper/funtoken_from_coin.go b/x/evm/keeper/funtoken_from_coin.go index 82d7017f6..6f0f2efd0 100644 --- a/x/evm/keeper/funtoken_from_coin.go +++ b/x/evm/keeper/funtoken_from_coin.go @@ -78,7 +78,7 @@ func (k *Keeper) deployERC20ForBankCoin( bytecodeForCall := append(embeds.SmartContract_ERC20Minter.Bytecode, packedArgs...) // nil address for contract creation - _, err = k.CallContractWithInput( + _, _, err = k.CallContractWithInput( ctx, evm.EVM_MODULE_ADDRESS, nil, true, bytecodeForCall, ) if err != nil { diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index 1b2ed1c4d..9cc9290e9 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -284,7 +284,7 @@ func (k *Keeper) EthCall( txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash())) // pass false to not commit StateDB - res, err := k.ApplyEvmMsg(ctx, msg, nil, false, cfg, txConfig) + res, _, err := k.ApplyEvmMsg(ctx, msg, nil, false, cfg, txConfig) if err != nil { return nil, grpcstatus.Error(grpccodes.Internal, err.Error()) } @@ -422,7 +422,7 @@ func (k Keeper) EstimateGasForEvmCallType( WithTransientKVGasConfig(storetypes.GasConfig{}) } // pass false to not commit StateDB - rsp, err = k.ApplyEvmMsg(tmpCtx, msg, nil, false, cfg, txConfig) + rsp, _, err = k.ApplyEvmMsg(tmpCtx, msg, nil, false, cfg, txConfig) if err != nil { if errors.Is(err, core.ErrIntrinsicGas) { return true, nil, nil // Special case, raise gas limit @@ -518,7 +518,7 @@ func (k Keeper) TraceTx( ctx = ctx.WithGasMeter(eth.NewInfiniteGasMeterWithLimit(msg.Gas())). WithKVGasConfig(storetypes.GasConfig{}). WithTransientKVGasConfig(storetypes.GasConfig{}) - rsp, err := k.ApplyEvmMsg(ctx, msg, evm.NewNoOpTracer(), true, cfg, txConfig) + rsp, _, err := k.ApplyEvmMsg(ctx, msg, evm.NewNoOpTracer(), true, cfg, txConfig) if err != nil { continue } @@ -800,7 +800,7 @@ func (k *Keeper) TraceEthTxMsg( ctx = ctx.WithGasMeter(eth.NewInfiniteGasMeterWithLimit(msg.Gas())). WithKVGasConfig(storetypes.GasConfig{}). WithTransientKVGasConfig(storetypes.GasConfig{}) - res, err := k.ApplyEvmMsg(ctx, msg, tracer, commitMessage, cfg, txConfig) + res, _, err := k.ApplyEvmMsg(ctx, msg, tracer, commitMessage, cfg, txConfig) if err != nil { return nil, 0, grpcstatus.Error(grpccodes.Internal, err.Error()) } diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 27e80a610..b53f2f476 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -59,10 +59,10 @@ func (k *Keeper) EthereumTx( return nil, errors.Wrap(err, "failed to return ethereum transaction as core message") } - tmpCtx, commit := ctx.CacheContext() + tmpCtx, commitCtx := ctx.CacheContext() // pass true to commit the StateDB - evmResp, err = k.ApplyEvmMsg(tmpCtx, msg, nil, true, evmConfig, txConfig) + evmResp, _, err = k.ApplyEvmMsg(tmpCtx, msg, nil, true, evmConfig, txConfig) if err != nil { // when a transaction contains multiple msg, as long as one of the msg fails // all gas will be deducted. so is not msg.Gas() @@ -72,37 +72,18 @@ func (k *Keeper) EthereumTx( logs := evm.LogsToEthereum(evmResp.Logs) - cumulativeGasUsed := evmResp.GasUsed - if ctx.BlockGasMeter() != nil { - limit := ctx.BlockGasMeter().Limit() - cumulativeGasUsed += ctx.BlockGasMeter().GasConsumed() - if cumulativeGasUsed > limit { - cumulativeGasUsed = limit - } - } - var contractAddr gethcommon.Address if msg.To() == nil { contractAddr = crypto.CreateAddress(msg.From(), msg.Nonce()) } receipt := &gethcore.Receipt{ - Type: tx.Type(), - PostState: nil, // TODO: intermediate state root - CumulativeGasUsed: cumulativeGasUsed, - Bloom: k.EvmState.CalcBloomFromLogs(ctx, logs), - Logs: logs, - TxHash: txConfig.TxHash, - ContractAddress: contractAddr, - GasUsed: evmResp.GasUsed, - BlockHash: txConfig.BlockHash, - BlockNumber: big.NewInt(ctx.BlockHeight()), - TransactionIndex: txConfig.TxIndex, + Bloom: k.EvmState.CalcBloomFromLogs(ctx, logs), + Logs: logs, } if !evmResp.Failed() { - receipt.Status = gethcore.ReceiptStatusSuccessful - commit() + commitCtx() } // refund gas in order to match the Ethereum gas consumption instead of the default SDK one. @@ -282,14 +263,14 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, commit bool, evmConfig *statedb.EVMConfig, txConfig statedb.TxConfig, -) (*evm.MsgEthereumTxResponse, error) { +) (resp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) { var ( ret []byte // return bytes from evm execution vmErr error // vm errors do not effect consensus and are therefore not assigned to err ) stateDB := statedb.New(ctx, k, txConfig) - evmObj := k.NewEVM(ctx, msg, evmConfig, tracer, stateDB) + evmObj = k.NewEVM(ctx, msg, evmConfig, tracer, stateDB) leftoverGas := msg.Gas() @@ -308,7 +289,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, intrinsicGas, err := k.GetEthIntrinsicGas(ctx, msg, evmConfig.ChainConfig, contractCreation) if err != nil { // should have already been checked on Ante Handler - return nil, errors.Wrap(err, "intrinsic gas failed") + return nil, evmObj, errors.Wrap(err, "intrinsic gas failed") } // Check if the provided gas in the message is enough to cover the intrinsic @@ -319,7 +300,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, // don't go through Ante Handler. if leftoverGas < intrinsicGas { // eth_estimateGas will check for this exact error - return nil, errors.Wrapf( + return nil, evmObj, errors.Wrapf( core.ErrIntrinsicGas, "apply message msg.Gas = %d, intrinsic gas = %d.", leftoverGas, intrinsicGas, @@ -339,7 +320,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, msgWei, err := ParseWeiAsMultipleOfMicronibi(msg.Value()) if err != nil { - return nil, err + return nil, evmObj, err } if contractCreation { @@ -369,7 +350,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, // calculate gas refund if msg.Gas() < leftoverGas { - return nil, errors.Wrap(evm.ErrGasOverflow, "apply message") + return nil, evmObj, errors.Wrap(evm.ErrGasOverflow, "apply message") } // refund gas temporaryGasUsed := msg.Gas() - leftoverGas @@ -388,7 +369,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, // The dirty states in `StateDB` is either committed or discarded after return if commit { if err := stateDB.Commit(); err != nil { - return nil, fmt.Errorf("failed to commit stateDB: %w", err) + return nil, evmObj, fmt.Errorf("failed to commit stateDB: %w", err) } } @@ -397,11 +378,11 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, minimumGasUsed := gasLimit.Mul(minGasMultiplier) if !minimumGasUsed.TruncateInt().IsUint64() { - return nil, errors.Wrapf(evm.ErrGasOverflow, "minimumGasUsed(%s) is not a uint64", minimumGasUsed.TruncateInt().String()) + return nil, evmObj, errors.Wrapf(evm.ErrGasOverflow, "minimumGasUsed(%s) is not a uint64", minimumGasUsed.TruncateInt().String()) } if msg.Gas() < leftoverGas { - return nil, errors.Wrapf(evm.ErrGasOverflow, "message gas limit < leftover gas (%d < %d)", msg.Gas(), leftoverGas) + return nil, evmObj, errors.Wrapf(evm.ErrGasOverflow, "message gas limit < leftover gas (%d < %d)", msg.Gas(), leftoverGas) } gasUsed := math.LegacyMaxDec(minimumGasUsed, math.LegacyNewDec(int64(temporaryGasUsed))).TruncateInt().Uint64() @@ -417,7 +398,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context, Ret: ret, Logs: evm.NewLogsFromEth(stateDB.Logs()), Hash: txConfig.TxHash.Hex(), - }, nil + }, evmObj, nil } func ParseWeiAsMultipleOfMicronibi(weiInt *big.Int) (newWeiInt *big.Int, err error) { diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 2c7d13924..b1050dfe2 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -51,16 +51,15 @@ func (p precompileFunToken) Run( defer func() { err = ErrPrecompileRun(err, p) }() - - res, err := OnRunStart(evm, contract, p.ABI()) + start, err := OnRunStart(evm, contract, p.ABI()) if err != nil { return nil, err } - method, args, ctx := res.Method, res.Args, res.Ctx + method := start.Method switch PrecompileMethod(method.Name) { case FunTokenMethod_BankSend: - bz, err = p.bankSend(ctx, contract.CallerAddress, method, args, readonly) + bz, err = p.bankSend(start, contract.CallerAddress, readonly) default: // Note that this code path should be impossible to reach since // "DecomposeInput" parses methods directly from the ABI. @@ -70,11 +69,8 @@ func (p precompileFunToken) Run( if err != nil { return nil, err } - if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { - return nil, err - } - res.WriteCtx() - return bz, nil + // Dirty journal entries in `StateDB` must be committed + return bz, start.StateDB.Commit() } func PrecompileFunToken(keepers keepers.PublicKeepers) vm.PrecompiledContract { @@ -103,12 +99,11 @@ var executionGuard sync.Mutex // function bankSend(address erc20, uint256 amount, string memory to) external; // ``` func (p precompileFunToken) bankSend( - ctx sdk.Context, + start OnRunStartResult, caller gethcommon.Address, - method *gethabi.Method, - args []any, readOnly bool, ) (bz []byte, err error) { + ctx, method, args := start.Ctx, start.Method, start.Args if e := assertNotReadonlyTx(readOnly, true); e != nil { err = e return diff --git a/x/evm/precompile/funtoken_test.go b/x/evm/precompile/funtoken_test.go index 62512968f..dd5176fb3 100644 --- a/x/evm/precompile/funtoken_test.go +++ b/x/evm/precompile/funtoken_test.go @@ -112,7 +112,7 @@ func (s *FuntokenSuite) TestHappyPath() { { input, err := embeds.SmartContract_ERC20Minter.ABI.Pack("mint", deps.Sender.EthAddr, big.NewInt(69_420)) s.NoError(err) - _, err = deps.EvmKeeper.CallContractWithInput( + _, _, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &erc20, true, input, ) s.ErrorContains(err, "Ownable: caller is not the owner") diff --git a/x/evm/precompile/oracle_test.go b/x/evm/precompile/oracle_test.go index 0752016be..efab80118 100644 --- a/x/evm/precompile/oracle_test.go +++ b/x/evm/precompile/oracle_test.go @@ -58,7 +58,7 @@ func (s *OracleSuite) TestOracle_HappyPath() { deps.App.OracleKeeper.SetPrice(deps.Ctx, "unibi:uusd", sdk.MustNewDecFromStr("0.067")) input, err := embeds.SmartContract_Oracle.ABI.Pack("queryExchangeRate", "unibi:uusd") s.NoError(err) - resp, err := deps.EvmKeeper.CallContractWithInput( + resp, _, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Oracle, true, input, ) s.NoError(err) diff --git a/x/evm/precompile/precompile.go b/x/evm/precompile/precompile.go index 43cc4e0f0..8330e0428 100644 --- a/x/evm/precompile/precompile.go +++ b/x/evm/precompile/precompile.go @@ -143,17 +143,10 @@ type OnRunStartResult struct { // operations to occur that can be reverted by the EVM's [statedb.StateDB]. Ctx sdk.Context - // WriteCtx commits the cached context changes to the parent context. - WriteCtx func() - // Method is the ABI method for the precompiled contract call. Method *gethabi.Method - // SnapshotBeforeRun captures the state before precompile execution to enable - // proper state reversal if the call fails or if [statedb.JournalChange] - // is reverted in general. - SnapshotBeforeRun statedb.PrecompileSnapshotBeforeRun - StateDB *statedb.StateDB + StateDB *statedb.StateDB } // OnRunStart prepares the execution environment for a precompiled contract call. @@ -178,9 +171,8 @@ type OnRunStartResult struct { // } // // ... // // Use res.Ctx for state changes -// // Use res.WriteCtx to commit changes to the multistore -// // after successful execution -// res.WriteCtx() +// // Use res.StateDB.CommitContext() before any non-EVM state changes +// // to guarantee the context and [statedb.StateDB] are in sync. // } // ``` func OnRunStart( @@ -196,41 +188,19 @@ func OnRunStart( err = fmt.Errorf("failed to load the sdk.Context from the EVM StateDB") return } - cacheCtx, writeCacheCtx, stateDBSnapshot := stateDB.CacheCtxForPrecompile() - if err = stateDB.CommitContext(cacheCtx); err != nil { - return res, fmt.Errorf("error committing dirty journal entries for the precompile call to the cache ctx: %w", err) + ctx := stateDB.GetContext() + if err = stateDB.CommitContext(ctx); err != nil { + return res, fmt.Errorf("error committing dirty journal entries: %w", err) } return OnRunStartResult{ - Args: args, - Ctx: cacheCtx, - WriteCtx: writeCacheCtx, - Method: method, - SnapshotBeforeRun: stateDBSnapshot, - StateDB: stateDB, + Args: args, + Ctx: ctx, + Method: method, + StateDB: stateDB, }, nil } -// OnRunEnd finalizes a precompile execution by saving its state snapshot to the -// journal. This ensures that any state changes can be properly reverted if needed. -// -// Args: -// - stateDB: The EVM state database -// - snapshot: The state snapshot taken before the precompile executed -// - precompileAddr: The address of the precompiled contract -// -// The snapshot is critical for maintaining state consistency when: -// - The operation gets reverted ([statedb.JournalChange] Revert). -// - The precompile modifies state in other modules (e.g., bank, wasm) -// - Multiple precompiles are called within a single transaction -func OnRunEnd( - stateDB *statedb.StateDB, - snapshot statedb.PrecompileSnapshotBeforeRun, - precompileAddr gethcommon.Address, -) error { - return stateDB.SavePrecompileSnapshotToJournal(precompileAddr, snapshot) -} - var precompileMethodIsTxMap map[PrecompileMethod]bool = map[PrecompileMethod]bool{ WasmMethod_execute: true, WasmMethod_instantiate: true, diff --git a/x/evm/precompile/test/export.go b/x/evm/precompile/test/export.go new file mode 100644 index 000000000..966dd3359 --- /dev/null +++ b/x/evm/precompile/test/export.go @@ -0,0 +1,316 @@ +package test + +import ( + "encoding/json" + "os" + "os/exec" + "path" + "strings" + + wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" + wasm "github.com/CosmWasm/wasmd/x/wasm/types" + "github.com/ethereum/go-ethereum/core/vm" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + "github.com/NibiruChain/nibiru/v2/app" + "github.com/NibiruChain/nibiru/v2/x/evm/embeds" + "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" + "github.com/NibiruChain/nibiru/v2/x/evm/precompile" + "github.com/NibiruChain/nibiru/v2/x/evm/statedb" +) + +// SetupWasmContracts stores all Wasm bytecode and has the "deps.Sender" +// instantiate each Wasm contract using the precompile. +func SetupWasmContracts(deps *evmtest.TestDeps, s *suite.Suite) ( + contracts []sdk.AccAddress, +) { + wasmCodes := DeployWasmBytecode(s, deps.Ctx, deps.Sender.NibiruAddr, deps.App) + + otherArgs := []struct { + InstMsg []byte + Label string + }{ + { + InstMsg: []byte("{}"), + Label: "https://github.com/NibiruChain/nibiru-wasm/blob/main/contracts/nibi-stargate/src/contract.rs", + }, + { + InstMsg: []byte(`{"count": 0}`), + Label: "https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter", + }, + } + + for wasmCodeIdx, wasmCode := range wasmCodes { + s.T().Logf("Instantiate using Wasm precompile: %s", wasmCode.binPath) + codeId := wasmCode.codeId + + m := wasm.MsgInstantiateContract{ + Admin: "", + CodeID: codeId, + Label: otherArgs[wasmCodeIdx].Label, + Msg: otherArgs[wasmCodeIdx].InstMsg, + Funds: []sdk.Coin{}, + } + + msgArgsBz, err := json.Marshal(m.Msg) + s.NoError(err) + + var funds []precompile.WasmBankCoin + fundsJson, err := m.Funds.MarshalJSON() + s.NoErrorf(err, "fundsJson: %s", fundsJson) + err = json.Unmarshal(fundsJson, &funds) + s.Require().NoError(err) + + callArgs := []any{m.Admin, m.CodeID, msgArgsBz, m.Label, funds} + input, err := embeds.SmartContract_Wasm.ABI.Pack( + string(precompile.WasmMethod_instantiate), + callArgs..., + ) + s.Require().NoError(err) + + ethTxResp, evmObj, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, + ) + s.Require().NoError(err) + s.Require().NotEmpty(ethTxResp.Ret) + + // Finalize transaction + err = evmObj.StateDB.(*statedb.StateDB).Commit() + s.Require().NoError(err) + + s.T().Log("Parse the response contract addr and response bytes") + var contractAddrStr string + var data []byte + err = embeds.SmartContract_Wasm.ABI.UnpackIntoInterface( + &[]any{&contractAddrStr, &data}, + string(precompile.WasmMethod_instantiate), + ethTxResp.Ret, + ) + s.Require().NoError(err) + contractAddr, err := sdk.AccAddressFromBech32(contractAddrStr) + s.NoError(err) + contracts = append(contracts, contractAddr) + } + + return contracts +} + +// DeployWasmBytecode is a setup function that stores all Wasm bytecode used in +// the test suite. +func DeployWasmBytecode( + s *suite.Suite, + ctx sdk.Context, + sender sdk.AccAddress, + nibiru *app.NibiruApp, +) (codeIds []struct { + codeId uint64 + binPath string +}, +) { + // rootPath, _ := exec.Command("go list -m -f {{.Dir}}").Output() + // Run: go list -m -f {{.Dir}} + // This returns the path to the root of the project. + rootPathBz, err := exec.Command("go", "list", "-m", "-f", "{{.Dir}}").Output() + s.Require().NoError(err) + rootPath := strings.Trim(string(rootPathBz), "\n") + for _, pathToWasmBin := range []string{ + // nibi_stargate.wasm is a compiled version of: + // https://github.com/NibiruChain/nibiru-wasm/blob/main/contracts/nibi-stargate/src/contract.rs + "x/tokenfactory/fixture/nibi_stargate.wasm", + + // hello_world_counter.wasm is a compiled version of: + // https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter + "x/evm/precompile/hello_world_counter.wasm", + + // Add other wasm bytecode here if needed... + } { + pathToWasmBin = path.Join(string(rootPath), pathToWasmBin) + wasmBytecode, err := os.ReadFile(pathToWasmBin) + s.Require().NoErrorf( + err, + "rootPath %s, pathToWasmBin %s", rootPath, pathToWasmBin, + ) + + // The "Create" fn is private on the nibiru.WasmKeeper. By placing it as the + // decorated keeper in PermissionedKeeper type, we can access "Create" as a + // public fn. + wasmPermissionedKeeper := wasmkeeper.NewDefaultPermissionKeeper(nibiru.WasmKeeper) + instantiateAccess := &wasm.AccessConfig{ + Permission: wasm.AccessTypeEverybody, + } + codeId, _, err := wasmPermissionedKeeper.Create( + ctx, sender, wasmBytecode, instantiateAccess, + ) + s.Require().NoError(err) + codeIds = append(codeIds, struct { + codeId uint64 + binPath string + }{codeId, pathToWasmBin}) + } + + return codeIds +} + +// From IWasm.query of Wasm.sol: +// +// ```solidity +// function query( +// string memory contractAddr, +// bytes memory req +// ) external view returns (bytes memory response); +// ``` +func AssertWasmCounterState( + s *suite.Suite, + deps evmtest.TestDeps, + wasmContract sdk.AccAddress, + wantCount int64, +) (evmObj *vm.EVM) { + msgArgsBz := []byte(` + { + "count": {} + } + `) + + callArgs := []any{ + // string memory contractAddr + wasmContract.String(), + // bytes memory req + msgArgsBz, + } + input, err := embeds.SmartContract_Wasm.ABI.Pack( + string(precompile.WasmMethod_query), + callArgs..., + ) + s.Require().NoError(err) + + ethTxResp, evmObj, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, + ) + s.Require().NoError(err) + s.Require().NotEmpty(ethTxResp.Ret) + + s.T().Log("Parse the response contract addr and response bytes") + s.T().Logf("ethTxResp.Ret: %s", ethTxResp.Ret) + var queryResp []byte + err = embeds.SmartContract_Wasm.ABI.UnpackIntoInterface( + // Since there's only one return value, don't unpack as a slice. + // If there were two or more return values, we'd use + // &[]any{...} + &queryResp, + string(precompile.WasmMethod_query), + ethTxResp.Ret, + ) + s.Require().NoError(err) + s.T().Logf("queryResp: %s", queryResp) + + s.T().Log("Response is a JSON-encoded struct from the Wasm contract") + var wasmMsg wasm.RawContractMessage + err = json.Unmarshal(queryResp, &wasmMsg) + s.NoError(err) + s.NoError(wasmMsg.ValidateBasic()) + var typedResp QueryMsgCountResp + err = json.Unmarshal(wasmMsg, &typedResp) + s.NoError(err) + + s.EqualValues(wantCount, typedResp.Count) + s.EqualValues(deps.Sender.NibiruAddr.String(), typedResp.Owner) + return evmObj +} + +// Result of QueryMsg::Count from the [hello_world_counter] Wasm contract: +// +// ```rust +// #[cw_serde] +// pub struct State { +// pub count: i64, +// pub owner: Addr, +// } +// ``` +// +// [hello_world_counter]: https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter +type QueryMsgCountResp struct { + Count int64 `json:"count"` + Owner string `json:"owner"` +} + +// From evm/embeds/contracts/Wasm.sol: +// +// ```solidity +// struct WasmExecuteMsg { +// string contractAddr; +// bytes msgArgs; +// BankCoin[] funds; +// } +// +// /// @notice Identical to "execute", except for multiple contract calls. +// function executeMulti( +// WasmExecuteMsg[] memory executeMsgs +// ) payable external returns (bytes[] memory responses); +// ``` +// +// The increment call corresponds to the ExecuteMsg from +// the [hello_world_counter] Wasm contract: +// +// ```rust +// #[cw_serde] +// pub enum ExecuteMsg { +// Increment {}, // Increase count by 1 +// Reset { count: i64 }, // Reset to any i64 value +// } +// ``` +// +// [hello_world_counter]: https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter +func IncrementWasmCounterWithExecuteMulti( + s *suite.Suite, + deps *evmtest.TestDeps, + wasmContract sdk.AccAddress, + times uint, +) (evmObj *vm.EVM) { + msgArgsBz := []byte(` + { + "increment": {} + } + `) + + // Parse funds argument. + var funds []precompile.WasmBankCoin // blank funds + fundsJson, err := json.Marshal(funds) + s.NoErrorf(err, "fundsJson: %s", fundsJson) + err = json.Unmarshal(fundsJson, &funds) + s.Require().NoError(err, "fundsJson %s, funds %s", fundsJson, funds) + + // The "times" arg determines the number of messages in the executeMsgs slice + executeMsgs := []struct { + ContractAddr string `json:"contractAddr"` + MsgArgs []byte `json:"msgArgs"` + Funds []precompile.WasmBankCoin `json:"funds"` + }{ + {wasmContract.String(), msgArgsBz, funds}, + } + if times == 0 { + executeMsgs = executeMsgs[:0] // force empty + } else { + for i := uint(1); i < times; i++ { + executeMsgs = append(executeMsgs, executeMsgs[0]) + } + } + s.Require().Len(executeMsgs, int(times)) // sanity check assertion + + callArgs := []any{ + executeMsgs, + } + input, err := embeds.SmartContract_Wasm.ABI.Pack( + string(precompile.WasmMethod_executeMulti), + callArgs..., + ) + s.Require().NoError(err) + + ethTxResp, evmObj, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, + ) + s.Require().NoError(err) + s.Require().NotEmpty(ethTxResp.Ret) + return evmObj +} diff --git a/x/evm/precompile/wasm.go b/x/evm/precompile/wasm.go index bba973bb1..10817c673 100644 --- a/x/evm/precompile/wasm.go +++ b/x/evm/precompile/wasm.go @@ -36,23 +36,23 @@ func (p precompileWasm) Run( defer func() { err = ErrPrecompileRun(err, p) }() - res, err := OnRunStart(evm, contract, p.ABI()) + start, err := OnRunStart(evm, contract, p.ABI()) if err != nil { return nil, err } - method, args, ctx := res.Method, res.Args, res.Ctx + method := start.Method switch PrecompileMethod(method.Name) { case WasmMethod_execute: - bz, err = p.execute(ctx, contract.CallerAddress, method, args, readonly) + bz, err = p.execute(start, contract.CallerAddress, readonly) case WasmMethod_query: - bz, err = p.query(ctx, method, args, contract) + bz, err = p.query(start, contract) case WasmMethod_instantiate: - bz, err = p.instantiate(ctx, contract.CallerAddress, method, args, readonly) + bz, err = p.instantiate(start, contract.CallerAddress, readonly) case WasmMethod_executeMulti: - bz, err = p.executeMulti(ctx, contract.CallerAddress, method, args, readonly) + bz, err = p.executeMulti(start, contract.CallerAddress, readonly) case WasmMethod_queryRaw: - bz, err = p.queryRaw(ctx, method, args, contract) + bz, err = p.queryRaw(start, contract) default: // Note that this code path should be impossible to reach since // "DecomposeInput" parses methods directly from the ABI. @@ -63,11 +63,8 @@ func (p precompileWasm) Run( return nil, err } - if err := OnRunEnd(res.StateDB, res.SnapshotBeforeRun, p.Address()); err != nil { - return nil, err - } - res.WriteCtx() - return + // Dirty journal entries in `StateDB` must be committed + return bz, start.StateDB.Commit() } type precompileWasm struct { @@ -123,12 +120,11 @@ func PrecompileWasm(keepers keepers.PublicKeepers) vm.PrecompiledContract { // - funds: Optional funds to supply during the execute call. It's // uncommon to use this field, so you'll pass an empty array most of the time. func (p precompileWasm) execute( - ctx sdk.Context, + start OnRunStartResult, caller gethcommon.Address, - method *gethabi.Method, - args []any, readOnly bool, ) (bz []byte, err error) { + method, args, ctx := start.Method, start.Args, start.Ctx defer func() { if err != nil { err = ErrMethodCalled(method, err) @@ -164,11 +160,10 @@ func (p precompileWasm) execute( // ) external view returns (bytes memory response); // ``` func (p precompileWasm) query( - ctx sdk.Context, - method *gethabi.Method, - args []any, + start OnRunStartResult, contract *vm.Contract, ) (bz []byte, err error) { + method, args, ctx := start.Method, start.Args, start.Ctx defer func() { if err != nil { err = ErrMethodCalled(method, err) @@ -209,12 +204,11 @@ func (p precompileWasm) query( // ) payable external returns (string memory contractAddr, bytes memory data); // ``` func (p precompileWasm) instantiate( - ctx sdk.Context, + start OnRunStartResult, caller gethcommon.Address, - method *gethabi.Method, - args []any, readOnly bool, ) (bz []byte, err error) { + method, args, ctx := start.Method, start.Args, start.Ctx defer func() { if err != nil { err = ErrMethodCalled(method, err) @@ -261,12 +255,11 @@ func (p precompileWasm) instantiate( // ) payable external returns (bytes[] memory responses); // ``` func (p precompileWasm) executeMulti( - ctx sdk.Context, + start OnRunStartResult, caller gethcommon.Address, - method *gethabi.Method, - args []any, readOnly bool, ) (bz []byte, err error) { + method, args, ctx := start.Method, start.Args, start.Ctx defer func() { if err != nil { err = ErrMethodCalled(method, err) @@ -327,11 +320,10 @@ func (p precompileWasm) executeMulti( // - bz: The encoded raw data stored at the queried key // - err: Any error that occurred during the query func (p precompileWasm) queryRaw( - ctx sdk.Context, - method *gethabi.Method, - args []any, + start OnRunStartResult, contract *vm.Contract, ) (bz []byte, err error) { + method, args, ctx := start.Method, start.Args, start.Ctx defer func() { if err != nil { err = ErrMethodCalled(method, err) diff --git a/x/evm/precompile/wasm_test.go b/x/evm/precompile/wasm_test.go index 6db1d7642..950b5fb9a 100644 --- a/x/evm/precompile/wasm_test.go +++ b/x/evm/precompile/wasm_test.go @@ -4,16 +4,14 @@ import ( "encoding/json" "fmt" "math/big" - "os" - wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" wasm "github.com/CosmWasm/wasmd/x/wasm/types" - "github.com/NibiruChain/nibiru/v2/app" "github.com/NibiruChain/nibiru/v2/x/common/testutil" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" "github.com/NibiruChain/nibiru/v2/x/evm/precompile" + "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" tokenfactory "github.com/NibiruChain/nibiru/v2/x/tokenfactory/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -24,127 +22,9 @@ type WasmSuite struct { suite.Suite } -// SetupWasmContracts stores all Wasm bytecode and has the "deps.Sender" -// instantiate each Wasm contract using the precompile. -func SetupWasmContracts(deps *evmtest.TestDeps, s *suite.Suite) ( - contracts []sdk.AccAddress, -) { - wasmCodes := DeployWasmBytecode(s, deps.Ctx, deps.Sender.NibiruAddr, deps.App) - - otherArgs := []struct { - InstMsg []byte - Label string - }{ - { - InstMsg: []byte("{}"), - Label: "https://github.com/NibiruChain/nibiru-wasm/blob/main/contracts/nibi-stargate/src/contract.rs", - }, - { - InstMsg: []byte(`{"count": 0}`), - Label: "https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter", - }, - } - - for wasmCodeIdx, wasmCode := range wasmCodes { - s.T().Logf("Instantiate using Wasm precompile: %s", wasmCode.binPath) - codeId := wasmCode.codeId - - m := wasm.MsgInstantiateContract{ - Admin: "", - CodeID: codeId, - Label: otherArgs[wasmCodeIdx].Label, - Msg: otherArgs[wasmCodeIdx].InstMsg, - Funds: []sdk.Coin{}, - } - - msgArgsBz, err := json.Marshal(m.Msg) - s.NoError(err) - - var funds []precompile.WasmBankCoin - fundsJson, err := m.Funds.MarshalJSON() - s.NoErrorf(err, "fundsJson: %s", fundsJson) - err = json.Unmarshal(fundsJson, &funds) - s.Require().NoError(err) - - callArgs := []any{m.Admin, m.CodeID, msgArgsBz, m.Label, funds} - input, err := embeds.SmartContract_Wasm.ABI.Pack( - string(precompile.WasmMethod_instantiate), - callArgs..., - ) - s.Require().NoError(err) - - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( - deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, - ) - s.Require().NoError(err) - s.Require().NotEmpty(ethTxResp.Ret) - - s.T().Log("Parse the response contract addr and response bytes") - var contractAddrStr string - var data []byte - err = embeds.SmartContract_Wasm.ABI.UnpackIntoInterface( - &[]any{&contractAddrStr, &data}, - string(precompile.WasmMethod_instantiate), - ethTxResp.Ret, - ) - s.Require().NoError(err) - contractAddr, err := sdk.AccAddressFromBech32(contractAddrStr) - s.NoError(err) - contracts = append(contracts, contractAddr) - } - - return contracts -} - -// DeployWasmBytecode is a setup function that stores all Wasm bytecode used in -// the test suite. -func DeployWasmBytecode( - s *suite.Suite, - ctx sdk.Context, - sender sdk.AccAddress, - nibiru *app.NibiruApp, -) (codeIds []struct { - codeId uint64 - binPath string -}, -) { - for _, pathToWasmBin := range []string{ - // nibi_stargate.wasm is a compiled version of: - // https://github.com/NibiruChain/nibiru-wasm/blob/main/contracts/nibi-stargate/src/contract.rs - "../../tokenfactory/fixture/nibi_stargate.wasm", - - // hello_world_counter.wasm is a compiled version of: - // https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter - "./hello_world_counter.wasm", - - // Add other wasm bytecode here if needed... - } { - wasmBytecode, err := os.ReadFile(pathToWasmBin) - s.Require().NoError(err) - - // The "Create" fn is private on the nibiru.WasmKeeper. By placing it as the - // decorated keeper in PermissionedKeeper type, we can access "Create" as a - // public fn. - wasmPermissionedKeeper := wasmkeeper.NewDefaultPermissionKeeper(nibiru.WasmKeeper) - instantiateAccess := &wasm.AccessConfig{ - Permission: wasm.AccessTypeEverybody, - } - codeId, _, err := wasmPermissionedKeeper.Create( - ctx, sender, wasmBytecode, instantiateAccess, - ) - s.Require().NoError(err) - codeIds = append(codeIds, struct { - codeId uint64 - binPath string - }{codeId, pathToWasmBin}) - } - - return codeIds -} - func (s *WasmSuite) TestExecuteHappy() { deps := evmtest.NewTestDeps() - wasmContracts := SetupWasmContracts(&deps, &s.Suite) + wasmContracts := test.SetupWasmContracts(&deps, &s.Suite) wasmContract := wasmContracts[0] // nibi_stargate.wasm s.T().Log("Execute: create denom") @@ -172,7 +52,7 @@ func (s *WasmSuite) TestExecuteHappy() { ) s.Require().NoError(err) - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( + ethTxResp, _, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, ) s.Require().NoError(err) @@ -201,7 +81,7 @@ func (s *WasmSuite) TestExecuteHappy() { callArgs..., ) s.Require().NoError(err) - ethTxResp, err = deps.EvmKeeper.CallContractWithInput( + ethTxResp, _, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, ) s.Require().NoError(err) @@ -211,178 +91,27 @@ func (s *WasmSuite) TestExecuteHappy() { ) } -// Result of QueryMsg::Count from the [hello_world_counter] Wasm contract: -// -// ```rust -// #[cw_serde] -// pub struct State { -// pub count: i64, -// pub owner: Addr, -// } -// ``` -// -// [hello_world_counter]: https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter -type QueryMsgCountResp struct { - Count int64 `json:"count"` - Owner string `json:"owner"` -} - func (s *WasmSuite) TestExecuteMultiHappy() { deps := evmtest.NewTestDeps() - wasmContracts := SetupWasmContracts(&deps, &s.Suite) + wasmContracts := test.SetupWasmContracts(&deps, &s.Suite) wasmContract := wasmContracts[1] // hello_world_counter.wasm - s.assertWasmCounterState(deps, wasmContract, 0) // count = 0 - s.incrementWasmCounterWithExecuteMulti(&deps, wasmContract, 2) // count += 2 - s.assertWasmCounterState(deps, wasmContract, 2) // count = 2 + // count = 0 + test.AssertWasmCounterState(&s.Suite, deps, wasmContract, 0) + // count += 2 + test.IncrementWasmCounterWithExecuteMulti( + &s.Suite, &deps, wasmContract, 2) + // count = 2 + test.AssertWasmCounterState(&s.Suite, deps, wasmContract, 2) s.assertWasmCounterStateRaw(deps, wasmContract, 2) - s.incrementWasmCounterWithExecuteMulti(&deps, wasmContract, 67) // count += 67 - s.assertWasmCounterState(deps, wasmContract, 69) // count = 69 + // count += 67 + test.IncrementWasmCounterWithExecuteMulti( + &s.Suite, &deps, wasmContract, 67) + // count = 69 + test.AssertWasmCounterState(&s.Suite, deps, wasmContract, 69) s.assertWasmCounterStateRaw(deps, wasmContract, 69) } -// From IWasm.query of Wasm.sol: -// -// ```solidity -// function query( -// string memory contractAddr, -// bytes memory req -// ) external view returns (bytes memory response); -// ``` -func (s *WasmSuite) assertWasmCounterState( - deps evmtest.TestDeps, - wasmContract sdk.AccAddress, - wantCount int64, -) { - msgArgsBz := []byte(` - { - "count": {} - } - `) - - callArgs := []any{ - // string memory contractAddr - wasmContract.String(), - // bytes memory req - msgArgsBz, - } - input, err := embeds.SmartContract_Wasm.ABI.Pack( - string(precompile.WasmMethod_query), - callArgs..., - ) - s.Require().NoError(err) - - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( - deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, - ) - s.Require().NoError(err) - s.Require().NotEmpty(ethTxResp.Ret) - - s.T().Log("Parse the response contract addr and response bytes") - s.T().Logf("ethTxResp.Ret: %s", ethTxResp.Ret) - var queryResp []byte - err = embeds.SmartContract_Wasm.ABI.UnpackIntoInterface( - // Since there's only one return value, don't unpack as a slice. - // If there were two or more return values, we'd use - // &[]any{...} - &queryResp, - string(precompile.WasmMethod_query), - ethTxResp.Ret, - ) - s.Require().NoError(err) - s.T().Logf("queryResp: %s", queryResp) - - s.T().Log("Response is a JSON-encoded struct from the Wasm contract") - var wasmMsg wasm.RawContractMessage - err = json.Unmarshal(queryResp, &wasmMsg) - s.NoError(err) - s.NoError(wasmMsg.ValidateBasic()) - var typedResp QueryMsgCountResp - err = json.Unmarshal(wasmMsg, &typedResp) - s.NoError(err) - - s.EqualValues(wantCount, typedResp.Count) - s.EqualValues(deps.Sender.NibiruAddr.String(), typedResp.Owner) -} - -// From evm/embeds/contracts/Wasm.sol: -// -// ```solidity -// struct WasmExecuteMsg { -// string contractAddr; -// bytes msgArgs; -// BankCoin[] funds; -// } -// -// /// @notice Identical to "execute", except for multiple contract calls. -// function executeMulti( -// WasmExecuteMsg[] memory executeMsgs -// ) payable external returns (bytes[] memory responses); -// ``` -// -// The increment call corresponds to the ExecuteMsg from -// the [hello_world_counter] Wasm contract: -// -// ```rust -// #[cw_serde] -// pub enum ExecuteMsg { -// Increment {}, // Increase count by 1 -// Reset { count: i64 }, // Reset to any i64 value -// } -// ``` -// -// [hello_world_counter]: https://github.com/NibiruChain/nibiru-wasm/tree/ec3ab9f09587a11fbdfbd4021c7617eca3912044/contracts/00-hello-world-counter -func (s *WasmSuite) incrementWasmCounterWithExecuteMulti( - deps *evmtest.TestDeps, - wasmContract sdk.AccAddress, - times uint, -) { - msgArgsBz := []byte(` - { - "increment": {} - } - `) - - // Parse funds argument. - var funds []precompile.WasmBankCoin // blank funds - fundsJson, err := json.Marshal(funds) - s.NoErrorf(err, "fundsJson: %s", fundsJson) - err = json.Unmarshal(fundsJson, &funds) - s.Require().NoError(err, "fundsJson %s, funds %s", fundsJson, funds) - - // The "times" arg determines the number of messages in the executeMsgs slice - executeMsgs := []struct { - ContractAddr string `json:"contractAddr"` - MsgArgs []byte `json:"msgArgs"` - Funds []precompile.WasmBankCoin `json:"funds"` - }{ - {wasmContract.String(), msgArgsBz, funds}, - } - if times == 0 { - executeMsgs = executeMsgs[:0] // force empty - } else { - for i := uint(1); i < times; i++ { - executeMsgs = append(executeMsgs, executeMsgs[0]) - } - } - s.Require().Len(executeMsgs, int(times)) // sanity check assertion - - callArgs := []any{ - executeMsgs, - } - input, err := embeds.SmartContract_Wasm.ABI.Pack( - string(precompile.WasmMethod_executeMulti), - callArgs..., - ) - s.Require().NoError(err) - - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( - deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, - ) - s.Require().NoError(err) - s.Require().NotEmpty(ethTxResp.Ret) -} - // From IWasm.query of Wasm.sol: // // ```solidity @@ -407,7 +136,7 @@ func (s *WasmSuite) assertWasmCounterStateRaw( ) s.Require().NoError(err) - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( + ethTxResp, _, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, ) s.Require().NoError(err) @@ -430,7 +159,7 @@ func (s *WasmSuite) assertWasmCounterStateRaw( s.T().Logf("wasmMsg: %s", wasmMsg) s.NoError(wasmMsg.ValidateBasic()) - var typedResp QueryMsgCountResp + var typedResp test.QueryMsgCountResp s.NoError(json.Unmarshal(wasmMsg, &typedResp)) s.EqualValues(wantCount, typedResp.Count) s.EqualValues(deps.Sender.NibiruAddr.String(), typedResp.Owner) @@ -577,7 +306,7 @@ func (s *WasmSuite) TestSadArgsExecute() { ) s.Require().NoError(err) - ethTxResp, err := deps.EvmKeeper.CallContractWithInput( + ethTxResp, _, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, deps.Sender.EthAddr, &precompile.PrecompileAddr_Wasm, true, input, ) s.ErrorContains(err, tc.wantError) diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index ab72839c6..8228b9a23 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -21,8 +21,6 @@ import ( "math/big" "sort" - store "github.com/cosmos/cosmos-sdk/store/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" ) @@ -88,47 +86,17 @@ func (j *journal) Revert(statedb *StateDB, snapshot int) { j.entries = j.entries[:snapshot] } -// length returns the current number of entries in the journal. -func (j *journal) length() int { +// Length returns the current number of entries in the journal. +func (j *journal) Length() int { return len(j.entries) } -// ------------------------------------------------------ -// PrecompileSnapshotBeforeRun - -// PrecompileSnapshotBeforeRun: Precompiles can alter persistent storage of other -// modules. These changes to persistent storage are not reverted by a `Revert` of -// [JournalChange] by default, as it generally manages only changes to accounts -// and Bank balances for ether (NIBI). -// -// As a workaround to make state changes from precompiles reversible, we store -// [PrecompileSnapshotBeforeRun] snapshots that sync and record the prior state -// of the other modules, allowing precompile calls to truly be reverted. -// -// As a simple example, suppose that a transaction calls a precompile. -// 1. If the precompile changes the state in the Bank Module or Wasm module -// 2. The call gets reverted (`revert()` in Solidity), which shoud restore the -// state to a in-memory snapshot recorded on the StateDB journal. -// 3. This could cause a problem where changes to the rest of the blockchain state -// are still in effect following the reversion in the EVM state DB. -type PrecompileSnapshotBeforeRun struct { - MultiStore store.CacheMultiStore - Events sdk.Events -} - -var _ JournalChange = PrecompileSnapshotBeforeRun{} - -func (ch PrecompileSnapshotBeforeRun) Revert(s *StateDB) { - s.cacheCtx = s.cacheCtx.WithMultiStore(ch.MultiStore) - // Rewrite the `writeCacheCtxFn` using the same logic as sdk.Context.CacheCtx - s.writeCacheCtxFn = func() { - s.ctx.EventManager().EmitEvents(ch.Events) - ch.MultiStore.Write() - } +func (j *journal) EntriesCopy() []JournalChange { + return j.entries } -func (ch PrecompileSnapshotBeforeRun) Dirtied() *common.Address { - return nil +func (j *journal) DirtiesLen() int { + return len(j.dirties) } // ------------------------------------------------------ diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go new file mode 100644 index 000000000..80e93f320 --- /dev/null +++ b/x/evm/statedb/journal_test.go @@ -0,0 +1,88 @@ +package statedb_test + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/core/vm" + + "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" + "github.com/NibiruChain/nibiru/v2/x/evm" + "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" + "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" + precompiletest "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" + "github.com/NibiruChain/nibiru/v2/x/evm/statedb" +) + +func (s *Suite) TestPrecompileSnapshots() { + deps := evmtest.NewTestDeps() + bankDenom := evm.EVMBankDenom + s.Require().NoError(testapp.FundAccount( + deps.App.BankKeeper, + deps.Ctx, + deps.Sender.NibiruAddr, + sdk.NewCoins(sdk.NewCoin(bankDenom, sdk.NewInt(69_420))), + )) + + s.T().Log("Set up helloworldcounter.wasm") + wasmContract := precompiletest.SetupWasmContracts(&deps, &s.Suite)[1] + type Transition struct { + Run func(deps *evmtest.TestDeps) *vm.EVM + AssertionsBeforeRun func(deps *evmtest.TestDeps) + } + fmt.Printf("wasmContract: %v\n", wasmContract) + stateTransitions := []Transition{ + { + AssertionsBeforeRun: func(deps *evmtest.TestDeps) { + precompiletest.AssertWasmCounterState( + &s.Suite, *deps, wasmContract, 0, + ) + }, + Run: func(deps *evmtest.TestDeps) *vm.EVM { + return test.IncrementWasmCounterWithExecuteMulti( + &s.Suite, deps, wasmContract, 7, + ) + }, + }, + { + AssertionsBeforeRun: func(deps *evmtest.TestDeps) { + precompiletest.AssertWasmCounterState( + &s.Suite, *deps, wasmContract, 7, + ) + }, + Run: func(deps *evmtest.TestDeps) *vm.EVM { + return test.IncrementWasmCounterWithExecuteMulti( + &s.Suite, deps, wasmContract, 5, + ) + }, + }, + { + AssertionsBeforeRun: func(deps *evmtest.TestDeps) { + precompiletest.AssertWasmCounterState( + &s.Suite, *deps, wasmContract, 12, + ) + }, + }, + } + + s.T().Log("Assert before transition") + + transitionIdx := 0 + st := stateTransitions[transitionIdx] + st.AssertionsBeforeRun(&deps) + + s.T().Log("Run state transition") + + evmObj := st.Run(&deps) + stateDB, ok := evmObj.StateDB.(*statedb.StateDB) + s.Require().True(ok, "error retrieving StateDB from the EVM") + + entries := stateDB.Journal.EntriesCopy() + s.Require().Len(entries, 13, "expect 13 journal entries") + s.Equal(0, stateDB.Journal.DirtiesLen()) + + assertionsAfter := stateTransitions[transitionIdx+1].AssertionsBeforeRun + assertionsAfter(&deps) + + // st.AssertionsBeforeRun(&deps) +} diff --git a/x/evm/statedb/state_object.go b/x/evm/statedb/state_object.go index e357de7d1..ce9d21f5b 100644 --- a/x/evm/statedb/state_object.go +++ b/x/evm/statedb/state_object.go @@ -170,7 +170,7 @@ func (s *stateObject) SubBalance(amount *big.Int) { // SetBalance update account balance. func (s *stateObject) SetBalance(amount *big.Int) { - s.db.journal.append(balanceChange{ + s.db.Journal.append(balanceChange{ account: &s.address, prevWei: new(big.Int).Set(s.account.BalanceWei), }) @@ -212,7 +212,7 @@ func (s *stateObject) CodeSize() int { // SetCode set contract code to account func (s *stateObject) SetCode(codeHash common.Hash, code []byte) { prevcode := s.Code() - s.db.journal.append(codeChange{ + s.db.Journal.append(codeChange{ account: &s.address, prevhash: s.CodeHash(), prevcode: prevcode, @@ -228,7 +228,7 @@ func (s *stateObject) setCode(codeHash common.Hash, code []byte) { // SetNonce set nonce to account func (s *stateObject) SetNonce(nonce uint64) { - s.db.journal.append(nonceChange{ + s.db.Journal.append(nonceChange{ account: &s.address, prev: s.account.Nonce, }) @@ -281,7 +281,7 @@ func (s *stateObject) SetState(key common.Hash, value common.Hash) { return } // New value is different, update and journal the change - s.db.journal.append(storageChange{ + s.db.Journal.append(storageChange{ account: &s.address, key: key, prevalue: prev, diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index 5ae1586a6..ab33cc4ea 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -33,22 +33,9 @@ type StateDB struct { // ctx is the persistent context used for official `StateDB.Commit` calls. ctx sdk.Context - // cacheCtx: An sdk.Context produced from the [StateDB.ctx] with the - // multi-store cached and a new event manager. The cached context - // (`cacheCtx`) is written to the persistent context (`ctx`) when - // `writeCacheCtx` is called. - cacheCtx sdk.Context - - // Events are automatically emitted on the parent context's - // EventManager when the caller executes the [writeCacheCtxFn]. - writeCacheCtxFn func() - - // The number of precompiled contract calls within the current transaction - precompileSnapshotsCount uint8 - // Journal of state modifications. This is the backbone of // Snapshot and RevertToSnapshot. - journal *journal + Journal *journal validRevisions []revision nextRevisionID int @@ -72,7 +59,7 @@ func New(ctx sdk.Context, keeper Keeper, txConfig TxConfig) *StateDB { keeper: keeper, ctx: ctx, stateObjects: make(map[common.Address]*stateObject), - journal: newJournal(), + Journal: newJournal(), accessList: newAccessList(), txConfig: txConfig, @@ -91,7 +78,7 @@ func (s *StateDB) GetContext() sdk.Context { // AddLog adds a log, called by evm. func (s *StateDB) AddLog(log *gethcore.Log) { - s.journal.append(addLogChange{}) + s.Journal.append(addLogChange{}) log.TxHash = s.txConfig.TxHash log.BlockHash = s.txConfig.BlockHash @@ -107,14 +94,14 @@ func (s *StateDB) Logs() []*gethcore.Log { // AddRefund adds gas to the refund counter func (s *StateDB) AddRefund(gas uint64) { - s.journal.append(refundChange{prev: s.refund}) + s.Journal.append(refundChange{prev: s.refund}) s.refund += gas } // SubRefund removes gas from the refund counter. // This method will panic if the refund counter goes below zero func (s *StateDB) SubRefund(gas uint64) { - s.journal.append(refundChange{prev: s.refund}) + s.Journal.append(refundChange{prev: s.refund}) if gas > s.refund { panic(fmt.Sprintf("Refund counter below zero (gas: %d > refund: %d)", gas, s.refund)) } @@ -253,9 +240,9 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) newobj = newObject(s, addr, Account{}) if prev == nil { - s.journal.append(createObjectChange{account: &addr}) + s.Journal.append(createObjectChange{account: &addr}) } else { - s.journal.append(resetObjectChange{prev: prev}) + s.Journal.append(resetObjectChange{prev: prev}) } s.setStateObject(newobj) if prev != nil { @@ -357,7 +344,7 @@ func (s *StateDB) Suicide(addr common.Address) bool { if stateObject == nil { return false } - s.journal.append(suicideChange{ + s.Journal.append(suicideChange{ account: &addr, prev: stateObject.suicided, prevbalance: new(big.Int).Set(stateObject.Balance()), @@ -402,7 +389,7 @@ func (s *StateDB) PrepareAccessList( // AddAddressToAccessList adds the given address to the access list func (s *StateDB) AddAddressToAccessList(addr common.Address) { if s.accessList.AddAddress(addr) { - s.journal.append(accessListAddAccountChange{&addr}) + s.Journal.append(accessListAddAccountChange{&addr}) } } @@ -414,10 +401,10 @@ func (s *StateDB) AddSlotToAccessList(addr common.Address, slot common.Hash) { // scope of 'address' without having the 'address' become already added // to the access list (via call-variant, create, etc). // Better safe than sorry, though - s.journal.append(accessListAddAccountChange{&addr}) + s.Journal.append(accessListAddAccountChange{&addr}) } if slotMod { - s.journal.append(accessListAddSlotChange{ + s.Journal.append(accessListAddSlotChange{ address: &addr, slot: &slot, }) @@ -438,7 +425,7 @@ func (s *StateDB) SlotInAccessList(addr common.Address, slot common.Hash) (addre func (s *StateDB) Snapshot() int { id := s.nextRevisionID s.nextRevisionID++ - s.validRevisions = append(s.validRevisions, revision{id, s.journal.length()}) + s.validRevisions = append(s.validRevisions, revision{id, s.Journal.Length()}) return id } @@ -454,7 +441,7 @@ func (s *StateDB) RevertToSnapshot(revid int) { snapshot := s.validRevisions[idx].journalIndex // Replay the journal to undo changes and remove invalidated snapshots - s.journal.Revert(s, snapshot) + s.Journal.Revert(s, snapshot) s.validRevisions = s.validRevisions[:idx] } @@ -467,18 +454,18 @@ func errorf(format string, args ...any) error { // StateDB object cannot be reused after [CommitContext] has completed. A new // object needs to be created from the EVM. func (s *StateDB) CommitContext(ctx sdk.Context) error { - for _, addr := range s.journal.sortedDirties() { + for _, addr := range s.Journal.sortedDirties() { obj := s.stateObjects[addr] if obj.suicided { if err := s.keeper.DeleteAccount(ctx, obj.Address()); err != nil { - return errorf("failed to delete account: %w") + return errorf("failed to delete account: %w", err) } } else { if obj.code != nil && obj.dirtyCode { s.keeper.SetCode(ctx, obj.CodeHash(), obj.code) } if err := s.keeper.SetAccount(ctx, obj.Address(), obj.account.ToNative()); err != nil { - return errorf("failed to set account: %w") + return errorf("failed to set account: %w", err) } for _, key := range obj.dirtyStorage.SortedKeys() { value := obj.dirtyStorage[key] @@ -497,8 +484,7 @@ func (s *StateDB) CommitContext(ctx sdk.Context) error { // StateDB object cannot be reused after [CommitContext] has completed. A new // object needs to be created from the EVM. func (s *StateDB) Commit() error { - ctx := s.ctx - return s.CommitContext(ctx) + return s.CommitContext(s.ctx) } // StateObjects: Returns a copy of the [StateDB.stateObjects] map. @@ -509,38 +495,3 @@ func (s *StateDB) StateObjects() map[common.Address]*stateObject { } return copyOfMap } - -func (s *StateDB) CacheCtxForPrecompile() ( - sdk.Context, func(), PrecompileSnapshotBeforeRun, -) { - if s.writeCacheCtxFn == nil { - s.cacheCtx, s.writeCacheCtxFn = s.ctx.CacheContext() - } - return s.cacheCtx, s.writeCacheCtxFn, PrecompileSnapshotBeforeRun{ - MultiStore: s.cacheCtx.MultiStore().CacheMultiStore(), - Events: s.cacheCtx.EventManager().Events(), - } -} - -// SavePrecompileSnapshotToJournal adds a snapshot of the commit multistore -// ([PrecompileSnapshotBeforeRun]) to the [StateDB] journal at the end of -// successful invocation of a precompiled contract. This is necessary to revert -// intermediate states where an EVM contract augments the multistore with a -// precompile and an inconsistency occurs between the EVM module and other -// modules. -// -// See [PrecompileSnapshotBeforeRun] for more info. -func (s *StateDB) SavePrecompileSnapshotToJournal( - precompileAddr common.Address, - snapshot PrecompileSnapshotBeforeRun, -) error { - obj := s.getOrNewStateObject(precompileAddr) - obj.db.journal.append(snapshot) - s.precompileSnapshotsCount++ - if s.precompileSnapshotsCount > maxPrecompileCalls { - return fmt.Errorf("exceeded maximum allowed number of precompiled contract calls in one transaction (%d)", maxPrecompileCalls) - } - return nil -} - -const maxPrecompileCalls uint8 = 10 From cc7a802bf667d01f7620758da9fb77ee1059c164 Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Wed, 23 Oct 2024 21:03:50 -0500 Subject: [PATCH 09/12] wip!: checkpoint --- x/evm/keeper/erc20.go | 7 ++- x/evm/keeper/erc20_test.go | 7 ++- x/evm/statedb/journal.go | 4 -- x/evm/statedb/journal_test.go | 88 ++++++++++++++++++----------------- 4 files changed, 54 insertions(+), 52 deletions(-) diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index 280630e82..4546c412c 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -53,13 +53,12 @@ See [nibiru/x/evm/embeds]. func (e erc20Calls) Mint( contract, from, to gethcommon.Address, amount *big.Int, ctx sdk.Context, -) (evmResp *evm.MsgEthereumTxResponse, err error) { +) (evmResp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) { input, err := e.ABI.Pack("mint", to, amount) if err != nil { - return nil, fmt.Errorf("failed to pack ABI args: %w", err) + return nil, nil, fmt.Errorf("failed to pack ABI args: %w", err) } - evmResp, _, err = e.CallContractWithInput(ctx, from, &contract, true, input) - return + return e.CallContractWithInput(ctx, from, &contract, true, input) } /* diff --git a/x/evm/keeper/erc20_test.go b/x/evm/keeper/erc20_test.go index d328ea4e6..2b49e8192 100644 --- a/x/evm/keeper/erc20_test.go +++ b/x/evm/keeper/erc20_test.go @@ -16,13 +16,16 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Mint tokens - Fail from non-owner") { - _, err := deps.EvmKeeper.ERC20().Mint(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx) + _, _, err := deps.EvmKeeper.ERC20().Mint( + contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, + big.NewInt(69_420), deps.Ctx, + ) s.ErrorContains(err, evm.ErrOwnable) } s.T().Log("Mint tokens - Success") { - _, err := deps.EvmKeeper.ERC20().Mint(contract, evm.EVM_MODULE_ADDRESS, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx) + _, _, err := deps.EvmKeeper.ERC20().Mint(contract, evm.EVM_MODULE_ADDRESS, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx) s.Require().NoError(err) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0)) diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index 8228b9a23..0c75cf0e3 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -91,10 +91,6 @@ func (j *journal) Length() int { return len(j.entries) } -func (j *journal) EntriesCopy() []JournalChange { - return j.entries -} - func (j *journal) DirtiesLen() int { return len(j.dirties) } diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index 80e93f320..026db7e41 100644 --- a/x/evm/statedb/journal_test.go +++ b/x/evm/statedb/journal_test.go @@ -8,6 +8,7 @@ import ( "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" "github.com/NibiruChain/nibiru/v2/x/evm" + "github.com/NibiruChain/nibiru/v2/x/evm/embeds" "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" precompiletest "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" @@ -25,64 +26,67 @@ func (s *Suite) TestPrecompileSnapshots() { )) s.T().Log("Set up helloworldcounter.wasm") + wasmContract := precompiletest.SetupWasmContracts(&deps, &s.Suite)[1] type Transition struct { Run func(deps *evmtest.TestDeps) *vm.EVM AssertionsBeforeRun func(deps *evmtest.TestDeps) } fmt.Printf("wasmContract: %v\n", wasmContract) - stateTransitions := []Transition{ - { - AssertionsBeforeRun: func(deps *evmtest.TestDeps) { - precompiletest.AssertWasmCounterState( - &s.Suite, *deps, wasmContract, 0, - ) - }, - Run: func(deps *evmtest.TestDeps) *vm.EVM { - return test.IncrementWasmCounterWithExecuteMulti( - &s.Suite, deps, wasmContract, 7, - ) - }, - }, - { - AssertionsBeforeRun: func(deps *evmtest.TestDeps) { - precompiletest.AssertWasmCounterState( - &s.Suite, *deps, wasmContract, 7, - ) - }, - Run: func(deps *evmtest.TestDeps) *vm.EVM { - return test.IncrementWasmCounterWithExecuteMulti( - &s.Suite, deps, wasmContract, 5, - ) - }, - }, - { - AssertionsBeforeRun: func(deps *evmtest.TestDeps) { - precompiletest.AssertWasmCounterState( - &s.Suite, *deps, wasmContract, 12, - ) - }, - }, + assertionsBeforeRun := func(deps *evmtest.TestDeps) { + precompiletest.AssertWasmCounterState( + &s.Suite, *deps, wasmContract, 0, + ) + } + run := func(deps *evmtest.TestDeps) *vm.EVM { + return test.IncrementWasmCounterWithExecuteMulti( + &s.Suite, deps, wasmContract, 7, + ) + } + assertionsAfterRun := func(deps *evmtest.TestDeps) { + precompiletest.AssertWasmCounterState( + &s.Suite, *deps, wasmContract, 7, + ) } s.T().Log("Assert before transition") - transitionIdx := 0 - st := stateTransitions[transitionIdx] - st.AssertionsBeforeRun(&deps) + assertionsBeforeRun(&deps) + + s.T().Log("Populate dirty journal entries") + + deployArgs := []any{"name", "SYMBOL", uint8(18)} + deployResp, err := evmtest.DeployContract( + &deps, + embeds.SmartContract_ERC20Minter, + deployArgs..., + ) + s.Require().NoError(err, deployResp) + + deps.EvmKeeper.ERC20().Mint() + _, _, err := deps.EvmKeeper.ERC20().Mint( + contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, + big.NewInt(69_420), deps.Ctx, + ) + + evmtest.TransferWei() s.T().Log("Run state transition") - evmObj := st.Run(&deps) + evmObj := run(&deps) stateDB, ok := evmObj.StateDB.(*statedb.StateDB) s.Require().True(ok, "error retrieving StateDB from the EVM") - - entries := stateDB.Journal.EntriesCopy() - s.Require().Len(entries, 13, "expect 13 journal entries") s.Equal(0, stateDB.Journal.DirtiesLen()) - assertionsAfter := stateTransitions[transitionIdx+1].AssertionsBeforeRun - assertionsAfter(&deps) + ctxBefore, _ := deps.Ctx.CacheContext() + assertionsAfterRun(&deps) + err := stateDB.Commit() + s.NoError(err) + assertionsAfterRun(&deps) + + s.Equal(0, stateDB.Journal.DirtiesLen()) - // st.AssertionsBeforeRun(&deps) + s.Require().EqualValues(ctxBefore, deps.Ctx, + "StateDB should have been committed by the precompile", + ) } From be2c0a28bfaf810310500adcd14fa6af1f1bd0fd Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Thu, 24 Oct 2024 03:37:23 -0500 Subject: [PATCH 10/12] fix: improve StateDB robustness. Make commit debuggable --- x/evm/evmmodule/genesis_test.go | 4 +- x/evm/keeper/erc20.go | 12 +-- x/evm/keeper/erc20_test.go | 4 +- x/evm/keeper/msg_server.go | 4 +- x/evm/precompile/funtoken.go | 2 +- x/evm/precompile/precompile.go | 4 +- x/evm/statedb/journal.go | 44 ++++++++++- x/evm/statedb/journal_test.go | 136 ++++++++++++++++++++++++++------ x/evm/statedb/state_object.go | 22 +++--- x/evm/statedb/statedb.go | 50 ++++++------ 10 files changed, 207 insertions(+), 75 deletions(-) diff --git a/x/evm/evmmodule/genesis_test.go b/x/evm/evmmodule/genesis_test.go index 690745e84..ff19ef46f 100644 --- a/x/evm/evmmodule/genesis_test.go +++ b/x/evm/evmmodule/genesis_test.go @@ -54,11 +54,11 @@ func (s *Suite) TestExportInitGenesis() { s.Require().NoError(err) // Transfer ERC-20 tokens to user A - _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx) + _, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx) s.Require().NoError(err) // Transfer ERC-20 tokens to user B - _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx) + _, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx) s.Require().NoError(err) // Create fungible token from bank coin diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index 8a41cb237..59c98b628 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -73,23 +73,23 @@ Transfer implements "ERC20.transfer" func (e erc20Calls) Transfer( contract, from, to gethcommon.Address, amount *big.Int, ctx sdk.Context, -) (out bool, err error) { +) (out bool, evmObj *vm.EVM, err error) { input, err := e.ABI.Pack("transfer", to, amount) if err != nil { - return false, fmt.Errorf("failed to pack ABI args: %w", err) + return false, nil, fmt.Errorf("failed to pack ABI args: %w", err) } - resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input) + resp, evmObj, err := e.CallContractWithInput(ctx, from, &contract, true, input) if err != nil { - return false, err + return false, nil, err } var erc20Bool ERC20Bool err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret) if err != nil { - return false, err + return false, nil, err } - return erc20Bool.Value, nil + return erc20Bool.Value, evmObj, nil } // BalanceOf retrieves the balance of an ERC20 token for a specific account. diff --git a/x/evm/keeper/erc20_test.go b/x/evm/keeper/erc20_test.go index 2b49e8192..15807b714 100644 --- a/x/evm/keeper/erc20_test.go +++ b/x/evm/keeper/erc20_test.go @@ -34,7 +34,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Transfer - Not enough funds") { - _, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx) + _, _, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx) s.ErrorContains(err, "ERC20: transfer amount exceeds balance") // balances unchanged evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0)) @@ -43,7 +43,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Transfer - Success (sanity check)") { - _, err := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx) + _, _, err := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx) s.Require().NoError(err) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(9_420)) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000)) diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 4656f7db2..5c52acc0f 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -590,7 +590,7 @@ func (k Keeper) convertCoinNativeERC20( } // unescrow ERC-20 tokens from EVM module address - res, err := k.ERC20().Transfer( + res, _, err := k.ERC20().Transfer( erc20Addr, evm.EVM_MODULE_ADDRESS, recipient, @@ -686,7 +686,7 @@ func (k *Keeper) EmitEthereumTxEvents( // Emit typed events if !evmResp.Failed() { if recipient == nil { // contract creation - var contractAddr = crypto.CreateAddress(msg.From(), msg.Nonce()) + contractAddr := crypto.CreateAddress(msg.From(), msg.Nonce()) _ = ctx.EventManager().EmitTypedEvent(&evm.EventContractDeployed{ Sender: msg.From().Hex(), ContractAddr: contractAddr.String(), diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index b1050dfe2..3b453d597 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -141,7 +141,7 @@ func (p precompileFunToken) bankSend( // Caller transfers ERC20 to the EVM account transferTo := evm.EVM_MODULE_ADDRESS - _, err = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx) + _, _, err = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx) if err != nil { return nil, fmt.Errorf("failed to send from caller to the EVM account: %w", err) } diff --git a/x/evm/precompile/precompile.go b/x/evm/precompile/precompile.go index 8330e0428..a6bbfefc4 100644 --- a/x/evm/precompile/precompile.go +++ b/x/evm/precompile/precompile.go @@ -171,7 +171,7 @@ type OnRunStartResult struct { // } // // ... // // Use res.Ctx for state changes -// // Use res.StateDB.CommitContext() before any non-EVM state changes +// // Use res.StateDB.Commit() before any non-EVM state changes // // to guarantee the context and [statedb.StateDB] are in sync. // } // ``` @@ -189,7 +189,7 @@ func OnRunStart( return } ctx := stateDB.GetContext() - if err = stateDB.CommitContext(ctx); err != nil { + if err = stateDB.Commit(); err != nil { return res, fmt.Errorf("error committing dirty journal entries: %w", err) } diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index 0c75cf0e3..2a6ffe953 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -91,8 +91,46 @@ func (j *journal) Length() int { return len(j.entries) } -func (j *journal) DirtiesLen() int { - return len(j.dirties) +// DirtiesCount is a test helper to inspect how many entries in the journal are +// still dirty (uncommitted). After calling [StateDB.Commit], this function should +// return zero. +func (s *StateDB) DirtiesCount() int { + dirtiesCount := 0 + for _, dirtyCount := range s.Journal.dirties { + dirtiesCount += dirtyCount + } + return dirtiesCount + // for addr := range s.Journal.dirties { + // obj := s.stateObjects[addr] + // // suicided without deletion means obj is dirty + // if obj.Suicided { + // dirtiesCount++ + // // continue + // } + // // dirty code means obj is dirty + // if obj.code != nil && obj.DirtyCode { + // dirtiesCount++ + // // continue + // } + + // // mismatch between dirty storage and origin means obj is dirty + // for k, v := range obj.DirtyStorage { + // // All object (k,v) tuples matching between dirty and origin storage + // // signifies that the entry is committed. + // if v != obj.OriginStorage[k] { + // dirtiesCount++ + // } + // } + // } + // return dirtiesCount +} + +func (s *StateDB) Dirties() map[common.Address]int { + return s.Journal.dirties +} + +func (s *StateDB) Entries() []JournalChange { + return s.Journal.entries } // ------------------------------------------------------ @@ -148,7 +186,7 @@ var _ JournalChange = suicideChange{} func (ch suicideChange) Revert(s *StateDB) { obj := s.getStateObject(*ch.account) if obj != nil { - obj.suicided = ch.prev + obj.Suicided = ch.prev obj.setBalance(ch.prevbalance) } } diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index 0aadf3c53..b7894bb98 100644 --- a/x/evm/statedb/journal_test.go +++ b/x/evm/statedb/journal_test.go @@ -3,10 +3,14 @@ package statedb_test import ( "fmt" "math/big" + "strings" + "testing" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/core/vm" + serverconfig "github.com/NibiruChain/nibiru/v2/app/server/config" + "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" @@ -50,8 +54,6 @@ func (s *Suite) TestPrecompileSnapshots() { assertionsBeforeRun(&deps) - s.T().Log("Populate dirty journal entries") - deployArgs := []any{"name", "SYMBOL", uint8(18)} deployResp, err := evmtest.DeployContract( &deps, @@ -60,34 +62,120 @@ func (s *Suite) TestPrecompileSnapshots() { ) s.Require().NoError(err, deployResp) - // deps.EvmKeeper.ERC20().Mint() contract := deployResp.ContractAddr _, evmObj, err := deps.EvmKeeper.ERC20().Mint( - contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, + contract, deps.Sender.EthAddr, deps.Sender.EthAddr, big.NewInt(69_420), deps.Ctx, ) s.Require().NoError(err) - stateDB := evmObj.StateDB.(*statedb.StateDB) - s.Equal(50, stateDB.Journal.DirtiesLen()) - // evmtest.TransferWei() - - s.T().Log("Run state transition") - - evmObj = run(&deps) - stateDB, ok := evmObj.StateDB.(*statedb.StateDB) - s.Require().True(ok, "error retrieving StateDB from the EVM") - s.Equal(0, stateDB.Journal.DirtiesLen()) - - _, _ = deps.Ctx.CacheContext() - assertionsAfterRun(&deps) - err = stateDB.Commit() - s.NoError(err) - assertionsAfterRun(&deps) + s.Run("Populate dirty journal entries. Remove with Commit", func() { + stateDB := evmObj.StateDB.(*statedb.StateDB) + s.Equal(0, stateDB.DirtiesCount()) + + randomAcc := evmtest.NewEthPrivAcc().EthAddr + balDelta := evm.NativeToWei(big.NewInt(4)) + // 2 dirties from [createObjectChange, balanceChange] + stateDB.AddBalance(randomAcc, balDelta) + // 1 dirties from [balanceChange] + stateDB.AddBalance(randomAcc, balDelta) + // 1 dirties from [balanceChange] + stateDB.SubBalance(randomAcc, balDelta) + if stateDB.DirtiesCount() != 4 { + debugDirtiesCountMismatch(stateDB, s.T()) + s.FailNow("expected 4 dirty journal changes") + } + + err = stateDB.Commit() // Dirties should be gone + s.NoError(err) + if stateDB.DirtiesCount() != 0 { + debugDirtiesCountMismatch(stateDB, s.T()) + s.FailNow("expected 0 dirty journal changes") + } + }) + + s.Run("Emulate a contract that calls another contract", func() { + randomAcc := evmtest.NewEthPrivAcc().EthAddr + to, amount := randomAcc, big.NewInt(69_000) + input, err := embeds.SmartContract_ERC20Minter.ABI.Pack("transfer", to, amount) + s.Require().NoError(err) + + leftoverGas := serverconfig.DefaultEthCallGasLimit + _, _, err = evmObj.Call( + vm.AccountRef(deps.Sender.EthAddr), + contract, + input, + leftoverGas, + big.NewInt(0), + ) + s.Require().NoError(err) + stateDB := evmObj.StateDB.(*statedb.StateDB) + if stateDB.DirtiesCount() != 2 { + debugDirtiesCountMismatch(stateDB, s.T()) + s.FailNow("expected 2 dirty journal changes") + } + + // The contract calling itself is invalid in this context. + // Note the comment in vm.Contract: + // + // type Contract struct { + // // CallerAddress is the result of the caller which initialized this + // // contract. However when the "call method" is delegated this value + // // needs to be initialized to that of the caller's caller. + // CallerAddress common.Address + // // ... + // } + // // + _, _, err = evmObj.Call( + vm.AccountRef(contract), + contract, + input, + leftoverGas, + big.NewInt(0), + ) + s.Require().ErrorContains(err, vm.ErrExecutionReverted.Error()) + }) + + s.Run("Precompile calls also start and end clean (no dirty changes)", func() { + evmObj = run(&deps) + assertionsAfterRun(&deps) + stateDB, ok := evmObj.StateDB.(*statedb.StateDB) + s.Require().True(ok, "error retrieving StateDB from the EVM") + if stateDB.DirtiesCount() != 0 { + debugDirtiesCountMismatch(stateDB, s.T()) + s.FailNow("expected 0 dirty journal changes") + } + }) +} - s.Equal(0, stateDB.Journal.DirtiesLen()) +func debugDirtiesCountMismatch(db *statedb.StateDB, t *testing.T) string { + lines := []string{} + dirties := db.Dirties() + stateObjects := db.StateObjects() + for addr, dirtyCountForAddr := range dirties { + lines = append(lines, fmt.Sprintf("Dirty addr: %s, dirtyCountForAddr=%d", addr, dirtyCountForAddr)) + + // Inspect the actual state object + maybeObj := stateObjects[addr] + if maybeObj == nil { + lines = append(lines, " no state object found!") + continue + } + obj := *maybeObj + + lines = append(lines, fmt.Sprintf(" balance: %s", obj.Balance())) + lines = append(lines, fmt.Sprintf(" suicided: %v", obj.Suicided)) + lines = append(lines, fmt.Sprintf(" dirtyCode: %v", obj.DirtyCode)) + + // Print storage state + lines = append(lines, fmt.Sprintf(" len(obj.DirtyStorage) entries: %d", len(obj.DirtyStorage))) + for k, v := range obj.DirtyStorage { + lines = append(lines, fmt.Sprintf(" key: %s, value: %s", k.Hex(), v.Hex())) + origVal := obj.OriginStorage[k] + lines = append(lines, fmt.Sprintf(" origin value: %s", origVal.Hex())) + } + } - // s.Require().EqualValues(ctxBefore, deps.Ctx, - // "StateDB should have been committed by the precompile", - // ) + t.Log("debugDirtiesCountMismatch:\n", strings.Join(lines, "\n")) + return "" } diff --git a/x/evm/statedb/state_object.go b/x/evm/statedb/state_object.go index ce9d21f5b..e371beae0 100644 --- a/x/evm/statedb/state_object.go +++ b/x/evm/statedb/state_object.go @@ -115,14 +115,14 @@ type stateObject struct { code []byte // state storage - originStorage Storage - dirtyStorage Storage + OriginStorage Storage + DirtyStorage Storage address common.Address // flags - dirtyCode bool - suicided bool + DirtyCode bool + Suicided bool } // newObject creates a state object. @@ -138,8 +138,8 @@ func newObject(db *StateDB, address common.Address, account Account) *stateObjec address: address, // Reflect the micronibi (unibi) balance in wei account: account.ToWei(), - originStorage: make(Storage), - dirtyStorage: make(Storage), + OriginStorage: make(Storage), + DirtyStorage: make(Storage), } } @@ -223,7 +223,7 @@ func (s *stateObject) SetCode(codeHash common.Hash, code []byte) { func (s *stateObject) setCode(codeHash common.Hash, code []byte) { s.code = code s.account.CodeHash = codeHash[:] - s.dirtyCode = true + s.DirtyCode = true } // SetNonce set nonce to account @@ -256,18 +256,18 @@ func (s *stateObject) Nonce() uint64 { // GetCommittedState query the committed state func (s *stateObject) GetCommittedState(key common.Hash) common.Hash { - if value, cached := s.originStorage[key]; cached { + if value, cached := s.OriginStorage[key]; cached { return value } // If no live objects are available, load it from keeper value := s.db.keeper.GetState(s.db.ctx, s.Address(), key) - s.originStorage[key] = value + s.OriginStorage[key] = value return value } // GetState query the current state (including dirty state) func (s *stateObject) GetState(key common.Hash) common.Hash { - if value, dirty := s.dirtyStorage[key]; dirty { + if value, dirty := s.DirtyStorage[key]; dirty { return value } return s.GetCommittedState(key) @@ -290,5 +290,5 @@ func (s *stateObject) SetState(key common.Hash, value common.Hash) { } func (s *stateObject) setState(key, value common.Hash) { - s.dirtyStorage[key] = value + s.DirtyStorage[key] = value } diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index ab33cc4ea..a29371431 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -194,7 +194,7 @@ func (s *StateDB) GetRefund() uint64 { func (s *StateDB) HasSuicided(addr common.Address) bool { stateObject := s.getStateObject(addr) if stateObject != nil { - return stateObject.suicided + return stateObject.Suicided } return false } @@ -275,7 +275,7 @@ func (s *StateDB) ForEachStorage(addr common.Address, cb func(key, value common. return nil } s.keeper.ForEachStorage(s.ctx, addr, func(key, value common.Hash) bool { - if value, dirty := so.dirtyStorage[key]; dirty { + if value, dirty := so.DirtyStorage[key]; dirty { return cb(key, value) } if len(value) > 0 { @@ -346,10 +346,10 @@ func (s *StateDB) Suicide(addr common.Address) bool { } s.Journal.append(suicideChange{ account: &addr, - prev: stateObject.suicided, + prev: stateObject.Suicided, prevbalance: new(big.Int).Set(stateObject.Balance()), }) - stateObject.suicided = true + stateObject.Suicided = true stateObject.account.BalanceWei = new(big.Int) return true @@ -450,43 +450,49 @@ func errorf(format string, args ...any) error { return fmt.Errorf("StateDB error: "+format, args...) } -// CommitContext writes the dirty journal state changes to the EVM Keeper. The -// StateDB object cannot be reused after [CommitContext] has completed. A new +// Commit writes the dirty journal state changes to the EVM Keeper. The +// StateDB object cannot be reused after [Commit] has completed. A new // object needs to be created from the EVM. -func (s *StateDB) CommitContext(ctx sdk.Context) error { +func (s *StateDB) Commit() error { + ctx := s.GetContext() for _, addr := range s.Journal.sortedDirties() { - obj := s.stateObjects[addr] - if obj.suicided { + obj := s.getStateObject(addr) + if obj == nil { + continue + } + if obj.Suicided { + // Invariant: After [StateDB.Suicide] for some address, the + // corresponding account's state object is only available until the + // state is committed. if err := s.keeper.DeleteAccount(ctx, obj.Address()); err != nil { return errorf("failed to delete account: %w", err) } + delete(s.stateObjects, addr) } else { - if obj.code != nil && obj.dirtyCode { + if obj.code != nil && obj.DirtyCode { s.keeper.SetCode(ctx, obj.CodeHash(), obj.code) } if err := s.keeper.SetAccount(ctx, obj.Address(), obj.account.ToNative()); err != nil { return errorf("failed to set account: %w", err) } - for _, key := range obj.dirtyStorage.SortedKeys() { - value := obj.dirtyStorage[key] - // Skip noop changes, persist actual changes - if value == obj.originStorage[key] { + for _, key := range obj.DirtyStorage.SortedKeys() { + dirtyVal := obj.DirtyStorage[key] + // Values that match origin storage are not dirty. + if dirtyVal == obj.OriginStorage[key] { continue } - s.keeper.SetState(ctx, obj.Address(), key, value.Bytes()) + // Persist committed changes + s.keeper.SetState(ctx, obj.Address(), key, dirtyVal.Bytes()) + obj.OriginStorage[key] = dirtyVal } } + // Clear the dirty counts because all state changes have been + // committed. + s.Journal.dirties[addr] = 0 } return nil } -// Commit writes the dirty journal state changes to the EVM Keeper. The -// StateDB object cannot be reused after [CommitContext] has completed. A new -// object needs to be created from the EVM. -func (s *StateDB) Commit() error { - return s.CommitContext(s.ctx) -} - // StateObjects: Returns a copy of the [StateDB.stateObjects] map. func (s *StateDB) StateObjects() map[common.Address]*stateObject { copyOfMap := make(map[common.Address]*stateObject) From 0e6afd8e12d84998efff135c529f2a532fa6390d Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Thu, 24 Oct 2024 05:17:27 -0500 Subject: [PATCH 11/12] fix(precompile-funtoken): modify StateDB after bank send operations --- x/evm/precompile/funtoken.go | 68 +++++++++++++++++++++++++++++++++-- x/evm/statedb/journal.go | 23 ------------ x/evm/statedb/journal_test.go | 8 ++--- x/evm/statedb/statedb.go | 15 +++++--- 4 files changed, 79 insertions(+), 35 deletions(-) diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 3b453d597..610ccab05 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -7,15 +7,18 @@ import ( "cosmossdk.io/math" 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" gethabi "github.com/ethereum/go-ethereum/accounts/abi" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" "github.com/NibiruChain/nibiru/v2/app/keepers" + "github.com/NibiruChain/nibiru/v2/eth" "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper" + "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) var _ vm.PrecompiledContract = (*precompileFunToken)(nil) @@ -148,7 +151,7 @@ func (p precompileFunToken) bankSend( // EVM account mints FunToken.BankDenom to module account amt := math.NewIntFromBigInt(amount) - coins := sdk.NewCoins(sdk.NewCoin(funtoken.BankDenom, amt)) + coinToSend := sdk.NewCoin(funtoken.BankDenom, amt) if funtoken.IsMadeFromCoin { // If the FunToken mapping was created from a bank coin, then the EVM account // owns the ERC20 contract and was the original minter of the ERC20 tokens. @@ -160,7 +163,7 @@ func (p precompileFunToken) bankSend( return } } else { - err = p.bankKeeper.MintCoins(ctx, evm.ModuleName, coins) + err = SafeMintCoins(ctx, evm.ModuleName, coinToSend, p.bankKeeper, start.StateDB) if err != nil { return nil, fmt.Errorf("mint failed for module \"%s\" (%s): contract caller %s: %w", evm.ModuleName, evm.EVM_MODULE_ADDRESS.Hex(), caller.Hex(), err, @@ -169,7 +172,14 @@ func (p precompileFunToken) bankSend( } // Transfer the bank coin - err = p.bankKeeper.SendCoinsFromModuleToAccount(ctx, evm.ModuleName, toAddr, coins) + err = SafeSendCoinFromModuleToAccount( + ctx, + evm.ModuleName, + toAddr, + coinToSend, + p.bankKeeper, + start.StateDB, + ) if err != nil { return nil, fmt.Errorf("send failed for module \"%s\" (%s): contract caller %s: %w", evm.ModuleName, evm.EVM_MODULE_ADDRESS.Hex(), caller.Hex(), err, @@ -181,6 +191,58 @@ func (p precompileFunToken) bankSend( return method.Outputs.Pack() } +func SafeMintCoins( + ctx sdk.Context, + moduleName string, + amt sdk.Coin, + bk bankkeeper.Keeper, + db *statedb.StateDB, +) error { + err := bk.MintCoins(ctx, evm.ModuleName, sdk.NewCoins(amt)) + if err != nil { + return err + } + if amt.Denom == evm.EVMBankDenom { + evmBech32Addr := auth.NewModuleAddress(evm.ModuleName) + balAfter := bk.GetBalance(ctx, evmBech32Addr, amt.Denom).Amount.BigInt() + db.SetBalanceWei( + evm.EVM_MODULE_ADDRESS, + evm.NativeToWei(balAfter), + ) + } + + return nil +} + +func SafeSendCoinFromModuleToAccount( + ctx sdk.Context, + senderModule string, + recipientAddr sdk.AccAddress, + amt sdk.Coin, + bk bankkeeper.Keeper, + db *statedb.StateDB, +) error { + err := bk.SendCoinsFromModuleToAccount(ctx, senderModule, recipientAddr, sdk.NewCoins(amt)) + if err != nil { + return err + } + if amt.Denom == evm.EVMBankDenom { + evmBech32Addr := auth.NewModuleAddress(evm.ModuleName) + balAfterFrom := bk.GetBalance(ctx, evmBech32Addr, amt.Denom).Amount.BigInt() + db.SetBalanceWei( + evm.EVM_MODULE_ADDRESS, + evm.NativeToWei(balAfterFrom), + ) + + balAfterTo := bk.GetBalance(ctx, recipientAddr, amt.Denom).Amount.BigInt() + db.SetBalanceWei( + eth.NibiruAddrToEthAddr(recipientAddr), + evm.NativeToWei(balAfterTo), + ) + } + return nil +} + func (p precompileFunToken) decomposeBankSendArgs(args []any) ( erc20 gethcommon.Address, amount *big.Int, diff --git a/x/evm/statedb/journal.go b/x/evm/statedb/journal.go index 2a6ffe953..ac041b617 100644 --- a/x/evm/statedb/journal.go +++ b/x/evm/statedb/journal.go @@ -100,29 +100,6 @@ func (s *StateDB) DirtiesCount() int { dirtiesCount += dirtyCount } return dirtiesCount - // for addr := range s.Journal.dirties { - // obj := s.stateObjects[addr] - // // suicided without deletion means obj is dirty - // if obj.Suicided { - // dirtiesCount++ - // // continue - // } - // // dirty code means obj is dirty - // if obj.code != nil && obj.DirtyCode { - // dirtiesCount++ - // // continue - // } - - // // mismatch between dirty storage and origin means obj is dirty - // for k, v := range obj.DirtyStorage { - // // All object (k,v) tuples matching between dirty and origin storage - // // signifies that the entry is committed. - // if v != obj.OriginStorage[k] { - // dirtiesCount++ - // } - // } - // } - // return dirtiesCount } func (s *StateDB) Dirties() map[common.Address]int { diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index b7894bb98..2c19af77f 100644 --- a/x/evm/statedb/journal_test.go +++ b/x/evm/statedb/journal_test.go @@ -10,13 +10,11 @@ import ( "github.com/ethereum/go-ethereum/core/vm" serverconfig "github.com/NibiruChain/nibiru/v2/app/server/config" - "github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp" "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" "github.com/NibiruChain/nibiru/v2/x/evm/evmtest" "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" - precompiletest "github.com/NibiruChain/nibiru/v2/x/evm/precompile/test" "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) @@ -32,10 +30,10 @@ func (s *Suite) TestPrecompileSnapshots() { s.T().Log("Set up helloworldcounter.wasm") - wasmContract := precompiletest.SetupWasmContracts(&deps, &s.Suite)[1] + wasmContract := test.SetupWasmContracts(&deps, &s.Suite)[1] fmt.Printf("wasmContract: %s\n", wasmContract) assertionsBeforeRun := func(deps *evmtest.TestDeps) { - precompiletest.AssertWasmCounterState( + test.AssertWasmCounterState( &s.Suite, *deps, wasmContract, 0, ) } @@ -45,7 +43,7 @@ func (s *Suite) TestPrecompileSnapshots() { ) } assertionsAfterRun := func(deps *evmtest.TestDeps) { - precompiletest.AssertWasmCounterState( + test.AssertWasmCounterState( &s.Suite, *deps, wasmContract, 7, ) } diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index a29371431..223e92edb 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -295,18 +295,25 @@ func (s *StateDB) setStateObject(object *stateObject) { */ // AddBalance adds amount to the account associated with addr. -func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) { +func (s *StateDB) AddBalance(addr common.Address, wei *big.Int) { stateObject := s.getOrNewStateObject(addr) if stateObject != nil { - stateObject.AddBalance(amount) + stateObject.AddBalance(wei) } } // SubBalance subtracts amount from the account associated with addr. -func (s *StateDB) SubBalance(addr common.Address, amount *big.Int) { +func (s *StateDB) SubBalance(addr common.Address, wei *big.Int) { stateObject := s.getOrNewStateObject(addr) if stateObject != nil { - stateObject.SubBalance(amount) + stateObject.SubBalance(wei) + } +} + +func (s *StateDB) SetBalanceWei(addr common.Address, wei *big.Int) { + stateObject := s.getOrNewStateObject(addr) + if stateObject != nil { + stateObject.SetBalance(wei) } } From 65b2cd61bd20dd6dde9a26ab8507318900c8915c Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Thu, 24 Oct 2024 09:39:06 -0500 Subject: [PATCH 12/12] PR feedback --- x/evm/evmmodule/genesis_test.go | 4 ++-- x/evm/keeper/erc20.go | 25 +++++++++++-------------- x/evm/keeper/erc20_test.go | 8 ++++---- x/evm/keeper/msg_server.go | 2 +- x/evm/keeper/vm_config.go | 12 ++++++------ x/evm/precompile/funtoken.go | 2 +- x/evm/statedb/config.go | 14 ++------------ x/evm/statedb/journal_test.go | 8 +++++--- x/evm/statedb/statedb_test.go | 11 ++++++----- 9 files changed, 38 insertions(+), 48 deletions(-) diff --git a/x/evm/evmmodule/genesis_test.go b/x/evm/evmmodule/genesis_test.go index ff19ef46f..690745e84 100644 --- a/x/evm/evmmodule/genesis_test.go +++ b/x/evm/evmmodule/genesis_test.go @@ -54,11 +54,11 @@ func (s *Suite) TestExportInitGenesis() { s.Require().NoError(err) // Transfer ERC-20 tokens to user A - _, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx) + _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx) s.Require().NoError(err) // Transfer ERC-20 tokens to user B - _, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx) + _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx) s.Require().NoError(err) // Create fungible token from bank coin diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index 59c98b628..10404bea4 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -16,7 +16,6 @@ import ( "github.com/NibiruChain/nibiru/v2/x/evm" "github.com/NibiruChain/nibiru/v2/x/evm/embeds" - "github.com/NibiruChain/nibiru/v2/x/evm/statedb" ) // ERC20 returns a mutable reference to the keeper with an ERC20 contract ABI and @@ -52,12 +51,13 @@ See [nibiru/x/evm/embeds]. func (e erc20Calls) Mint( contract, from, to gethcommon.Address, amount *big.Int, ctx sdk.Context, -) (evmResp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) { +) (evmResp *evm.MsgEthereumTxResponse, err error) { input, err := e.ABI.Pack("mint", to, amount) if err != nil { - return nil, nil, fmt.Errorf("failed to pack ABI args: %w", err) + return nil, fmt.Errorf("failed to pack ABI args: %w", err) } - return e.CallContractWithInput(ctx, from, &contract, true, input) + evmResp, _, err = e.CallContractWithInput(ctx, from, &contract, true, input) + return evmResp, err } /* @@ -73,23 +73,23 @@ Transfer implements "ERC20.transfer" func (e erc20Calls) Transfer( contract, from, to gethcommon.Address, amount *big.Int, ctx sdk.Context, -) (out bool, evmObj *vm.EVM, err error) { +) (out bool, err error) { input, err := e.ABI.Pack("transfer", to, amount) if err != nil { - return false, nil, fmt.Errorf("failed to pack ABI args: %w", err) + return false, fmt.Errorf("failed to pack ABI args: %w", err) } - resp, evmObj, err := e.CallContractWithInput(ctx, from, &contract, true, input) + resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input) if err != nil { - return false, nil, err + return false, err } var erc20Bool ERC20Bool err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret) if err != nil { - return false, nil, err + return false, err } - return erc20Bool.Value, evmObj, nil + return erc20Bool.Value, nil } // BalanceOf retrieves the balance of an ERC20 token for a specific account. @@ -216,10 +216,7 @@ func (k Keeper) CallContractWithInput( // Generating TxConfig with an empty tx hash as there is no actual eth tx // sent by a user - blockHash := gethcommon.BytesToHash(ctx.HeaderHash()) - txConfig := statedb.NewEmptyTxConfig(blockHash) - txConfig.TxIndex = uint(k.EvmState.BlockTxIndex.GetOr(ctx, 0)) - txConfig.LogIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)) + txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0))) // Using tmp context to not modify the state in case of evm revert tmpCtx, commitCtx := ctx.CacheContext() diff --git a/x/evm/keeper/erc20_test.go b/x/evm/keeper/erc20_test.go index 15807b714..4b1dc10fa 100644 --- a/x/evm/keeper/erc20_test.go +++ b/x/evm/keeper/erc20_test.go @@ -16,7 +16,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Mint tokens - Fail from non-owner") { - _, _, err := deps.EvmKeeper.ERC20().Mint( + _, err := deps.EvmKeeper.ERC20().Mint( contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx, ) @@ -25,7 +25,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Mint tokens - Success") { - _, _, err := deps.EvmKeeper.ERC20().Mint(contract, evm.EVM_MODULE_ADDRESS, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx) + _, err := deps.EvmKeeper.ERC20().Mint(contract, evm.EVM_MODULE_ADDRESS, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), deps.Ctx) s.Require().NoError(err) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0)) @@ -34,7 +34,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Transfer - Not enough funds") { - _, _, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx) + _, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx) s.ErrorContains(err, "ERC20: transfer amount exceeds balance") // balances unchanged evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0)) @@ -43,7 +43,7 @@ func (s *Suite) TestERC20Calls() { s.T().Log("Transfer - Success (sanity check)") { - _, _, err := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx) + _, err := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx) s.Require().NoError(err) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(9_420)) evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000)) diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 5c52acc0f..89a249dd3 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -590,7 +590,7 @@ func (k Keeper) convertCoinNativeERC20( } // unescrow ERC-20 tokens from EVM module address - res, _, err := k.ERC20().Transfer( + res, err := k.ERC20().Transfer( erc20Addr, evm.EVM_MODULE_ADDRESS, recipient, diff --git a/x/evm/keeper/vm_config.go b/x/evm/keeper/vm_config.go index 241cae816..2f8232ad6 100644 --- a/x/evm/keeper/vm_config.go +++ b/x/evm/keeper/vm_config.go @@ -39,12 +39,12 @@ func (k *Keeper) GetEVMConfig( func (k *Keeper) TxConfig( ctx sdk.Context, txHash common.Hash, ) statedb.TxConfig { - return statedb.NewTxConfig( - common.BytesToHash(ctx.HeaderHash()), // BlockHash - txHash, // TxHash - uint(k.EvmState.BlockTxIndex.GetOr(ctx, 0)), // TxIndex - uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)), // LogIndex - ) + return statedb.TxConfig{ + BlockHash: common.BytesToHash(ctx.HeaderHash()), + TxHash: txHash, + TxIndex: uint(k.EvmState.BlockTxIndex.GetOr(ctx, 0)), + LogIndex: uint(k.EvmState.BlockLogSize.GetOr(ctx, 0)), + } } // VMConfig creates an EVM configuration from the debug setting and the extra diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 610ccab05..5c585c2e9 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -144,7 +144,7 @@ func (p precompileFunToken) bankSend( // Caller transfers ERC20 to the EVM account transferTo := evm.EVM_MODULE_ADDRESS - _, _, err = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx) + _, err = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx) if err != nil { return nil, fmt.Errorf("failed to send from caller to the EVM account: %w", err) } diff --git a/x/evm/statedb/config.go b/x/evm/statedb/config.go index c75cdc179..887f591c5 100644 --- a/x/evm/statedb/config.go +++ b/x/evm/statedb/config.go @@ -18,21 +18,11 @@ type TxConfig struct { LogIndex uint // the index of next log within current block } -// NewTxConfig returns a TxConfig -func NewTxConfig(bhash, thash gethcommon.Hash, txIndex, logIndex uint) TxConfig { - return TxConfig{ - BlockHash: bhash, - TxHash: thash, - TxIndex: txIndex, - LogIndex: logIndex, - } -} - // NewEmptyTxConfig construct an empty TxConfig, // used in context where there's no transaction, e.g. `eth_call`/`eth_estimateGas`. -func NewEmptyTxConfig(bhash gethcommon.Hash) TxConfig { +func NewEmptyTxConfig(blockHash gethcommon.Hash) TxConfig { return TxConfig{ - BlockHash: bhash, + BlockHash: blockHash, TxHash: gethcommon.Hash{}, TxIndex: 0, LogIndex: 0, diff --git a/x/evm/statedb/journal_test.go b/x/evm/statedb/journal_test.go index 2c19af77f..5863face5 100644 --- a/x/evm/statedb/journal_test.go +++ b/x/evm/statedb/journal_test.go @@ -61,9 +61,11 @@ func (s *Suite) TestPrecompileSnapshots() { s.Require().NoError(err, deployResp) contract := deployResp.ContractAddr - _, evmObj, err := deps.EvmKeeper.ERC20().Mint( - contract, deps.Sender.EthAddr, deps.Sender.EthAddr, - big.NewInt(69_420), deps.Ctx, + to, amount := deps.Sender.EthAddr, big.NewInt(69_420) + input, err := deps.EvmKeeper.ERC20().ABI.Pack("mint", to, amount) + s.Require().NoError(err) + _, evmObj, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, deps.Sender.EthAddr, &contract, true, input, ) s.Require().NoError(err) diff --git a/x/evm/statedb/statedb_test.go b/x/evm/statedb/statedb_test.go index b8b4d1741..7919d3da0 100644 --- a/x/evm/statedb/statedb_test.go +++ b/x/evm/statedb/statedb_test.go @@ -513,11 +513,12 @@ func (s *Suite) TestLog() { txIdx = uint(1) logIdx = uint(1) ) - txConfig := statedb.NewTxConfig( - blockHash, - txHash, - txIdx, logIdx, - ) + txConfig := statedb.TxConfig{ + BlockHash: blockHash, + TxHash: txHash, + TxIndex: txIdx, + LogIndex: logIdx, + } deps := evmtest.NewTestDeps() db := statedb.New(deps.Ctx, &deps.App.EvmKeeper, txConfig)