Skip to content

Commit

Permalink
feat: Prototype to align account numbers behavior with legacy statedb
Browse files Browse the repository at this point in the history
  • Loading branch information
drklee3 committed Mar 13, 2024
1 parent 9b2d90a commit 6958d5a
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 4 deletions.
4 changes: 1 addition & 3 deletions x/evm/keeper/state_transition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand Down
55 changes: 55 additions & 0 deletions x/evm/keeper/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
package keeper

import (
"bytes"
"fmt"
"math/big"
"sort"

sdkmath "cosmossdk.io/math"

Expand Down Expand Up @@ -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) {

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion Error

Potential integer overflow by integer type conversion
return fmt.Errorf(
"account number mismatch: expected %d, got %d",
accountNumberStart+uint64(i), acct.GetAccountNumber(),

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion Error

Potential integer overflow by integer type conversion
)
}
}

// 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))

Check failure on line 220 in x/evm/keeper/statedb.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint

Error return value of `ethAcct.SetAccountNumber` is not checked (errcheck)

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion Error

Potential integer overflow by integer type conversion

Check warning

Code scanning / gosec

Errors unhandled. Warning

Errors unhandled.
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))
Expand Down
2 changes: 1 addition & 1 deletion x/evm/statedb/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions x/evm/statedb/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions x/evm/statedb/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
2 changes: 2 additions & 0 deletions x/evm/statedb/statedb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions x/evm/statedb/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type StoreRevertKey struct {
SuicidedAccountsIndex int
LogsIndex int
ContractStatesIndex int
CreatedAccountsIndex int
}

// ContractStateKey represents the state key of a contract.
Expand All @@ -28,6 +29,7 @@ type EphemeralStore struct {
SuicidedAccountStates []common.Address
Logs []*ethtypes.Log
ContractStates []ContractStateKey
CreatedAccounts []common.Address
}

// NewEphemeralStore creates a new EphemeralStore.
Expand All @@ -37,6 +39,7 @@ func NewEphemeralStore() *EphemeralStore {
SuicidedAccountStates: nil,
Logs: nil,
ContractStates: nil,
CreatedAccounts: nil,
}
}

Expand All @@ -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),
}
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

0 comments on commit 6958d5a

Please sign in to comment.