diff --git a/x/evm/keeper/state_transition_test.go b/x/evm/keeper/state_transition_test.go index 4ef3c719fb..ee153de14c 100644 --- a/x/evm/keeper/state_transition_test.go +++ b/x/evm/keeper/state_transition_test.go @@ -933,7 +933,7 @@ func (suite *KeeperTestSuite) TestAccountNumberOrder() { }) } - suite.T().Skip("CacheCtx StateDB does not currently support account number ordering") + /// suite.T().Skip("CacheCtx StateDB does not currently support account number ordering") ctxStateDBConstructor := func() StateDBCommit { return statedb.New(suite.Ctx, suite.App.EvmKeeper, emptyTxConfig) @@ -958,8 +958,6 @@ func (suite *KeeperTestSuite) TestAccountNumberOrder() { } func (suite *KeeperTestSuite) TestNoopStateChange_UnmodifiedIAVLTree() { - // suite.T().Skip("CacheCtx StateDB does not currently skip noop state changes") - // On StateDB.Commit(), if there is a dirty state change that matches the // committed state, it should be skipped. Only state changes that are // different from committed state should be applied to the underlying store. diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 0f4ff48a3f..a212d6a1c9 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -16,8 +16,10 @@ package keeper import ( + "bytes" "fmt" "math/big" + "sort" sdkmath "cosmossdk.io/math" @@ -169,6 +171,59 @@ func (k *Keeper) SetAccount(ctx sdk.Context, addr common.Address, account stated return nil } +func (k *Keeper) ReassignAccountNumbers(ctx sdk.Context, addrs []common.Address) error { + if len(addrs) == 0 { + return nil + } + + accounts := make([]authtypes.AccountI, len(addrs)) + for i, addr := range addrs { + cosmosAddr := sdk.AccAddress(addr.Bytes()) + acct := k.accountKeeper.GetAccount(ctx, cosmosAddr) + if acct == nil { + return fmt.Errorf("account not found: %s", addr) + } + + accounts[i] = acct + } + + // Sort accounts by account number + sort.Slice(accounts, func(i, j int) bool { + return accounts[i].GetAccountNumber() < accounts[j].GetAccountNumber() + }) + + accountNumberStart := accounts[0].GetAccountNumber() + + // Ensure there are no number gaps + for i, acct := range accounts { + if acct.GetAccountNumber() != accountNumberStart+uint64(i) { + return fmt.Errorf( + "account number mismatch: expected %d, got %d", + accountNumberStart+uint64(i), acct.GetAccountNumber(), + ) + } + } + + // Sort accounts by address + sort.Slice(accounts, func(i, j int) bool { + return bytes.Compare(accounts[i].GetAddress(), accounts[j].GetAddress()) < 0 + }) + + // Reassign account numbers + for i, acct := range accounts { + ethAcct, ok := acct.(ethermint.EthAccountI) + if !ok { + return fmt.Errorf("invalid account type: %T", acct) + } + + // set account number + ethAcct.SetAccountNumber(accountNumberStart + uint64(i)) + k.accountKeeper.SetAccount(ctx, acct) + } + + return nil +} + // SetState update contract storage, delete if value is empty. func (k *Keeper) SetState(ctx sdk.Context, addr common.Address, key, value common.Hash) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.AddressStoragePrefix(addr)) diff --git a/x/evm/statedb/ctx.go b/x/evm/statedb/ctx.go index 685221eae0..5fe1bebfbb 100644 --- a/x/evm/statedb/ctx.go +++ b/x/evm/statedb/ctx.go @@ -29,7 +29,7 @@ func NewSnapshotCtx(initialCtx sdk.Context) *SnapshotCommitCtx { // Create an initial snapshot of the initialCtx so no state is written until // Commit() is called. The ID is -1 but disregarded along with the // StoreRevertKey indices as this is only to branch the ctx. - _ = sCtx.Snapshot(0, StoreRevertKey{0, 0, 0, 0}) + _ = sCtx.Snapshot(0, StoreRevertKey{0, 0, 0, 0, 0}) return sCtx } diff --git a/x/evm/statedb/interfaces.go b/x/evm/statedb/interfaces.go index 2fcdcacc4e..0e108160ee 100644 --- a/x/evm/statedb/interfaces.go +++ b/x/evm/statedb/interfaces.go @@ -47,6 +47,8 @@ type Keeper interface { SetState(ctx sdk.Context, addr common.Address, key, value common.Hash) UnsetState(ctx sdk.Context, addr common.Address, key common.Hash) error + ReassignAccountNumbers(ctx sdk.Context, addrs []common.Address) error + SetCode(ctx sdk.Context, codeHash []byte, code []byte) SetBalance(ctx sdk.Context, addr common.Address, amount *big.Int) error DeleteAccount(ctx sdk.Context, addr common.Address) error diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index ef6d419a53..f85dcc3f06 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -202,6 +202,7 @@ func (s *StateDB) getOrNewAccount(addr common.Address) *Account { account := s.keeper.GetAccount(s.ctx.CurrentCtx(), addr) if account == nil { account = NewEmptyAccount() + s.ephemeralStore.AddCreatedAccount(addr) } return account @@ -475,6 +476,13 @@ func (s *StateDB) Commit() error { } } + // Re-assign account numbers for newly created accounts in this tx to be + // ordered by address. + createdAddrs := s.ephemeralStore.GetAllCreatedAccounts() + if err := s.keeper.ReassignAccountNumbers(s.ctx.CurrentCtx(), createdAddrs); err != nil { + return err + } + // Commit after account deletions s.ctx.Commit() diff --git a/x/evm/statedb/statedb_test.go b/x/evm/statedb/statedb_test.go index 3cbedae2bb..518b05f297 100644 --- a/x/evm/statedb/statedb_test.go +++ b/x/evm/statedb/statedb_test.go @@ -12,6 +12,7 @@ import ( "github.com/evmos/ethermint/x/evm/keeper" "github.com/evmos/ethermint/x/evm/statedb" "github.com/evmos/ethermint/x/evm/testutil" + "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/suite" ) @@ -382,6 +383,7 @@ func (suite *StateDBTestSuite) TestRevertSnapshot() { for _, tc := range testCases { suite.Run(tc.name, func() { suite.SetupTest() + suite.App.AccountKeeper.GetModuleAccount(suite.Ctx, types.ModuleName) ctx := suite.Ctx keeper := suite.App.EvmKeeper diff --git a/x/evm/statedb/store.go b/x/evm/statedb/store.go index d18ea0af59..c6d98f72e4 100644 --- a/x/evm/statedb/store.go +++ b/x/evm/statedb/store.go @@ -13,6 +13,7 @@ type StoreRevertKey struct { SuicidedAccountsIndex int LogsIndex int ContractStatesIndex int + CreatedAccountsIndex int } // ContractStateKey represents the state key of a contract. @@ -28,6 +29,7 @@ type EphemeralStore struct { SuicidedAccountStates []common.Address Logs []*ethtypes.Log ContractStates []ContractStateKey + CreatedAccounts []common.Address } // NewEphemeralStore creates a new EphemeralStore. @@ -37,6 +39,7 @@ func NewEphemeralStore() *EphemeralStore { SuicidedAccountStates: nil, Logs: nil, ContractStates: nil, + CreatedAccounts: nil, } } @@ -47,6 +50,7 @@ func (es *EphemeralStore) GetRevertKey() StoreRevertKey { SuicidedAccountsIndex: len(es.SuicidedAccountStates), LogsIndex: len(es.Logs), ContractStatesIndex: len(es.ContractStates), + CreatedAccountsIndex: len(es.CreatedAccounts), } } @@ -60,6 +64,7 @@ func (es *EphemeralStore) Revert(key StoreRevertKey) { es.SuicidedAccountStates = es.SuicidedAccountStates[:key.SuicidedAccountsIndex] es.Logs = es.Logs[:key.LogsIndex] es.ContractStates = es.ContractStates[:key.ContractStatesIndex] + es.CreatedAccounts = es.CreatedAccounts[:key.CreatedAccountsIndex] } func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error { @@ -91,6 +96,13 @@ func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error { ) } + if key.CreatedAccountsIndex > len(es.CreatedAccounts) { + return fmt.Errorf( + "invalid CreatedAccountsIndex, %d is greater than the length of the created accounts (%d)", + key.CreatedAccountsIndex, len(es.CreatedAccounts), + ) + } + return nil } @@ -192,3 +204,30 @@ func (es *EphemeralStore) HasContractStateKey(addr common.Address, key common.Ha func (es *EphemeralStore) GetContractStateKeys() []ContractStateKey { return es.ContractStates } + +// ----------------------------------------------------------------------------- +// Created accounts + +// AddCreatedAccount adds a created account to the store. +func (es *EphemeralStore) AddCreatedAccount(addr common.Address) { + if es.HasCreatedAccount(addr) { + return + } + + es.CreatedAccounts = append(es.CreatedAccounts, addr) +} + +func (es *EphemeralStore) HasCreatedAccount(addr common.Address) bool { + for _, a := range es.CreatedAccounts { + if a == addr { + return true + } + } + + return false +} + +// GetAllCreatedAccounts returns all created accounts. +func (es *EphemeralStore) GetAllCreatedAccounts() []common.Address { + return es.CreatedAccounts +}