From e4b17380ccb2ab9a9c867187d1a85a661582831d Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 09:37:20 +0200 Subject: [PATCH 1/6] evm: use transient store for access list --- x/evm/keeper/statedb.go | 33 +++++++++++++++++++++++---------- x/evm/types/key.go | 12 ++++++++---- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index f63d060aca..9d84e339bf 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -495,35 +495,48 @@ func (k *Keeper) PrepareAccessList(sender common.Address, dest *common.Address, // AddressInAccessList returns true if the address is registered on the access list map. func (k *Keeper) AddressInAccessList(addr common.Address) bool { - return k.accessList.ContainsAddress(addr) + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListAddress) + return ts.Has(addr.Bytes()) } func (k *Keeper) SlotInAccessList(addr common.Address, slot common.Hash) (addressOk bool, slotOk bool) { - return k.accessList.Contains(addr, slot) + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + + addressOk = k.AddressInAccessList(addr) + slotOk = ts.Has(key) + return addressOk, slotOk } // AddAddressToAccessList adds the given address to the access list. This operation is safe to perform // even if the feature/fork is not active yet func (k *Keeper) AddAddressToAccessList(addr common.Address) { - // NOTE: only update the access list during DeliverTx - if k.ctx.IsCheckTx() || k.ctx.IsReCheckTx() { + if k.AddressInAccessList(addr) { return } - // NOTE: ignore change return bool because we don't have to keep a journal for state changes - _ = k.accessList.AddAddress(addr) + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListAddress) + ts.Set(addr.Bytes(), []byte{0x1}) } // AddSlotToAccessList adds the given (address,slot) to the access list. This operation is safe to perform // even if the feature/fork is not active yet func (k *Keeper) AddSlotToAccessList(addr common.Address, slot common.Hash) { - // NOTE: only update the access list during DeliverTx - if k.ctx.IsCheckTx() || k.ctx.IsReCheckTx() { + addrOk, slotOk := k.SlotInAccessList(addr, slot) + + switch { + case !addrOk: + k.AddAddressToAccessList(addr) + fallthrough + case !slotOk: + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + ts.Set(key, []byte{0x1}) + default: + // access list and slot in acccess list return } - // NOTE: ignore change return booleans because we don't have to keep a journal for state changes - _, _ = k.accessList.AddSlot(addr, slot) } // ---------------------------------------------------------------------------- diff --git a/x/evm/types/key.go b/x/evm/types/key.go index dc69ad99b6..1e7d51d891 100644 --- a/x/evm/types/key.go +++ b/x/evm/types/key.go @@ -40,6 +40,8 @@ const ( prefixTransientBloom prefixTransientTxIndex prefixTransientRefund + prefixTransientAccessListAddress + prefixTransientAccessListSlot ) // KVStore key prefixes @@ -55,10 +57,12 @@ var ( ) var ( - KeyPrefixTransientSuicided = []byte{prefixTransientSuicided} - KeyPrefixTransientBloom = []byte{prefixTransientBloom} - KeyPrefixTransientTxIndex = []byte{prefixTransientTxIndex} - KeyPrefixTransientRefund = []byte{prefixTransientRefund} + KeyPrefixTransientSuicided = []byte{prefixTransientSuicided} + KeyPrefixTransientBloom = []byte{prefixTransientBloom} + KeyPrefixTransientTxIndex = []byte{prefixTransientTxIndex} + KeyPrefixTransientRefund = []byte{prefixTransientRefund} + KeyPrefixTransientAccessListAddress = []byte{prefixTransientAccessListAddress} + KeyPrefixTransientAccessListSlot = []byte{prefixTransientAccessListSlot} ) // BloomKey defines the store key for a block Bloom From 769cdb0cc91168cbd051e010b2e1bee58c988bca Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 09:52:52 +0200 Subject: [PATCH 2/6] evm: remove address and slot access list mappings --- x/evm/keeper/keeper.go | 9 +-- x/evm/keeper/statedb.go | 1 - x/evm/types/access_list.go | 130 ------------------------------------- x/evm/types/journal.go | 4 +- x/evm/types/statedb.go | 115 +++++++++++++++++--------------- 5 files changed, 66 insertions(+), 193 deletions(-) diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 3b39e27621..df7978f604 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -45,12 +45,6 @@ type Keeper struct { // Ethermint concrete implementation on the EVM StateDB interface CommitStateDB *types.CommitStateDB - // Per-transaction access list - // See EIP-2930 for more info: https://eips.ethereum.org/EIPS/eip-2930 - // TODO: (@fedekunze) for how long should we persist the entries in the access list? - // same block (i.e Transient Store)? 2 or more (KVStore with module Parameter which resets the state after that window)? - accessList *types.AccessListMappings - // hash header for the current height. Reset during abci.RequestBeginBlock headerHash common.Hash } @@ -73,8 +67,7 @@ func NewKeeper( bankKeeper: bankKeeper, storeKey: storeKey, transientKey: transientKey, - CommitStateDB: types.NewCommitStateDB(sdk.Context{}, storeKey, paramSpace, ak, bankKeeper), - accessList: types.NewAccessListMappings(), + CommitStateDB: types.NewCommitStateDB(sdk.Context{}, storeKey, transientKey, paramSpace, ak, bankKeeper), } } diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 9d84e339bf..20e94e0a45 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -536,7 +536,6 @@ func (k *Keeper) AddSlotToAccessList(addr common.Address, slot common.Hash) { // access list and slot in acccess list return } - } // ---------------------------------------------------------------------------- diff --git a/x/evm/types/access_list.go b/x/evm/types/access_list.go index e1b8e860e7..2e67ac9384 100644 --- a/x/evm/types/access_list.go +++ b/x/evm/types/access_list.go @@ -1,140 +1,10 @@ package types import ( - "github.com/ethereum/go-ethereum/common" ethcmn "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" ) -// AccessListMappings is copied from go-ethereum -// https://github.com/ethereum/go-ethereum/blob/cf856ea1ad96ac39ea477087822479b63417036a/core/state/access_list.go#L23 -type AccessListMappings struct { - addresses map[common.Address]int - slots []map[common.Hash]struct{} -} - -// ContainsAddress returns true if the address is in the access list. -func (al *AccessListMappings) ContainsAddress(address common.Address) bool { - _, ok := al.addresses[address] - return ok -} - -// Contains checks if a slot within an account is present in the access list, returning -// separate flags for the presence of the account and the slot respectively. -func (al *AccessListMappings) Contains(address common.Address, slot common.Hash) (addressPresent bool, slotPresent bool) { - idx, ok := al.addresses[address] - if !ok { - // no such address (and hence zero slots) - return false, false - } - if idx == -1 { - // address yes, but no slots - return true, false - } - - if idx >= len(al.slots) { - // return in case of out-of-range - return true, false - } - - _, slotPresent = al.slots[idx][slot] - return true, slotPresent -} - -// newAccessList creates a new AccessListMappings. -func NewAccessListMappings() *AccessListMappings { - return &AccessListMappings{ - addresses: make(map[common.Address]int), - } -} - -// Copy creates an independent copy of an AccessListMappings. -func (al *AccessListMappings) Copy() *AccessListMappings { - cp := NewAccessListMappings() - for k, v := range al.addresses { - cp.addresses[k] = v - } - cp.slots = make([]map[common.Hash]struct{}, len(al.slots)) - for i, slotMap := range al.slots { - newSlotmap := make(map[common.Hash]struct{}, len(slotMap)) - for k := range slotMap { - newSlotmap[k] = struct{}{} - } - cp.slots[i] = newSlotmap - } - return cp -} - -// AddAddress adds an address to the access list, and returns 'true' if the operation -// caused a change (addr was not previously in the list). -func (al *AccessListMappings) AddAddress(address common.Address) bool { - if _, present := al.addresses[address]; present { - return false - } - al.addresses[address] = -1 - return true -} - -// AddSlot adds the specified (addr, slot) combo to the access list. -// Return values are: -// - address added -// - slot added -// For any 'true' value returned, a corresponding journal entry must be made. -func (al *AccessListMappings) AddSlot(address common.Address, slot common.Hash) (addrChange bool, slotChange bool) { - idx, addrPresent := al.addresses[address] - if !addrPresent || idx == -1 { - // Address not present, or addr present but no slots there - al.addresses[address] = len(al.slots) - slotmap := map[common.Hash]struct{}{slot: {}} - al.slots = append(al.slots, slotmap) - return !addrPresent, true - } - - if idx >= len(al.slots) { - // return in case of out-of-range - return false, false - } - - // There is already an (address,slot) mapping - slotmap := al.slots[idx] - if _, ok := slotmap[slot]; !ok { - slotmap[slot] = struct{}{} - // journal add slot change - return false, true - } - // No changes required - return false, false -} - -// DeleteSlot removes an (address, slot)-tuple from the access list. -// This operation needs to be performed in the same order as the addition happened. -// This method is meant to be used by the journal, which maintains ordering of -// operations. -func (al *AccessListMappings) DeleteSlot(address common.Address, slot common.Hash) { - idx, addrOk := al.addresses[address] - // There are two ways this can fail - if !addrOk { - panic("reverting slot change, address not present in list") - } - slotmap := al.slots[idx] - delete(slotmap, slot) - // If that was the last (first) slot, remove it - // Since additions and rollbacks are always performed in order, - // we can delete the item without worrying about screwing up later indices - if len(slotmap) == 0 { - al.slots = al.slots[:idx] - al.addresses[address] = -1 - } -} - -// DeleteAddress removes an address from the access list. This operation -// needs to be performed in the same order as the addition happened. -// This method is meant to be used by the journal, which maintains ordering of -// operations. -func (al *AccessListMappings) DeleteAddress(address common.Address) { - delete(al.addresses, address) -} - // AccessList is an EIP-2930 access list that represents the slice of // the protobuf AccessTuples. type AccessList []AccessTuple diff --git a/x/evm/types/journal.go b/x/evm/types/journal.go index 20f28595a7..65a5b30795 100644 --- a/x/evm/types/journal.go +++ b/x/evm/types/journal.go @@ -364,7 +364,7 @@ func (ch accessListAddAccountChange) revert(s *CommitStateDB) { (addr) at this point, since no storage adds can remain when come upon a single (addr) change. */ - s.accessList.DeleteAddress(*ch.address) + // s.accessList.DeleteAddress(*ch.address) } func (ch accessListAddAccountChange) dirtied() *ethcmn.Address { @@ -372,7 +372,7 @@ func (ch accessListAddAccountChange) dirtied() *ethcmn.Address { } func (ch accessListAddSlotChange) revert(s *CommitStateDB) { - s.accessList.DeleteSlot(*ch.address, *ch.slot) + // s.accessList.DeleteSlot(*ch.address, *ch.slot) } func (ch accessListAddSlotChange) dirtied() *ethcmn.Address { diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index 4b41d00a2a..041f31bac3 100644 --- a/x/evm/types/statedb.go +++ b/x/evm/types/statedb.go @@ -38,6 +38,7 @@ type CommitStateDB struct { ctx sdk.Context storeKey sdk.StoreKey + transientKey sdk.StoreKey paramSpace paramtypes.Subspace accountKeeper AccountKeeper bankKeeper BankKeeper @@ -73,9 +74,6 @@ type CommitStateDB struct { validRevisions []revision nextRevisionID int - // Per-transaction access list - accessList *AccessListMappings - // mutex for state deep copying lock sync.Mutex } @@ -86,12 +84,13 @@ type CommitStateDB struct { // CONTRACT: Stores used for state must be cache-wrapped as the ordering of the // key/value space matters in determining the merkle root. func NewCommitStateDB( - ctx sdk.Context, storeKey sdk.StoreKey, paramSpace paramtypes.Subspace, + ctx sdk.Context, storeKey, tKey sdk.StoreKey, paramSpace paramtypes.Subspace, ak AccountKeeper, bankKeeper BankKeeper, ) *CommitStateDB { return &CommitStateDB{ ctx: ctx, storeKey: storeKey, + transientKey: tKey, paramSpace: paramSpace, accountKeeper: ak, bankKeeper: bankKeeper, @@ -101,7 +100,6 @@ func NewCommitStateDB( preimages: []preimageEntry{}, hashToPreimageIndex: make(map[ethcmn.Hash]int), journal: newJournal(), - accessList: NewAccessListMappings(), } } @@ -253,67 +251,82 @@ func (csdb *CommitStateDB) SubRefund(gas uint64) { csdb.refund -= gas } -// AddAddressToAccessList adds the given address to the access list -func (csdb *CommitStateDB) AddAddressToAccessList(addr ethcmn.Address) { - if csdb.accessList.AddAddress(addr) { - csdb.journal.append(accessListAddAccountChange{&addr}) - } -} - -// AddSlotToAccessList adds the given (address, slot)-tuple to the access list -func (csdb *CommitStateDB) AddSlotToAccessList(addr ethcmn.Address, slot ethcmn.Hash) { - addrMod, slotMod := csdb.accessList.AddSlot(addr, slot) - if addrMod { - // In practice, this should not happen, since there is no way to enter the - // scope of 'address' without having the 'address' become already added - // to the access list (via call-variant, create, etc). - // Better safe than sorry, though - csdb.journal.append(accessListAddAccountChange{&addr}) - } - if slotMod { - csdb.journal.append(accessListAddSlotChange{ - address: &addr, - slot: &slot, - }) - } -} - -// AddressInAccessList returns true if the given address is in the access list. -func (csdb *CommitStateDB) AddressInAccessList(addr ethcmn.Address) bool { - return csdb.accessList.ContainsAddress(addr) -} - -// SlotInAccessList returns true if the given (address, slot)-tuple is in the access list. -func (csdb *CommitStateDB) SlotInAccessList(addr ethcmn.Address, slot ethcmn.Hash) (bool, bool) { - return csdb.accessList.Contains(addr, slot) -} +// ---------------------------------------------------------------------------- +// Access List +// ---------------------------------------------------------------------------- // PrepareAccessList handles the preparatory steps for executing a state transition with // regards to both EIP-2929 and EIP-2930: // -// - Add sender to access list (2929) -// - Add destination to access list (2929) -// - Add precompiles to access list (2929) -// - Add the contents of the optional tx access list (2930) +// - Add sender to access list (2929) +// - Add destination to access list (2929) +// - Add precompiles to access list (2929) +// - Add the contents of the optional tx access list (2930) // // This method should only be called if Yolov3/Berlin/2929+2930 is applicable at the current number. -func (csdb *CommitStateDB) PrepareAccessList(sender ethcmn.Address, dst *ethcmn.Address, precompiles []ethcmn.Address, list ethtypes.AccessList) { +func (csdb *CommitStateDB) PrepareAccessList(sender ethcmn.Address, dest *ethcmn.Address, precompiles []ethcmn.Address, txAccesses ethtypes.AccessList) { csdb.AddAddressToAccessList(sender) - if dst != nil { - csdb.AddAddressToAccessList(*dst) + if dest != nil { + csdb.AddAddressToAccessList(*dest) // If it's a create-tx, the destination will be added inside evm.create } for _, addr := range precompiles { csdb.AddAddressToAccessList(addr) } - for _, el := range list { - csdb.AddAddressToAccessList(el.Address) - for _, key := range el.StorageKeys { - csdb.AddSlotToAccessList(el.Address, key) + for _, tuple := range txAccesses { + csdb.AddAddressToAccessList(tuple.Address) + for _, key := range tuple.StorageKeys { + csdb.AddSlotToAccessList(tuple.Address, key) } } } +// AddressInAccessList returns true if the address is registered on the access list map. +func (csdb *CommitStateDB) AddressInAccessList(addr ethcmn.Address) bool { + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListAddress) + return ts.Has(addr.Bytes()) +} + +func (csdb *CommitStateDB) SlotInAccessList(addr ethcmn.Address, slot ethcmn.Hash) (addressOk bool, slotOk bool) { + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + + addressOk = csdb.AddressInAccessList(addr) + slotOk = ts.Has(key) + return addressOk, slotOk +} + +// AddAddressToAccessList adds the given address to the access list. This operation is safe to perform +// even if the feature/fork is not active yet +func (csdb *CommitStateDB) AddAddressToAccessList(addr ethcmn.Address) { + if csdb.AddressInAccessList(addr) { + return + } + + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListAddress) + ts.Set(addr.Bytes(), []byte{0x1}) +} + +// AddSlotToAccessList adds the given (address,slot) to the access list. This operation is safe to perform +// even if the feature/fork is not active yet +func (csdb *CommitStateDB) AddSlotToAccessList(addr ethcmn.Address, slot ethcmn.Hash) { + addrOk, slotOk := csdb.SlotInAccessList(addr, slot) + + switch { + case !addrOk: + csdb.AddAddressToAccessList(addr) + fallthrough + case !slotOk: + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + ts.Set(key, []byte{0x1}) + default: + // access list and slot in acccess list + return + } + +} + // ---------------------------------------------------------------------------- // Getters // ---------------------------------------------------------------------------- @@ -721,7 +734,6 @@ func (csdb *CommitStateDB) Reset(_ ethcmn.Hash) error { csdb.logSize = 0 csdb.preimages = []preimageEntry{} csdb.hashToPreimageIndex = make(map[ethcmn.Hash]int) - csdb.accessList = NewAccessListMappings() csdb.clearJournalAndRefund() return nil @@ -824,7 +836,6 @@ func CopyCommitStateDB(from, to *CommitStateDB) { copy(validRevisions, from.validRevisions) to.validRevisions = validRevisions to.nextRevisionID = from.nextRevisionID - to.accessList = from.accessList.Copy() // copy the dirty states, logs, and preimages for _, dirty := range from.journal.dirties { From 06d10ebe92038f01b8645dc066c2d4e5782ead07 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 10:35:16 +0200 Subject: [PATCH 3/6] update tests --- x/evm/keeper/statedb.go | 41 ++++++++++++++++---------------- x/evm/keeper/statedb_test.go | 46 +++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 24 deletions(-) diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 20e94e0a45..e23bfe9693 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -493,23 +493,28 @@ func (k *Keeper) PrepareAccessList(sender common.Address, dest *common.Address, } } -// AddressInAccessList returns true if the address is registered on the access list map. +// AddressInAccessList returns true if the address is registered on the transient store. func (k *Keeper) AddressInAccessList(addr common.Address) bool { ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListAddress) return ts.Has(addr.Bytes()) } +// SlotInAccessList checks if the address and the slots are registered in the transient store func (k *Keeper) SlotInAccessList(addr common.Address, slot common.Hash) (addressOk bool, slotOk bool) { - ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) - key := append(addr.Bytes(), slot.Bytes()...) - addressOk = k.AddressInAccessList(addr) - slotOk = ts.Has(key) + slotOk = k.addressSlotInAccessList(addr, slot) return addressOk, slotOk } -// AddAddressToAccessList adds the given address to the access list. This operation is safe to perform -// even if the feature/fork is not active yet +// addressSlotInAccessList returns true if the address's slot is registered on the transient store. +func (k *Keeper) addressSlotInAccessList(addr common.Address, slot common.Hash) bool { + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + return ts.Has(key) +} + +// AddAddressToAccessList adds the given address to the access list. If the address is already +// in the access list, this function performs a no-op. func (k *Keeper) AddAddressToAccessList(addr common.Address) { if k.AddressInAccessList(addr) { return @@ -519,23 +524,17 @@ func (k *Keeper) AddAddressToAccessList(addr common.Address) { ts.Set(addr.Bytes(), []byte{0x1}) } -// AddSlotToAccessList adds the given (address,slot) to the access list. This operation is safe to perform -// even if the feature/fork is not active yet +// AddSlotToAccessList adds the given (address, slot) to the access list. If the address and slot are +// already in the access list, this function performs a no-op. func (k *Keeper) AddSlotToAccessList(addr common.Address, slot common.Hash) { - addrOk, slotOk := k.SlotInAccessList(addr, slot) - - switch { - case !addrOk: - k.AddAddressToAccessList(addr) - fallthrough - case !slotOk: - ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) - key := append(addr.Bytes(), slot.Bytes()...) - ts.Set(key, []byte{0x1}) - default: - // access list and slot in acccess list + k.AddAddressToAccessList(addr) + if k.addressSlotInAccessList(addr, slot) { return } + + ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + ts.Set(key, []byte{0x1}) } // ---------------------------------------------------------------------------- diff --git a/x/evm/keeper/statedb_test.go b/x/evm/keeper/statedb_test.go index 70e154812a..50f8f31606 100644 --- a/x/evm/keeper/statedb_test.go +++ b/x/evm/keeper/statedb_test.go @@ -506,7 +506,7 @@ func (suite *KeeperTestSuite) TestAddLog() { } } -func (suite *KeeperTestSuite) TestAccessList() { +func (suite *KeeperTestSuite) TestPrepareAccessList() { dest := tests.GenerateAddress() precompiles := []common.Address{tests.GenerateAddress(), tests.GenerateAddress()} accesses := ethtypes.AccessList{ @@ -526,12 +526,52 @@ func (suite *KeeperTestSuite) TestAccessList() { for _, access := range accesses { for _, key := range access.StorageKeys { addrOK, slotOK := suite.app.EvmKeeper.SlotInAccessList(access.Address, key) - suite.Require().True(addrOK) - suite.Require().True(slotOK) + suite.Require().True(addrOK, access.Address.Hex()) + suite.Require().True(slotOK, key.Hex()) } } } +func (suite *KeeperTestSuite) TestAddAddressToAccessList() { + testCases := []struct { + name string + addr common.Address + }{ + {"new address", suite.address}, + {"existing address", suite.address}, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.app.EvmKeeper.AddAddressToAccessList(tc.addr) + addrOk := suite.app.EvmKeeper.AddressInAccessList(tc.addr) + suite.Require().True(addrOk, tc.addr.Hex()) + }) + } +} + +func (suite *KeeperTestSuite) AddSlotToAccessList() { + testCases := []struct { + name string + addr common.Address + slot common.Hash + }{ + {"new address and slot (1)", tests.GenerateAddress(), common.BytesToHash([]byte("hash"))}, + {"new address and slot (2)", suite.address, common.Hash{}}, + {"existing address and slot", suite.address, common.Hash{}}, + {"existing address, new slot", suite.address, common.BytesToHash([]byte("hash"))}, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.app.EvmKeeper.AddSlotToAccessList(tc.addr, tc.slot) + addrOk, slotOk := suite.app.EvmKeeper.SlotInAccessList(tc.addr, tc.slot) + suite.Require().True(addrOk, tc.addr.Hex()) + suite.Require().True(slotOk, tc.slot.Hex()) + }) + } +} + func (suite *KeeperTestSuite) TestForEachStorage() { var storage types.Storage From fa067cfd2c07e3940b1c4188d0f7922b79a40391 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 10:42:48 +0200 Subject: [PATCH 4/6] update --- x/evm/types/access_list_test.go | 242 -------------------------------- x/evm/types/journal_test.go | 4 +- 2 files changed, 3 insertions(+), 243 deletions(-) delete mode 100644 x/evm/types/access_list_test.go diff --git a/x/evm/types/access_list_test.go b/x/evm/types/access_list_test.go deleted file mode 100644 index 3929b92157..0000000000 --- a/x/evm/types/access_list_test.go +++ /dev/null @@ -1,242 +0,0 @@ -package types - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/suite" - - ethcmn "github.com/ethereum/go-ethereum/common" - - "github.com/cosmos/ethermint/crypto/ethsecp256k1" -) - -type AccessListTestSuite struct { - suite.Suite - - address ethcmn.Address - accessList *AccessListMappings -} - -func (suite *AccessListTestSuite) SetupTest() { - privkey, err := ethsecp256k1.GenerateKey() - suite.Require().NoError(err) - - suite.address = ethcmn.BytesToAddress(privkey.PubKey().Address().Bytes()) - suite.accessList = NewAccessListMappings() - suite.accessList.addresses[suite.address] = 1 -} - -func TestAccessListTestSuite(t *testing.T) { - suite.Run(t, new(AccessListTestSuite)) -} - -func (suite *AccessListTestSuite) TestContainsAddress() { - found := suite.accessList.ContainsAddress(suite.address) - suite.Require().True(found) -} - -func (suite *AccessListTestSuite) TestContains() { - testCases := []struct { - name string - malleate func() - expAddrPresent bool - expSlotPresent bool - }{ - {"out of range", func() {}, true, false}, - { - "address, no slots", - func() { - suite.accessList.addresses[suite.address] = -1 - }, true, false, - }, - { - "no address, no slots", - func() { - delete(suite.accessList.addresses, suite.address) - }, false, false, - }, - { - "address, slot not present", - func() { - suite.accessList.addresses[suite.address] = 0 - suite.accessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - }, true, false, - }, - { - "address, slots", - func() { - suite.accessList.addresses[suite.address] = 0 - suite.accessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - suite.accessList.slots[0] = make(map[ethcmn.Hash]struct{}) - suite.accessList.slots[0][ethcmn.Hash{}] = struct{}{} - }, true, true, - }, - } - - for _, tc := range testCases { - tc.malleate() - - addrPresent, slotPresent := suite.accessList.Contains(suite.address, ethcmn.Hash{}) - - suite.Require().Equal(tc.expAddrPresent, addrPresent, tc.name) - suite.Require().Equal(tc.expSlotPresent, slotPresent, tc.name) - } -} - -func (suite *AccessListTestSuite) TestCopy() { - expAccessList := NewAccessListMappings() - - testCases := []struct { - name string - malleate func() - }{ - {"empty", func() { - expAccessList.slots = make([]map[ethcmn.Hash]struct{}, 0) - }}, - { - "single address", func() { - expAccessList = NewAccessListMappings() - expAccessList.slots = make([]map[ethcmn.Hash]struct{}, 0) - expAccessList.addresses[suite.address] = -1 - }, - }, - { - "single address, single slot", - func() { - expAccessList = NewAccessListMappings() - expAccessList.addresses[suite.address] = 0 - expAccessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - expAccessList.slots[0] = make(map[ethcmn.Hash]struct{}) - expAccessList.slots[0][ethcmn.Hash{}] = struct{}{} - }, - }, - { - "multiple addresses, single slot each", - func() { - expAccessList = NewAccessListMappings() - expAccessList.slots = make([]map[ethcmn.Hash]struct{}, 10) - for i := 0; i < 10; i++ { - expAccessList.addresses[ethcmn.BytesToAddress([]byte(fmt.Sprintf("%d", i)))] = i - expAccessList.slots[i] = make(map[ethcmn.Hash]struct{}) - expAccessList.slots[i][ethcmn.BytesToHash([]byte(fmt.Sprintf("%d", i)))] = struct{}{} - } - }, - }, - { - "multiple addresses, multiple slots each", - func() { - expAccessList = NewAccessListMappings() - expAccessList.slots = make([]map[ethcmn.Hash]struct{}, 10) - for i := 0; i < 10; i++ { - expAccessList.addresses[ethcmn.BytesToAddress([]byte(fmt.Sprintf("%d", i)))] = i - expAccessList.slots[i] = make(map[ethcmn.Hash]struct{}) - for j := 0; j < 10; j++ { - expAccessList.slots[i][ethcmn.BytesToHash([]byte(fmt.Sprintf("%d-%d", i, j)))] = struct{}{} - } - } - }, - }, - } - - for _, tc := range testCases { - tc.malleate() - - accessList := expAccessList.Copy() - suite.Require().EqualValues(expAccessList, accessList, tc.name) - } -} - -func (suite *AccessListTestSuite) TestAddAddress() { - testCases := []struct { - name string - address ethcmn.Address - ok bool - }{ - {"already present", suite.address, false}, - {"ok", ethcmn.Address{}, true}, - } - - for _, tc := range testCases { - ok := suite.accessList.AddAddress(tc.address) - suite.Require().Equal(tc.ok, ok, tc.name) - } -} - -func (suite *AccessListTestSuite) TestAddSlot() { - testCases := []struct { - name string - malleate func() - expAddrChange bool - expSlotChange bool - }{ - {"out of range", func() {}, false, false}, - { - "address not present added, slot added", - func() { - delete(suite.accessList.addresses, suite.address) - }, true, true, - }, - { - "address present, slot not present added", - func() { - suite.accessList.addresses[suite.address] = 0 - suite.accessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - suite.accessList.slots[0] = make(map[ethcmn.Hash]struct{}) - }, false, true, - }, - { - "address present, slot present", - func() { - suite.accessList.addresses[suite.address] = 0 - suite.accessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - suite.accessList.slots[0] = make(map[ethcmn.Hash]struct{}) - suite.accessList.slots[0][ethcmn.Hash{}] = struct{}{} - }, false, false, - }, - } - - for _, tc := range testCases { - tc.malleate() - - addrChange, slotChange := suite.accessList.AddSlot(suite.address, ethcmn.Hash{}) - suite.Require().Equal(tc.expAddrChange, addrChange, tc.name) - suite.Require().Equal(tc.expSlotChange, slotChange, tc.name) - } -} - -func (suite *AccessListTestSuite) TestDeleteSlot() { - testCases := []struct { - name string - malleate func() - expPanic bool - }{ - {"panics, out of range", func() {}, true}, - {"panics, address not found", func() { - delete(suite.accessList.addresses, suite.address) - }, true}, - { - "single slot present", - func() { - suite.accessList.addresses[suite.address] = 0 - suite.accessList.slots = make([]map[ethcmn.Hash]struct{}, 1) - suite.accessList.slots[0] = make(map[ethcmn.Hash]struct{}) - suite.accessList.slots[0][ethcmn.Hash{}] = struct{}{} - }, false, - }, - } - - for _, tc := range testCases { - tc.malleate() - - if tc.expPanic { - suite.Require().Panics(func() { - suite.accessList.DeleteSlot(suite.address, ethcmn.Hash{}) - }, tc.name) - } else { - suite.Require().NotPanics(func() { - suite.accessList.DeleteSlot(suite.address, ethcmn.Hash{}) - }, tc.name) - } - } -} diff --git a/x/evm/types/journal_test.go b/x/evm/types/journal_test.go index 4aad97912f..87d0961c77 100644 --- a/x/evm/types/journal_test.go +++ b/x/evm/types/journal_test.go @@ -104,6 +104,7 @@ func (suite *JournalTestSuite) setup() { authKey := sdk.NewKVStoreKey(authtypes.StoreKey) paramsKey := sdk.NewKVStoreKey(paramtypes.StoreKey) paramsTKey := sdk.NewTransientStoreKey(paramtypes.TStoreKey) + tKey := sdk.NewTransientStoreKey(TransientKey) bankKey := sdk.NewKVStoreKey(banktypes.StoreKey) storeKey := sdk.NewKVStoreKey(StoreKey) @@ -120,6 +121,7 @@ func (suite *JournalTestSuite) setup() { cms.MountStoreWithDB(paramsKey, sdk.StoreTypeIAVL, db) cms.MountStoreWithDB(storeKey, sdk.StoreTypeIAVL, db) cms.MountStoreWithDB(paramsTKey, sdk.StoreTypeTransient, db) + cms.MountStoreWithDB(tKey, sdk.StoreTypeTransient, db) err = cms.LoadLatestVersion() suite.Require().NoError(err) @@ -135,7 +137,7 @@ func (suite *JournalTestSuite) setup() { ak := authkeeper.NewAccountKeeper(cdc, authKey, authSubspace, ethermint.ProtoAccount, nil) bk := bankkeeper.NewBaseKeeper(cdc, bankKey, ak, bankSubspace, nil) suite.ctx = sdk.NewContext(cms, tmproto.Header{ChainID: "ethermint-8"}, false, tmlog.NewNopLogger()) - suite.stateDB = NewCommitStateDB(suite.ctx, storeKey, evmSubspace, ak, bk).WithContext(suite.ctx) + suite.stateDB = NewCommitStateDB(suite.ctx, storeKey, tKey, evmSubspace, ak, bk).WithContext(suite.ctx) suite.stateDB.SetParams(DefaultParams()) } From 83526e1bee69c4e0c318a4481302fc327dc08f10 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 12:00:19 +0200 Subject: [PATCH 5/6] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bcf8f1e8a..e53bbb2399 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Unreleased +### State Machine Breaking + +* (evm) [tharsis#72](https://github.com/tharsis/ethermint/issues/72) Update `AccessList` to use `TransientStore` instead of map. + ### API Breaking * (eth) [\#845](https://github.com/cosmos/ethermint/pull/845) The `eth` namespace must be included in the list of API's as default to run the rpc server without error. From 9c70cc93c38a9959a49dcec51475f09e642e3727 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 7 Jun 2021 12:04:57 +0200 Subject: [PATCH 6/6] update types --- x/evm/types/statedb.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index 041f31bac3..fa8aefb614 100644 --- a/x/evm/types/statedb.go +++ b/x/evm/types/statedb.go @@ -252,7 +252,7 @@ func (csdb *CommitStateDB) SubRefund(gas uint64) { } // ---------------------------------------------------------------------------- -// Access List +// Access List // TODO: deprecate // ---------------------------------------------------------------------------- // PrepareAccessList handles the preparatory steps for executing a state transition with @@ -288,14 +288,17 @@ func (csdb *CommitStateDB) AddressInAccessList(addr ethcmn.Address) bool { } func (csdb *CommitStateDB) SlotInAccessList(addr ethcmn.Address, slot ethcmn.Hash) (addressOk bool, slotOk bool) { - ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) - key := append(addr.Bytes(), slot.Bytes()...) - addressOk = csdb.AddressInAccessList(addr) - slotOk = ts.Has(key) + slotOk = csdb.addressSlotInAccessList(addr, slot) return addressOk, slotOk } +func (csdb *CommitStateDB) addressSlotInAccessList(addr ethcmn.Address, slot ethcmn.Hash) bool { + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + return ts.Has(key) +} + // AddAddressToAccessList adds the given address to the access list. This operation is safe to perform // even if the feature/fork is not active yet func (csdb *CommitStateDB) AddAddressToAccessList(addr ethcmn.Address) { @@ -310,21 +313,14 @@ func (csdb *CommitStateDB) AddAddressToAccessList(addr ethcmn.Address) { // AddSlotToAccessList adds the given (address,slot) to the access list. This operation is safe to perform // even if the feature/fork is not active yet func (csdb *CommitStateDB) AddSlotToAccessList(addr ethcmn.Address, slot ethcmn.Hash) { - addrOk, slotOk := csdb.SlotInAccessList(addr, slot) - - switch { - case !addrOk: - csdb.AddAddressToAccessList(addr) - fallthrough - case !slotOk: - ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) - key := append(addr.Bytes(), slot.Bytes()...) - ts.Set(key, []byte{0x1}) - default: - // access list and slot in acccess list + csdb.AddAddressToAccessList(addr) + if csdb.addressSlotInAccessList(addr, slot) { return } + ts := prefix.NewStore(csdb.ctx.TransientStore(csdb.transientKey), KeyPrefixTransientAccessListSlot) + key := append(addr.Bytes(), slot.Bytes()...) + ts.Set(key, []byte{0x1}) } // ----------------------------------------------------------------------------