evm: use TransientStore for AccessList

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.

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),
 	}
 }
 

x/evm/keeper/statedb.go

@@ -493,37 +493,48 @@ 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 {
-	return k.accessList.ContainsAddress(addr)
+	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) {
-	return k.accessList.Contains(addr, slot)
+	addressOk = k.AddressInAccessList(addr)
+	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) {
-	// 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
+// 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) {
-	// NOTE: only update the access list during DeliverTx
-	if k.ctx.IsCheckTx() || k.ctx.IsReCheckTx() {
+	k.AddAddressToAccessList(addr)
+	if k.addressSlotInAccessList(addr, slot) {
 		return
 	}
 
-	// NOTE: ignore change return booleans because we don't have to keep a journal for state changes
-	_, _ = k.accessList.AddSlot(addr, slot)
+	ts := prefix.NewStore(k.ctx.TransientStore(k.transientKey), types.KeyPrefixTransientAccessListSlot)
+	key := append(addr.Bytes(), slot.Bytes()...)
+	ts.Set(key, []byte{0x1})
 }
 
 // ----------------------------------------------------------------------------

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
 

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