From 6c2907f53907b9751a5f2b0cf219ce6bb494de59 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 7 Nov 2023 18:50:45 -0800 Subject: [PATCH 01/13] add executing block snapshot --- .../execution/storehouse/back_end_snapshot.go | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 engine/execution/storehouse/back_end_snapshot.go diff --git a/engine/execution/storehouse/back_end_snapshot.go b/engine/execution/storehouse/back_end_snapshot.go new file mode 100644 index 00000000000..2629cdbc1ab --- /dev/null +++ b/engine/execution/storehouse/back_end_snapshot.go @@ -0,0 +1,66 @@ +package storehouse + +import ( + "sync" + + "github.com/onflow/flow-go/engine/execution" + "github.com/onflow/flow-go/fvm/storage/snapshot" + "github.com/onflow/flow-go/model/flow" +) + +var _ snapshot.StorageSnapshot = (*BlockEndStateSnapshot)(nil) + +// BlockEndStateSnapshot represents the storage at the end of a block. +type BlockEndStateSnapshot struct { + storage execution.RegisterStore + + blockID flow.Identifier + height uint64 + + mutex sync.RWMutex + readCache map[flow.RegisterID]flow.RegisterValue // cache the reads from storage at baseBlock +} + +// the caller must ensure the block height is for the given block +func NewBlockEndStateSnapshot( + storage execution.RegisterStore, + blockID flow.Identifier, + height uint64, +) *BlockEndStateSnapshot { + return &BlockEndStateSnapshot{ + storage: storage, + blockID: blockID, + height: height, + readCache: make(map[flow.RegisterID]flow.RegisterValue), + } +} + +func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) { + value, ok := s.getFromCache(id) + if ok { + return value, nil + } + + value, err := s.getFromStorage(id) + if err != nil { + return nil, err + } + + s.mutex.Lock() + defer s.mutex.Unlock() + + s.readCache[id] = value + return value, nil +} + +func (s *BlockEndStateSnapshot) getFromCache(id flow.RegisterID) (flow.RegisterValue, bool) { + s.mutex.RLock() + defer s.mutex.RUnlock() + + value, ok := s.readCache[id] + return value, ok +} + +func (s *BlockEndStateSnapshot) getFromStorage(id flow.RegisterID) (flow.RegisterValue, error) { + return s.storage.GetRegister(s.height, s.blockID, id) +} From 1550ba3362332995481ef8f64c73bbc43d266e3b Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 7 Nov 2023 19:20:41 -0800 Subject: [PATCH 02/13] reuse convert functions --- .../{back_end_snapshot.go => block_end_snapshot.go} | 0 engine/execution/storehouse/block_end_snapshot_test.go | 6 ++++++ 2 files changed, 6 insertions(+) rename engine/execution/storehouse/{back_end_snapshot.go => block_end_snapshot.go} (100%) create mode 100644 engine/execution/storehouse/block_end_snapshot_test.go diff --git a/engine/execution/storehouse/back_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go similarity index 100% rename from engine/execution/storehouse/back_end_snapshot.go rename to engine/execution/storehouse/block_end_snapshot.go diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go new file mode 100644 index 00000000000..3ea435a6d8f --- /dev/null +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -0,0 +1,6 @@ +package storehouse + +import "testing" + +func TestBlockEndSnapshot(t *testing.T) { +} From 05bdd87843155cd952fbffd0ee76fe63c91490f2 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 9 Nov 2023 11:36:29 -0800 Subject: [PATCH 03/13] add block_end_snapshot tests --- .../storehouse/block_end_snapshot.go | 25 ++++- .../storehouse/block_end_snapshot_test.go | 101 +++++++++++++++++- 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index 2629cdbc1ab..0ba21a0b3d2 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -1,11 +1,13 @@ package storehouse import ( + "errors" "sync" "github.com/onflow/flow-go/engine/execution" "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/storage" ) var _ snapshot.StorageSnapshot = (*BlockEndStateSnapshot)(nil) @@ -35,22 +37,41 @@ func NewBlockEndStateSnapshot( } } +// Get returns the value of the register with the given register ID. +// It returns: +// - (value, nil) if the register exists +// - (nil, storage.ErrNotFound) if the register does not exist +// - (nil, storage.ErrHeightNotIndexed) if the height is below the first height that is indexed. +// - (nil, storehouse.ErrNotExecuted) if the block is not executed yet +// - (nil, storehouse.ErrNotExecuted) if the block is conflicting iwth finalized block +// - (nil, err) for any other exceptions func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) { value, ok := s.getFromCache(id) if ok { + if value == nil { + return nil, storage.ErrNotFound + } return value, nil } value, err := s.getFromStorage(id) if err != nil { - return nil, err + if errors.Is(err, storage.ErrNotFound) { + // if the error is not found, we return a nil RegisterValue, + // in this case, the nil value can be cached, because the storage will not change it + value = nil + } else { + // if the error is not found, such as storage.ErrHeightNotIndexed, storehouse.ErrNotExecuted + // we return the error without caching + return nil, err + } } s.mutex.Lock() defer s.mutex.Unlock() s.readCache[id] = value - return value, nil + return value, err } func (s *BlockEndStateSnapshot) getFromCache(id flow.RegisterID) (flow.RegisterValue, bool) { diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index 3ea435a6d8f..7fa5297d3e2 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -1,6 +1,103 @@ -package storehouse +package storehouse_test -import "testing" +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "go.uber.org/atomic" + + executionMock "github.com/onflow/flow-go/engine/execution/mock" + "github.com/onflow/flow-go/engine/execution/storehouse" + "github.com/onflow/flow-go/storage" + "github.com/onflow/flow-go/utils/unittest" +) func TestBlockEndSnapshot(t *testing.T) { + t.Run("Get register", func(t *testing.T) { + header := unittest.BlockHeaderFixture() + + // create mock for storage + store := &executionMock.RegisterStore{} + reg := unittest.MakeOwnerReg("key", "value") + store.On("GetRegister", header.Height, header.ID(), reg.Key).Return(reg.Value, nil).Once() + snapshot := storehouse.NewBlockEndStateSnapshot(store, header.ID(), header.Height) + + // test get from storage + value, err := snapshot.Get(reg.Key) + require.NoError(t, err) + require.Equal(t, reg.Value, value) + + // test get from cache + value, err = snapshot.Get(reg.Key) + require.NoError(t, err) + require.Equal(t, reg.Value, value) + + // test get non existing register + unknownReg := unittest.MakeOwnerReg("unknown", "unknown") + store.On("GetRegister", header.Height, header.ID(), unknownReg.Key). + Return(nil, fmt.Errorf("fail: %w", storage.ErrNotFound)).Once() + + value, err = snapshot.Get(unknownReg.Key) + require.Error(t, err) + require.True(t, errors.Is(err, storage.ErrNotFound)) + require.Nil(t, value) + + // test get non existing register from cache + _, err = snapshot.Get(unknownReg.Key) + require.Error(t, err) + require.True(t, errors.Is(err, storage.ErrNotFound)) + + // test getting storage.ErrHeightNotIndexed error + heightNotIndexed := unittest.MakeOwnerReg("height not index", "height not index") + store.On("GetRegister", header.Height, header.ID(), heightNotIndexed.Key). + Return(nil, fmt.Errorf("fail: %w", storage.ErrHeightNotIndexed)). + Twice() // to verify the result is not cached + + // verify getting the correct error + _, err = snapshot.Get(heightNotIndexed.Key) + require.Error(t, err) + require.True(t, errors.Is(err, storage.ErrHeightNotIndexed)) + + // verify result is not cached + _, err = snapshot.Get(heightNotIndexed.Key) + require.Error(t, err) + require.True(t, errors.Is(err, storage.ErrHeightNotIndexed)) + + // test getting storage.ErrNotExecuted error + heightNotExecuted := unittest.MakeOwnerReg("height not executed", "height not executed") + counter := atomic.NewInt32(0) + mocked := store.On("GetRegister", header.Height, header.ID(), heightNotExecuted.Key) + mocked.Times(2) // the first time hit error, the second get value, the third time is cached + mocked.RunFn = func(args mock.Arguments) { + counter.Inc() + // the first call should return error + if counter.Load() == 1 { + mocked.ReturnArguments = mock.Arguments{nil, fmt.Errorf("fail: %w", storehouse.ErrNotExecuted)} + return + } + // the second call, it returns value + mocked.ReturnArguments = mock.Arguments{heightNotExecuted.Value, nil} + } + + // first time should return error + _, err = snapshot.Get(heightNotExecuted.Key) + require.Error(t, err) + require.True(t, errors.Is(err, storehouse.ErrNotExecuted)) + + // second time should return value + value, err = snapshot.Get(heightNotExecuted.Key) + require.NoError(t, err) + require.Equal(t, heightNotExecuted.Value, value) + + // third time should be cached + value, err = snapshot.Get(heightNotExecuted.Key) + require.NoError(t, err) + require.Equal(t, heightNotExecuted.Value, value) + + store.AssertExpectations(t) + }) + } From e85ed70ee615a5f770b7fa46c5e0953384926b3b Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 16 Nov 2023 08:58:56 -0800 Subject: [PATCH 04/13] remove changes --- engine/execution/storehouse.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/engine/execution/storehouse.go b/engine/execution/storehouse.go index ab3ebf66e90..2618165817e 100644 --- a/engine/execution/storehouse.go +++ b/engine/execution/storehouse.go @@ -1,7 +1,6 @@ package execution import ( - "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/finalizedreader" "github.com/onflow/flow-go/storage" @@ -106,9 +105,3 @@ type WALReader interface { // It returns EOF when there are no more entries. Next() (height uint64, registers flow.RegisterEntries, err error) } - -type ExtendableStorageSnapshot interface { - snapshot.StorageSnapshot - Extend(newCommit flow.StateCommitment, updatedRegisters map[flow.RegisterID]flow.RegisterValue) ExtendableStorageSnapshot - Commitment() flow.StateCommitment -} From 9182729ea17584a8d7581496771e527aa94d4415 Mon Sep 17 00:00:00 2001 From: Leo Zhang Date: Thu, 16 Nov 2023 08:59:59 -0800 Subject: [PATCH 05/13] Apply suggestions from code review Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- engine/execution/storehouse/block_end_snapshot.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index 0ba21a0b3d2..5d19e6734ba 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -43,7 +43,7 @@ func NewBlockEndStateSnapshot( // - (nil, storage.ErrNotFound) if the register does not exist // - (nil, storage.ErrHeightNotIndexed) if the height is below the first height that is indexed. // - (nil, storehouse.ErrNotExecuted) if the block is not executed yet -// - (nil, storehouse.ErrNotExecuted) if the block is conflicting iwth finalized block +// - (nil, storehouse.ErrNotExecuted) if the block is conflicting with finalized block // - (nil, err) for any other exceptions func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) { value, ok := s.getFromCache(id) @@ -61,7 +61,7 @@ func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, err // in this case, the nil value can be cached, because the storage will not change it value = nil } else { - // if the error is not found, such as storage.ErrHeightNotIndexed, storehouse.ErrNotExecuted + // if the error is not ErrNotFound, such as storage.ErrHeightNotIndexed, storehouse.ErrNotExecuted // we return the error without caching return nil, err } From 5e22f683995e24cb4f96f91e7515e10b406eb430 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 16 Nov 2023 09:45:11 -0800 Subject: [PATCH 06/13] add todo --- engine/execution/storehouse/block_end_snapshot.go | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index 5d19e6734ba..e53fe69e0d7 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -70,6 +70,7 @@ func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, err s.mutex.Lock() defer s.mutex.Unlock() + // TODO: consider adding a limit/eviction policy for the cache s.readCache[id] = value return value, err } From 4bdab7133ef84acfbdd3c4131df9840de01ea873 Mon Sep 17 00:00:00 2001 From: Leo Zhang Date: Thu, 16 Nov 2023 13:02:16 -0800 Subject: [PATCH 07/13] Update engine/execution/storehouse/block_end_snapshot_test.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- .../storehouse/block_end_snapshot_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index 7fa5297d3e2..bb5855220ed 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -69,18 +69,18 @@ func TestBlockEndSnapshot(t *testing.T) { // test getting storage.ErrNotExecuted error heightNotExecuted := unittest.MakeOwnerReg("height not executed", "height not executed") counter := atomic.NewInt32(0) - mocked := store.On("GetRegister", header.Height, header.ID(), heightNotExecuted.Key) - mocked.Times(2) // the first time hit error, the second get value, the third time is cached - mocked.RunFn = func(args mock.Arguments) { + store. + On("GetRegister", header.Height, header.ID(), heightNotExecuted.Key). + Return(func(uint64, flow.Identifier, flow.RegisterID) (flow.RegisterValue, error) { counter.Inc() // the first call should return error if counter.Load() == 1 { - mocked.ReturnArguments = mock.Arguments{nil, fmt.Errorf("fail: %w", storehouse.ErrNotExecuted)} - return + return nil, fmt.Errorf("fail: %w", storehouse.ErrNotExecuted) } // the second call, it returns value - mocked.ReturnArguments = mock.Arguments{heightNotExecuted.Value, nil} - } + return heightNotExecuted.Value, nil + }). + Times(2) // first time should return error _, err = snapshot.Get(heightNotExecuted.Key) From c6f93b1ef3ec29f5f25536fc34a4d202de1a3a36 Mon Sep 17 00:00:00 2001 From: Leo Zhang Date: Thu, 16 Nov 2023 13:02:22 -0800 Subject: [PATCH 08/13] Update engine/execution/storehouse/block_end_snapshot_test.go Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com> --- engine/execution/storehouse/block_end_snapshot_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index bb5855220ed..61126494546 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -20,7 +20,7 @@ func TestBlockEndSnapshot(t *testing.T) { header := unittest.BlockHeaderFixture() // create mock for storage - store := &executionMock.RegisterStore{} + store := executionMock.NewRegisterStore(t) reg := unittest.MakeOwnerReg("key", "value") store.On("GetRegister", header.Height, header.ID(), reg.Key).Return(reg.Value, nil).Once() snapshot := storehouse.NewBlockEndStateSnapshot(store, header.ID(), header.Height) From 8d49e08e0564671c35cbab51ab49112778be4b6f Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 16 Nov 2023 13:04:29 -0800 Subject: [PATCH 09/13] fix lint --- .../storehouse/block_end_snapshot_test.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index 61126494546..3177d1310fe 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -5,12 +5,12 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/atomic" executionMock "github.com/onflow/flow-go/engine/execution/mock" "github.com/onflow/flow-go/engine/execution/storehouse" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/storage" "github.com/onflow/flow-go/utils/unittest" ) @@ -69,18 +69,18 @@ func TestBlockEndSnapshot(t *testing.T) { // test getting storage.ErrNotExecuted error heightNotExecuted := unittest.MakeOwnerReg("height not executed", "height not executed") counter := atomic.NewInt32(0) - store. - On("GetRegister", header.Height, header.ID(), heightNotExecuted.Key). - Return(func(uint64, flow.Identifier, flow.RegisterID) (flow.RegisterValue, error) { - counter.Inc() - // the first call should return error - if counter.Load() == 1 { - return nil, fmt.Errorf("fail: %w", storehouse.ErrNotExecuted) - } - // the second call, it returns value - return heightNotExecuted.Value, nil - }). - Times(2) + store. + On("GetRegister", header.Height, header.ID(), heightNotExecuted.Key). + Return(func(uint64, flow.Identifier, flow.RegisterID) (flow.RegisterValue, error) { + counter.Inc() + // the first call should return error + if counter.Load() == 1 { + return nil, fmt.Errorf("fail: %w", storehouse.ErrNotExecuted) + } + // the second call, it returns value + return heightNotExecuted.Value, nil + }). + Times(2) // first time should return error _, err = snapshot.Get(heightNotExecuted.Key) From d6c45f6507144a8e5f6d6f36cacdb4d6ebb72ef8 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Mon, 20 Nov 2023 09:39:53 -0800 Subject: [PATCH 10/13] update comment --- engine/execution/storehouse/block_end_snapshot.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index e53fe69e0d7..eaffb9dcd97 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -40,7 +40,7 @@ func NewBlockEndStateSnapshot( // Get returns the value of the register with the given register ID. // It returns: // - (value, nil) if the register exists -// - (nil, storage.ErrNotFound) if the register does not exist +// - (nil, nil) if the register does not exist // - (nil, storage.ErrHeightNotIndexed) if the height is below the first height that is indexed. // - (nil, storehouse.ErrNotExecuted) if the block is not executed yet // - (nil, storehouse.ErrNotExecuted) if the block is conflicting with finalized block From 6a57758ce85e3718990780a343931ada0fbc3758 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Mon, 20 Nov 2023 09:47:08 -0800 Subject: [PATCH 11/13] add back interface --- engine/execution/storehouse.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/engine/execution/storehouse.go b/engine/execution/storehouse.go index 2618165817e..ab3ebf66e90 100644 --- a/engine/execution/storehouse.go +++ b/engine/execution/storehouse.go @@ -1,6 +1,7 @@ package execution import ( + "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/finalizedreader" "github.com/onflow/flow-go/storage" @@ -105,3 +106,9 @@ type WALReader interface { // It returns EOF when there are no more entries. Next() (height uint64, registers flow.RegisterEntries, err error) } + +type ExtendableStorageSnapshot interface { + snapshot.StorageSnapshot + Extend(newCommit flow.StateCommitment, updatedRegisters map[flow.RegisterID]flow.RegisterValue) ExtendableStorageSnapshot + Commitment() flow.StateCommitment +} From 1894f9e880da1af60ce334fb8b4a50247f383b65 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 21 Nov 2023 09:02:03 -0800 Subject: [PATCH 12/13] update getFromStorage --- .../storehouse/block_end_snapshot.go | 23 +++++++++++-------- .../storehouse/block_end_snapshot_test.go | 3 +-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index eaffb9dcd97..5c53da58014 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -56,15 +56,7 @@ func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, err value, err := s.getFromStorage(id) if err != nil { - if errors.Is(err, storage.ErrNotFound) { - // if the error is not found, we return a nil RegisterValue, - // in this case, the nil value can be cached, because the storage will not change it - value = nil - } else { - // if the error is not ErrNotFound, such as storage.ErrHeightNotIndexed, storehouse.ErrNotExecuted - // we return the error without caching - return nil, err - } + return nil, err } s.mutex.Lock() @@ -84,5 +76,16 @@ func (s *BlockEndStateSnapshot) getFromCache(id flow.RegisterID) (flow.RegisterV } func (s *BlockEndStateSnapshot) getFromStorage(id flow.RegisterID) (flow.RegisterValue, error) { - return s.storage.GetRegister(s.height, s.blockID, id) + value, err := s.storage.GetRegister(s.height, s.blockID, id) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + // if the error is not found, we return a nil RegisterValue, + // in this case, the nil value can be cached, because the storage will not change it + return nil, nil + } + // if the error is not ErrNotFound, such as storage.ErrHeightNotIndexed, storehouse.ErrNotExecuted + // we return the error without caching + return nil, err + } + return value, nil } diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index 3177d1310fe..f3b0936193c 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -41,8 +41,7 @@ func TestBlockEndSnapshot(t *testing.T) { Return(nil, fmt.Errorf("fail: %w", storage.ErrNotFound)).Once() value, err = snapshot.Get(unknownReg.Key) - require.Error(t, err) - require.True(t, errors.Is(err, storage.ErrNotFound)) + require.NoError(t, err) require.Nil(t, value) // test get non existing register from cache From c212feb18c8a52a23e6d5fc4a1552e09490db82a Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Wed, 22 Nov 2023 08:20:33 -0800 Subject: [PATCH 13/13] handle not found case --- engine/execution/storehouse/block_end_snapshot.go | 3 --- engine/execution/storehouse/block_end_snapshot_test.go | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/engine/execution/storehouse/block_end_snapshot.go b/engine/execution/storehouse/block_end_snapshot.go index 5c53da58014..bf7718a9543 100644 --- a/engine/execution/storehouse/block_end_snapshot.go +++ b/engine/execution/storehouse/block_end_snapshot.go @@ -48,9 +48,6 @@ func NewBlockEndStateSnapshot( func (s *BlockEndStateSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) { value, ok := s.getFromCache(id) if ok { - if value == nil { - return nil, storage.ErrNotFound - } return value, nil } diff --git a/engine/execution/storehouse/block_end_snapshot_test.go b/engine/execution/storehouse/block_end_snapshot_test.go index f3b0936193c..3787ec2d552 100644 --- a/engine/execution/storehouse/block_end_snapshot_test.go +++ b/engine/execution/storehouse/block_end_snapshot_test.go @@ -46,8 +46,8 @@ func TestBlockEndSnapshot(t *testing.T) { // test get non existing register from cache _, err = snapshot.Get(unknownReg.Key) - require.Error(t, err) - require.True(t, errors.Is(err, storage.ErrNotFound)) + require.NoError(t, err) + require.Nil(t, value) // test getting storage.ErrHeightNotIndexed error heightNotIndexed := unittest.MakeOwnerReg("height not index", "height not index")