Skip to content

Commit

Permalink
Remove account number reassignment
Browse files Browse the repository at this point in the history
without the goal of state compatibility, removes extra complexity and unneeded logic
  • Loading branch information
drklee3 committed Apr 15, 2024
1 parent e94221d commit 63f9a48
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 106 deletions.
1 change: 0 additions & 1 deletion x/evm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ func (k *Keeper) GetAccountWithoutBalance(ctx sdk.Context, addr common.Address)
Nonce: acct.GetSequence(),
CodeHash: codeHash,
// Balance: nil,
AccountNumber: acct.GetAccountNumber(),
}
}

Expand Down
11 changes: 2 additions & 9 deletions x/evm/keeper/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,8 @@ func (k *Keeper) SetBalance(ctx sdk.Context, addr common.Address, amount *big.In

func (k *Keeper) SetAccountLegacy(ctx sdk.Context, addr common.Address, account legacystatedb.Account) error {
if err := k.SetAccount(ctx, addr, statedb.Account{
Nonce: account.Nonce,
CodeHash: account.CodeHash,
AccountNumber: 0,
Nonce: account.Nonce,
CodeHash: account.CodeHash,
}); err != nil {
return err
}
Expand Down Expand Up @@ -170,12 +169,6 @@ func (k *Keeper) SetAccount(ctx sdk.Context, addr common.Address, account stated
if err := ethAcct.SetCodeHash(codeHash); err != nil {
return err
}

if account.AccountNumber != 0 {
if err := ethAcct.SetAccountNumber(account.AccountNumber); err != nil {
return err
}
}
}

if !ok && account.IsContract() {
Expand Down
48 changes: 40 additions & 8 deletions x/evm/keeper/statedb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,49 +242,81 @@ func (suite *KeeperTestSuite) TestSetAccount() {
{
"new account, non-contract account",
tests.GenerateAddress(),
statedb.Account{10, types.EmptyCodeHash, 1},
statedb.Account{
Nonce: 10,
// Balance:,
CodeHash: types.EmptyCodeHash,
},
nil,
},
{
"new account, contract account",
tests.GenerateAddress(),
statedb.Account{10 /* big.NewInt(100),*/, crypto.Keccak256Hash([]byte("some code hash")).Bytes(), 0},
statedb.Account{
Nonce: 10,
// Balance: big.NewInt(100)
CodeHash: crypto.Keccak256Hash([]byte("some code hash")).Bytes(),
},
nil,
},
{
"existing eth account, non-contract account",
ethAddr,
statedb.Account{10 /* big.NewInt(1),*/, types.EmptyCodeHash, 0},
statedb.Account{
Nonce: 10,
// Balance: big.NewInt(1),
CodeHash: types.EmptyCodeHash,
},
nil,
},
{
"existing eth account, contract account",
ethAddr,
statedb.Account{10 /* big.NewInt(0),*/, crypto.Keccak256Hash([]byte("some code hash")).Bytes(), 0},
statedb.Account{
Nonce: 10,
// Balance: /* big.NewInt(0),*/,
CodeHash: crypto.Keccak256Hash([]byte("some code hash")).Bytes(),
},
nil,
},
{
"existing base account, non-contract account",
baseAddr,
statedb.Account{10 /* big.NewInt(10),*/, types.EmptyCodeHash, 0},
statedb.Account{
Nonce: 10,
// Balance: /* big.NewInt(10),*/,
CodeHash: types.EmptyCodeHash,
},
nil,
},
{
"existing base account, contract account",
baseAddr,
statedb.Account{10 /* big.NewInt(99),*/, crypto.Keccak256Hash([]byte("some code hash")).Bytes(), 0},
statedb.Account{
Nonce: 10,
// Balance: /* big.NewInt(99),*/,
CodeHash: crypto.Keccak256Hash([]byte("some code hash")).Bytes(),
},
nil,
},
{
"existing vesting account, non-contract account",
vestingAddr,
statedb.Account{10 /* big.NewInt(1000),*/, types.EmptyCodeHash, 0},
statedb.Account{
Nonce: 10,
// Balance: /* big.NewInt(1000),*/,
CodeHash: types.EmptyCodeHash,
},
nil,
},
{
"existing vesting account, contract account",
vestingAddr,
statedb.Account{10 /* big.NewInt(1001),*/, crypto.Keccak256Hash([]byte("some code hash")).Bytes(), 0},
statedb.Account{
Nonce: 10,
// Balance: /* big.NewInt(1001),*/,
CodeHash: crypto.Keccak256Hash([]byte("some code hash")).Bytes(),
},
types.ErrInvalidAccount,
},
}
Expand Down
34 changes: 2 additions & 32 deletions x/evm/statedb/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ type Account struct {
// Balance is *not* included as it is managed by bank
Nonce uint64
CodeHash []byte

AccountNumber uint64
}

// NewEmptyAccount returns an empty account.
Expand Down Expand Up @@ -79,18 +77,15 @@ type stateObject struct {
address common.Address

// flags
dirtyCode bool
suicided bool
isNew bool // created in this tx
dirtyBalance bool // balance has changed
dirtyCode bool
suicided bool
}

// newObject creates a state object.
func newObject(
db *StateDB,
address common.Address,
account Account,
isNew bool,
) *stateObject {
if account.CodeHash == nil {
account.CodeHash = emptyCodeHash
Expand All @@ -101,8 +96,6 @@ func newObject(
account: account,
originStorage: make(Storage),
dirtyStorage: make(Storage),
isNew: isNew,
dirtyBalance: false,
}
}

Expand All @@ -113,14 +106,6 @@ func (s *stateObject) empty() bool {
bytes.Equal(s.account.CodeHash, emptyCodeHash)
}

// createdByBankTransfer returns true if the account was created by an sdk bank
// transfer, i.e. balance changed for an account that previously did not exist.
// This is used to determine if the account number has been externally assigned
// on balance change and should be re-assigned to match the Commit() order.
func (s *stateObject) createdByBankTransfer() bool {
return s.isNew && s.dirtyBalance
}

func (s *stateObject) markSuicided() {
s.suicided = true
}
Expand Down Expand Up @@ -155,24 +140,9 @@ func (s *stateObject) SetBalance(amount *big.Int) {
}

func (s *stateObject) setBalance(amount *big.Int) {
logger := s.db.ctx.CurrentCtx().Logger().With("module", "statedb")

logger.Info(
"setBalance",
"address",
s.address.Hex(),
"prev_amount", s.Balance(),
"new_amount", amount,
)

if err := s.db.keeper.SetBalance(s.db.ctx.CurrentCtx(), s.address, amount); err != nil {
s.db.SetError(err)
}

// SetBalance will create the account if it doesn't exist yet, so we need to
// mark it as dirty to keep track of which accounts were created outside of
// Commit()
s.dirtyBalance = true
}

//
Expand Down
59 changes: 3 additions & 56 deletions x/evm/statedb/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func (s *StateDB) getStateObject(addr common.Address) *stateObject {
return nil
}
// Insert into the live set
obj := newObject(s, addr, *account, false)
obj := newObject(s, addr, *account)
s.setStateObject(obj)
return obj
}
Expand All @@ -260,7 +260,7 @@ func (s *StateDB) getOrNewStateObject(addr common.Address) *stateObject {
func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) {
prev = s.getStateObject(addr)

newobj = newObject(s, addr, Account{}, true)
newobj = newObject(s, addr, Account{})
if prev == nil {
s.journal.append(createObjectChange{account: &addr})
} else {
Expand Down Expand Up @@ -489,53 +489,7 @@ func (s *StateDB) Commit() error {
return s.sdkError
}

logger := s.ctx.CurrentCtx().Logger().With("module", "statedb")

sortedDirties := s.journal.sortedDirties()

dirtyHex := make([]string, len(sortedDirties))
for i, addr := range sortedDirties {
dirtyHex[i] = addr.Hex()
}

logger.Info(
"committing state changes",
"num_dirties",
len(sortedDirties),
"dirties",
dirtyHex,
)

// Gather all the new account numbers
bankCreatedAcc := make(map[common.Address]bool)
var accNumbers []uint64
for _, addr := range sortedDirties {
obj := s.stateObjects[addr]

// Account was both created AND had a balance change
if obj.createdByBankTransfer() {
accNumber, found := s.keeper.GetAccountNumber(s.ctx.CurrentCtx(), obj.Address())

// The account could be missing if the transfer was less than 1ukava
// as the evmutil handles balances less than 1ukava without calling
// the bank.MintCoins which would create the account.
if !found {
continue
}

bankCreatedAcc[obj.Address()] = true
accNumbers = append(accNumbers, accNumber)
}
}

// Sort ascending account numbers
sort.Slice(accNumbers, func(i, j int) bool {
return accNumbers[i] < accNumbers[j]
})

currentAccNumberIdx := 0

for _, addr := range sortedDirties {
for _, addr := range s.journal.sortedDirties() {
obj := s.stateObjects[addr]
if obj.suicided {
if err := s.keeper.DeleteAccount(s.ctx.CurrentCtx(), obj.Address()); err != nil {
Expand All @@ -546,13 +500,6 @@ func (s *StateDB) Commit() error {
s.keeper.SetCode(s.ctx.CurrentCtx(), obj.CodeHash(), obj.code)
}

// Re-assign account number to the next available number
if bankCreatedAcc[obj.Address()] {
accNumber := accNumbers[currentAccNumberIdx]
currentAccNumberIdx++
obj.account.AccountNumber = accNumber
}

if err := s.keeper.SetAccount(s.ctx.CurrentCtx(), obj.Address(), obj.account); err != nil {
return errorsmod.Wrap(err, "failed to set account")
}
Expand Down

0 comments on commit 63f9a48

Please sign in to comment.