From bffdfeccc930b537feb881967f75f8890d20939e Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 11 Jan 2024 14:09:50 -0800 Subject: [PATCH 1/5] store codes by codeHash --- fvm/evm/emulator/state/base.go | 141 ++++++++++++++++++++++------ fvm/evm/emulator/state/code.go | 62 ++++++++++++ fvm/evm/emulator/state/code_test.go | 35 +++++++ 3 files changed, 208 insertions(+), 30 deletions(-) create mode 100644 fvm/evm/emulator/state/code.go create mode 100644 fvm/evm/emulator/state/code_test.go diff --git a/fvm/evm/emulator/state/base.go b/fvm/evm/emulator/state/base.go index 173aa6d40f0..6495b218f01 100644 --- a/fvm/evm/emulator/state/base.go +++ b/fvm/evm/emulator/state/base.go @@ -204,7 +204,7 @@ func (v *BaseView) CreateAccount( var colID []byte // if is an smart contract account if len(code) > 0 { - err := v.storeCode(addr, code) + err := v.updateAccountCode(addr, code, codeHash) if err != nil { return err } @@ -235,17 +235,17 @@ func (v *BaseView) UpdateAccount( if acc == nil { return v.CreateAccount(addr, balance, nonce, code, codeHash) } - // if it has a code change - if codeHash != acc.CodeHash { - err := v.storeCode(addr, code) - if err != nil { - return err - } - // TODO: maybe purge the state in the future as well - // currently the behaviour of stateDB doesn't purge the data - // We don't need to check if the code is empty and we purge the state - // this is not possible right now. + + // update account code + err = v.updateAccountCode(addr, code, codeHash) + if err != nil { + return err } + // TODO: maybe purge the state in the future as well + // currently the behaviour of stateDB doesn't purge the data + // We don't need to check if the code is empty and we purge the state + // this is not possible right now. + newAcc := NewAccount(addr, balance, nonce, codeHash, acc.CollectionID) // no need to update the cache , storeAccount would update the cache return v.storeAccount(newAcc) @@ -263,23 +263,23 @@ func (v *BaseView) DeleteAccount(addr gethCommon.Address) error { return fmt.Errorf("account doesn't exist to be deleted") } - // 2. update the cache + // 2. remove the code + if acc.HasCode() { + err = v.updateAccountCode(addr, nil, gethTypes.EmptyCodeHash) + if err != nil { + return err + } + } + + // 3. update the cache delete(v.cachedAccounts, addr) - // 3. collections + // 4. collections err = v.accounts.Remove(addr.Bytes()) if err != nil { return err } - // 4. remove the code - if acc.HasCode() { - err = v.storeCode(addr, nil) - if err != nil { - return err - } - } - // 5. remove storage slots if len(acc.CollectionID) > 0 { col, found := v.slots[addr] @@ -391,29 +391,110 @@ func (v *BaseView) getCode(addr gethCommon.Address) ([]byte, error) { if found { return code, nil } - // check if account exist in cache and has codeHash - acc, found := v.cachedAccounts[addr] - if found && !acc.HasCode() { + + // get account + acc, err := v.getAccount(addr) + if err != nil { + return nil, err + } + + if acc == nil || !acc.HasCode() { + return nil, nil + } + + // collect the container from the code collection by codeHash + encoded, err := v.codes.Get(acc.CodeHash.Bytes()) + if err != nil { + return nil, err + } + if len(encoded) == 0 { return nil, nil } - // then collect it from the code collection - code, err := v.codes.Get(addr.Bytes()) + + codeCont, err := CodeContainerFromEncoded(encoded) if err != nil { return nil, err } + code = codeCont.Code() if code != nil { v.cachedCodes[addr] = code } return code, nil } -func (v *BaseView) storeCode(addr gethCommon.Address, code []byte) error { - if len(code) == 0 { +func (v *BaseView) updateAccountCode(addr gethCommon.Address, code []byte, codeHash gethCommon.Hash) error { + // get account + acc, err := v.getAccount(addr) + if err != nil { + return err + } + // if is a new account + if acc == nil { + if len(code) == 0 { + return nil + } + v.cachedCodes[addr] = code + return v.addCode(code, codeHash) + } + + // skip if is the same code + if acc.CodeHash == codeHash { + return nil + } + + // clean old code first if exist + if acc.HasCode() { delete(v.cachedCodes, addr) - return v.codes.Remove(addr.Bytes()) + err = v.removeCode(acc.CodeHash) + if err != nil { + return err + } + } + + // add new code + if len(code) == 0 { + return nil } v.cachedCodes[addr] = code - return v.codes.Set(addr.Bytes(), code) + return v.addCode(code, codeHash) +} + +func (v *BaseView) removeCode(codeHash gethCommon.Hash) error { + encoded, err := v.codes.Get(codeHash.Bytes()) + if err != nil { + return err + } + if len(encoded) == 0 { + return nil + } + + cc, err := CodeContainerFromEncoded(encoded) + if err != nil { + return err + } + if cc.DecRefCount() { + return v.codes.Remove(codeHash.Bytes()) + } + return v.codes.Set(codeHash.Bytes(), cc.Encoded()) +} + +func (v *BaseView) addCode(code []byte, codeHash gethCommon.Hash) error { + encoded, err := v.codes.Get(codeHash.Bytes()) + if err != nil { + return err + } + // if is the first time the code is getting deployed + if len(encoded) == 0 { + return v.codes.Set(codeHash.Bytes(), NewCodeContainer(code).Encoded()) + } + + // otherwise update the cc + cc, err := CodeContainerFromEncoded(encoded) + if err != nil { + return err + } + cc.IncRefCount() + return v.codes.Set(codeHash.Bytes(), cc.Encoded()) } func (v *BaseView) getSlot(sk types.SlotAddress) (gethCommon.Hash, error) { diff --git a/fvm/evm/emulator/state/code.go b/fvm/evm/emulator/state/code.go new file mode 100644 index 00000000000..176859fbd18 --- /dev/null +++ b/fvm/evm/emulator/state/code.go @@ -0,0 +1,62 @@ +package state + +import ( + "encoding/binary" + "fmt" +) + +// CodeContainer contains codes and keeps +// track of reference counts +type CodeContainer struct { + code []byte + refCount uint64 +} + +// NewCodeContainer constructs a new code container +func NewCodeContainer(code []byte) *CodeContainer { + return &CodeContainer{ + code: code, + refCount: 1, + } +} + +// CodeContainerFromEncoded constructs a code container from the encoded data +func CodeContainerFromEncoded(encoded []byte) (*CodeContainer, error) { + if len(encoded) < 8 { + return nil, fmt.Errorf("invalid length for the encoded code container") + } + return &CodeContainer{ + refCount: binary.BigEndian.Uint64(encoded[:8]), + code: encoded[8:], + }, nil +} + +// Code returns the code part of the code container +func (cc *CodeContainer) Code() []byte { + return cc.code +} + +// RefCount returns the ref count +func (cc *CodeContainer) RefCount() uint64 { + return cc.refCount +} + +// IncRefCount increment the ref count +func (cc *CodeContainer) IncRefCount() { + cc.refCount++ +} + +// DecRefCount decrement the ref count and +// returns true if the ref has reached to zero +func (cc *CodeContainer) DecRefCount() bool { + cc.refCount-- + return cc.refCount == 0 +} + +// Encoded returns the encoded content of the code container +func (cc *CodeContainer) Encoded() []byte { + encoded := make([]byte, 8+len(cc.code)) + binary.BigEndian.PutUint64(encoded[:8], cc.refCount) + copy(encoded[8:], cc.code) + return encoded +} diff --git a/fvm/evm/emulator/state/code_test.go b/fvm/evm/emulator/state/code_test.go new file mode 100644 index 00000000000..31178cd4885 --- /dev/null +++ b/fvm/evm/emulator/state/code_test.go @@ -0,0 +1,35 @@ +package state_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/onflow/flow-go/fvm/evm/emulator/state" +) + +func TestCodeContainer(t *testing.T) { + code := []byte("some code") + + // test construction + cc := state.NewCodeContainer(code) + require.Equal(t, uint64(1), cc.RefCount()) + require.Equal(t, code, cc.Code()) + + // test increment + cc.IncRefCount() + require.Equal(t, uint64(2), cc.RefCount()) + + // test encoding + encoded := cc.Encoded() + cc, err := state.CodeContainerFromEncoded(encoded) + require.NoError(t, err) + require.Equal(t, uint64(2), cc.RefCount()) + require.Equal(t, code, cc.Code()) + + // test decrement + require.Equal(t, false, cc.DecRefCount()) + require.Equal(t, uint64(1), cc.RefCount()) + require.Equal(t, true, cc.DecRefCount()) + require.Equal(t, uint64(0), cc.RefCount()) +} From d424b037e4e3977cdf5c18bdf85ee517e0806fc2 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 11 Jan 2024 16:50:11 -0800 Subject: [PATCH 2/5] add test --- fvm/evm/emulator/state/base.go | 10 +++ fvm/evm/emulator/state/base_test.go | 92 ++++++++++++++++++++++++++++ fvm/evm/emulator/state/collection.go | 5 ++ 3 files changed, 107 insertions(+) diff --git a/fvm/evm/emulator/state/base.go b/fvm/evm/emulator/state/base.go index 6495b218f01..4b0e0c2bd5a 100644 --- a/fvm/evm/emulator/state/base.go +++ b/fvm/evm/emulator/state/base.go @@ -338,6 +338,16 @@ func (v *BaseView) Commit() error { return nil } +// NumberOfContracts returns the number of unique contracts +func (v *BaseView) NumberOfContracts() uint64 { + return v.codes.Size() +} + +// NumberOfContracts returns the number of accounts +func (v *BaseView) NumberOfAccounts() uint64 { + return v.accounts.Size() +} + func (v *BaseView) fetchOrCreateCollection(path string) (collection *Collection, created bool, error error) { collectionID, err := v.ledger.GetValue(v.rootAddress[:], []byte(path)) if err != nil { diff --git a/fvm/evm/emulator/state/base_test.go b/fvm/evm/emulator/state/base_test.go index c216d365b55..3178513dda2 100644 --- a/fvm/evm/emulator/state/base_test.go +++ b/fvm/evm/emulator/state/base_test.go @@ -6,6 +6,7 @@ import ( gethCommon "github.com/ethereum/go-ethereum/common" gethTypes "github.com/ethereum/go-ethereum/core/types" + gethCrypto "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" "github.com/onflow/flow-go/fvm/evm/emulator/state" @@ -210,6 +211,97 @@ func TestBaseView(t *testing.T) { require.Equal(t, false, addrFound) require.Equal(t, false, slotFound) }) + + t.Run("test code storage", func(t *testing.T) { + ledger := testutils.GetSimpleValueStore() + rootAddr := flow.Address{1, 2, 3, 4, 5, 6, 7, 8} + view, err := state.NewBaseView(ledger, rootAddr) + require.NoError(t, err) + + bal := new(big.Int) + nonce := uint64(0) + + addr1 := testutils.RandomCommonAddress(t) + var code1 []byte + codeHash1 := gethTypes.EmptyCodeHash + err = view.CreateAccount(addr1, bal, nonce, code1, codeHash1) + require.NoError(t, err) + + ret, err := view.GetCode(addr1) + require.NoError(t, err) + require.Equal(t, code1, ret) + + addr2 := testutils.RandomCommonAddress(t) + code2 := []byte("code2") + codeHash2 := gethCrypto.Keccak256Hash(code2) + err = view.CreateAccount(addr2, bal, nonce, code2, codeHash2) + require.NoError(t, err) + + ret, err = view.GetCode(addr2) + require.NoError(t, err) + require.Equal(t, code2, ret) + + err = view.Commit() + require.NoError(t, err) + orgSize := ledger.TotalStorageSize() + require.Equal(t, uint64(1), view.NumberOfContracts()) + + err = view.UpdateAccount(addr1, bal, nonce, code2, codeHash2) + require.NoError(t, err) + + err = view.Commit() + require.NoError(t, err) + require.Equal(t, orgSize, ledger.TotalStorageSize()) + require.Equal(t, uint64(1), view.NumberOfContracts()) + + ret, err = view.GetCode(addr1) + require.NoError(t, err) + require.Equal(t, code2, ret) + + // now remove the code from account 1 + err = view.UpdateAccount(addr1, bal, nonce, code1, codeHash1) + require.NoError(t, err) + + // there should not be any side effect on the code return for account 2 + // and no impact on storage size + ret, err = view.GetCode(addr2) + require.NoError(t, err) + require.Equal(t, code2, ret) + + ret, err = view.GetCode(addr1) + require.NoError(t, err) + require.Equal(t, code1, ret) + + err = view.Commit() + require.NoError(t, err) + require.Equal(t, orgSize, ledger.TotalStorageSize()) + require.Equal(t, uint64(1), view.NumberOfContracts()) + + // now update account 2 and there should a reduction in storage + err = view.UpdateAccount(addr2, bal, nonce, code1, codeHash1) + require.NoError(t, err) + + ret, err = view.GetCode(addr2) + require.NoError(t, err) + require.Equal(t, code1, ret) + + err = view.Commit() + require.NoError(t, err) + require.Greater(t, orgSize, ledger.TotalStorageSize()) + require.Equal(t, uint64(0), view.NumberOfContracts()) + + // delete account 2 + err = view.DeleteAccount(addr2) + require.NoError(t, err) + + ret, err = view.GetCode(addr2) + require.NoError(t, err) + require.Len(t, ret, 0) + + require.Greater(t, orgSize, ledger.TotalStorageSize()) + require.Equal(t, uint64(1), view.NumberOfAccounts()) + }) + } func checkAccount(t *testing.T, diff --git a/fvm/evm/emulator/state/collection.go b/fvm/evm/emulator/state/collection.go index b2bb605c903..780d81652df 100644 --- a/fvm/evm/emulator/state/collection.go +++ b/fvm/evm/emulator/state/collection.go @@ -197,6 +197,11 @@ func (c *Collection) Destroy() ([][]byte, error) { return keys, c.storage.Remove(c.omap.StorageID()) } +// Size returns the number of items in the collection +func (c *Collection) Size() uint64 { + return c.omap.Count() +} + type ByteStringValue struct { data []byte size uint32 From daf060886ecbaedc983eceabaa243e765be65ede Mon Sep 17 00:00:00 2001 From: ramtinms Date: Mon, 15 Jan 2024 14:37:47 -0800 Subject: [PATCH 3/5] bug fix --- fvm/evm/emulator/state/base.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fvm/evm/emulator/state/base.go b/fvm/evm/emulator/state/base.go index 4b0e0c2bd5a..32f0f378ae3 100644 --- a/fvm/evm/emulator/state/base.go +++ b/fvm/evm/emulator/state/base.go @@ -131,11 +131,12 @@ func (v *BaseView) GetCode(addr gethCommon.Address) ([]byte, error) { // GetCodeHash returns the code hash of an address // -// for non-existent accounts or accounts without a code (e.g. EOAs) it returns default empty +// for non-existent accounts it returns gethCommon.Hash{} +// and for accounts without a code (e.g. EOAs) it returns default empty // hash value (gethTypes.EmptyCodeHash) func (v *BaseView) GetCodeHash(addr gethCommon.Address) (gethCommon.Hash, error) { acc, err := v.getAccount(addr) - codeHash := gethTypes.EmptyCodeHash + codeHash := gethCommon.Hash{} if acc != nil { codeHash = acc.CodeHash } @@ -426,7 +427,7 @@ func (v *BaseView) getCode(addr gethCommon.Address) ([]byte, error) { return nil, err } code = codeCont.Code() - if code != nil { + if len(code) > 0 { v.cachedCodes[addr] = code } return code, nil From 21c799a12d24306c1da7c253532ac8d3246a3a97 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Wed, 17 Jan 2024 19:21:34 -0800 Subject: [PATCH 4/5] apply PR feedback --- fvm/evm/emulator/state/base.go | 6 +++--- fvm/evm/emulator/state/code.go | 23 ++++++++++++++++++++--- fvm/evm/emulator/state/code_test.go | 2 +- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/fvm/evm/emulator/state/base.go b/fvm/evm/emulator/state/base.go index 32f0f378ae3..c2b83ce9fa3 100644 --- a/fvm/evm/emulator/state/base.go +++ b/fvm/evm/emulator/state/base.go @@ -486,7 +486,7 @@ func (v *BaseView) removeCode(codeHash gethCommon.Hash) error { if cc.DecRefCount() { return v.codes.Remove(codeHash.Bytes()) } - return v.codes.Set(codeHash.Bytes(), cc.Encoded()) + return v.codes.Set(codeHash.Bytes(), cc.Encode()) } func (v *BaseView) addCode(code []byte, codeHash gethCommon.Hash) error { @@ -496,7 +496,7 @@ func (v *BaseView) addCode(code []byte, codeHash gethCommon.Hash) error { } // if is the first time the code is getting deployed if len(encoded) == 0 { - return v.codes.Set(codeHash.Bytes(), NewCodeContainer(code).Encoded()) + return v.codes.Set(codeHash.Bytes(), NewCodeContainer(code).Encode()) } // otherwise update the cc @@ -505,7 +505,7 @@ func (v *BaseView) addCode(code []byte, codeHash gethCommon.Hash) error { return err } cc.IncRefCount() - return v.codes.Set(codeHash.Bytes(), cc.Encoded()) + return v.codes.Set(codeHash.Bytes(), cc.Encode()) } func (v *BaseView) getSlot(sk types.SlotAddress) (gethCommon.Hash, error) { diff --git a/fvm/evm/emulator/state/code.go b/fvm/evm/emulator/state/code.go index 176859fbd18..bb893a30d39 100644 --- a/fvm/evm/emulator/state/code.go +++ b/fvm/evm/emulator/state/code.go @@ -8,7 +8,9 @@ import ( // CodeContainer contains codes and keeps // track of reference counts type CodeContainer struct { - code []byte + code []byte + // keeping encoded so we can reuse it later + buffer []byte refCount uint64 } @@ -27,6 +29,7 @@ func CodeContainerFromEncoded(encoded []byte) (*CodeContainer, error) { } return &CodeContainer{ refCount: binary.BigEndian.Uint64(encoded[:8]), + buffer: encoded, // keep encoded as buffer for future use code: encoded[8:], }, nil } @@ -49,13 +52,27 @@ func (cc *CodeContainer) IncRefCount() { // DecRefCount decrement the ref count and // returns true if the ref has reached to zero func (cc *CodeContainer) DecRefCount() bool { + // check if ref is already zero + // this condition should never happen + // but better to be here to prevent underflow + if cc.refCount == 0 { + return true + } cc.refCount-- return cc.refCount == 0 } // Encoded returns the encoded content of the code container -func (cc *CodeContainer) Encoded() []byte { - encoded := make([]byte, 8+len(cc.code)) +func (cc *CodeContainer) Encode() []byte { + // try using the buffer if possible to avoid + // extra allocations + encodedLen := 8 + len(cc.code) + var encoded []byte + if len(cc.buffer) < encodedLen { + encoded = make([]byte, encodedLen) + } else { + encoded = cc.buffer[:encodedLen] + } binary.BigEndian.PutUint64(encoded[:8], cc.refCount) copy(encoded[8:], cc.code) return encoded diff --git a/fvm/evm/emulator/state/code_test.go b/fvm/evm/emulator/state/code_test.go index 31178cd4885..2a351e08073 100644 --- a/fvm/evm/emulator/state/code_test.go +++ b/fvm/evm/emulator/state/code_test.go @@ -21,7 +21,7 @@ func TestCodeContainer(t *testing.T) { require.Equal(t, uint64(2), cc.RefCount()) // test encoding - encoded := cc.Encoded() + encoded := cc.Encode() cc, err := state.CodeContainerFromEncoded(encoded) require.NoError(t, err) require.Equal(t, uint64(2), cc.RefCount()) From 9240c257410dbd38bcd1f060b4c66426471d4198 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Mon, 22 Jan 2024 11:26:46 -0800 Subject: [PATCH 5/5] update test --- fvm/evm/emulator/state/base_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fvm/evm/emulator/state/base_test.go b/fvm/evm/emulator/state/base_test.go index 3178513dda2..af4696abdfe 100644 --- a/fvm/evm/emulator/state/base_test.go +++ b/fvm/evm/emulator/state/base_test.go @@ -34,7 +34,7 @@ func TestBaseView(t *testing.T) { big.NewInt(0), uint64(0), nil, - gethTypes.EmptyCodeHash, + gethCommon.Hash{}, ) // create an account with code @@ -124,7 +124,7 @@ func TestBaseView(t *testing.T) { big.NewInt(0), uint64(0), nil, - gethTypes.EmptyCodeHash, + gethCommon.Hash{}, ) // commit the changes and create a new baseview @@ -141,7 +141,7 @@ func TestBaseView(t *testing.T) { big.NewInt(0), uint64(0), nil, - gethTypes.EmptyCodeHash, + gethCommon.Hash{}, ) })