From fb44a86dcb4488ce92abeb13615120fb4c7fc529 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Fri, 10 Nov 2023 09:47:44 -0800 Subject: [PATCH 1/6] add extending block snapshot --- engine/execution/storehouse.go | 7 ++ .../storehouse/executing_block_snapshot.go | 78 +++++++++++++++++++ .../executing_block_snapshot_test.go | 71 +++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 engine/execution/storehouse/executing_block_snapshot.go create mode 100644 engine/execution/storehouse/executing_block_snapshot_test.go diff --git a/engine/execution/storehouse.go b/engine/execution/storehouse.go index 2618165817e..5f20522e148 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 flow.RegisterEntries) ExtendableStorageSnapshot + Commitment() flow.StateCommitment +} diff --git a/engine/execution/storehouse/executing_block_snapshot.go b/engine/execution/storehouse/executing_block_snapshot.go new file mode 100644 index 00000000000..4f8b77fb953 --- /dev/null +++ b/engine/execution/storehouse/executing_block_snapshot.go @@ -0,0 +1,78 @@ +package storehouse + +import ( + "github.com/onflow/flow-go/engine/execution" + "github.com/onflow/flow-go/fvm/storage/snapshot" + "github.com/onflow/flow-go/model/flow" +) + +var _ execution.ExtendableStorageSnapshot = (*ExecutingBlockSnapshot)(nil) + +// ExecutingBlockSnapshot is a snapshot of the storage at an executed collection. +// It starts with a storage snapshot at the end of previous block, +// The register updates at the executed collection at baseHeight + 1 are cached in +// a map, such that retrieving register values at the snapshot will first check +// the cache, and then the storage. +type ExecutingBlockSnapshot struct { + // the snapshot at the end of previous block + previous snapshot.StorageSnapshot + + commitment flow.StateCommitment + registerUpdates map[flow.RegisterID]flow.RegisterValue +} + +// create a new storage snapshot for an executed collection +// at the base block at height h - 1 +func NewExecutingBlockSnapshot( + previous snapshot.StorageSnapshot, + // the statecommitment of a block at height h + commitment flow.StateCommitment, +) *ExecutingBlockSnapshot { + return &ExecutingBlockSnapshot{ + previous: previous, + commitment: commitment, + registerUpdates: make(map[flow.RegisterID]flow.RegisterValue), + } +} + +// Get returns the register value at the snapshot. +func (s *ExecutingBlockSnapshot) Get(id flow.RegisterID) (flow.RegisterValue, error) { + // get from latest updates first + value, ok := s.getFromUpdates(id) + if ok { + return value, nil + } + + // get from BlockEndStateSnapshot at previous block + value, err := s.previous.Get(id) + return value, err +} + +func (s *ExecutingBlockSnapshot) getFromUpdates(id flow.RegisterID) (flow.RegisterValue, bool) { + value, ok := s.registerUpdates[id] + return value, ok +} + +// Extend returns a new storage snapshot at the same block but but for a different state commitment, +// which contains the given trieUpdate +// Usually it's used to create a new storage snapshot at the next executed collection. +// The trieUpdate contains the register updates at the executed collection. +func (s *ExecutingBlockSnapshot) Extend(newCommit flow.StateCommitment, updatedRegisters flow.RegisterEntries) execution.ExtendableStorageSnapshot { + updates := make(map[flow.RegisterID]flow.RegisterValue) + + // add the old updates + // overwrite with new updates + for _, entry := range updatedRegisters { + updates[entry.Key] = entry.Value + } + + return &ExecutingBlockSnapshot{ + previous: s.previous, + commitment: newCommit, + registerUpdates: updates, + } +} + +func (s *ExecutingBlockSnapshot) Commitment() flow.StateCommitment { + return s.commitment +} diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go new file mode 100644 index 00000000000..06b61da0de0 --- /dev/null +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -0,0 +1,71 @@ +package storehouse_test + +import ( + "testing" + + "github.com/onflow/flow-go/engine/execution/storehouse" + "github.com/onflow/flow-go/fvm/storage/snapshot" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" + "github.com/stretchr/testify/require" +) + +func TestExtendingBlockSnapshot(t *testing.T) { + t.Run("Get register", func(t *testing.T) { + reg1 := makeReg("key1", "val1") + base := snapshot.MapStorageSnapshot{ + reg1.Key: reg1.Value, + } + baseCommit := unittest.StateCommitmentFixture() + snap := storehouse.NewExecutingBlockSnapshot(base, baseCommit) + + // should get value + value, err := snap.Get(reg1.Key) + require.NoError(t, err) + require.Equal(t, reg1.Value, value) + + // should get nil for unknown register + unknown := makeReg("unknown", "unknownV") + value, err = snap.Get(unknown.Key) + require.NoError(t, err) + require.Equal(t, nil, value) + }) + + t.Run("Extend snapshot", func(t *testing.T) { + reg1 := makeReg("key1", "val1") + reg2 := makeReg("key2", "val2") + base := snapshot.MapStorageSnapshot{ + reg1.Key: reg1.Value, + reg2.Key: reg2.Value, + } + snap1Commit := unittest.StateCommitmentFixture() + snap1 := storehouse.NewExecutingBlockSnapshot(base, snap1Commit) + + updatedReg2 := makeReg("key2", "val22") + reg3 := makeReg("key3", "val3") + snap2Commit := unittest.StateCommitmentFixture() + snap2 := snap1.Extend(snap2Commit, flow.RegisterEntries{ + updatedReg2, reg3, + }) + + // should get un-changed value + value, err := snap2.Get(reg1.Key) + require.NoError(t, err) + require.Equal(t, reg1.Value, value) + + // should get nil for unknown register + unknown := makeReg("unknown", "unknownV") + value, err = snap2.Get(unknown.Key) + require.NoError(t, err) + require.Equal(t, nil, value) + + // should get updated value + value, err = snap2.Get(reg2.Key) + require.NoError(t, err) + require.Equal(t, updatedReg2.Value, value) + }) +} + +func makeReg(key string, value string) flow.RegisterEntry { + panic("") +} From 3e1a67739ad5f984e8c70927944636e71531758b Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Fri, 10 Nov 2023 11:47:15 -0800 Subject: [PATCH 2/6] fix tests --- .../storehouse/executing_block_snapshot_test.go | 6 +++--- utils/unittest/fixtures.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go index 06b61da0de0..8afaaa88744 100644 --- a/engine/execution/storehouse/executing_block_snapshot_test.go +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -28,7 +28,7 @@ func TestExtendingBlockSnapshot(t *testing.T) { unknown := makeReg("unknown", "unknownV") value, err = snap.Get(unknown.Key) require.NoError(t, err) - require.Equal(t, nil, value) + require.Equal(t, []byte(nil), value) }) t.Run("Extend snapshot", func(t *testing.T) { @@ -57,7 +57,7 @@ func TestExtendingBlockSnapshot(t *testing.T) { unknown := makeReg("unknown", "unknownV") value, err = snap2.Get(unknown.Key) require.NoError(t, err) - require.Equal(t, nil, value) + require.Equal(t, []byte(nil), value) // should get updated value value, err = snap2.Get(reg2.Key) @@ -67,5 +67,5 @@ func TestExtendingBlockSnapshot(t *testing.T) { } func makeReg(key string, value string) flow.RegisterEntry { - panic("") + return unittest.MakeOwnerReg(key, value) } diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go index 8da04f65fc9..0483e6ce406 100644 --- a/utils/unittest/fixtures.go +++ b/utils/unittest/fixtures.go @@ -2764,3 +2764,13 @@ func RegisterEntryFixture() flow.RegisterEntry { Value: val, } } + +func MakeOwnerReg(key string, value string) flow.RegisterEntry { + return flow.RegisterEntry{ + Key: flow.RegisterID{ + Owner: "owner", + Key: key, + }, + Value: []byte(value), + } +} From edfce9eeeb87c5df061f525851a9193609f26ff7 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 14 Nov 2023 09:09:46 -0800 Subject: [PATCH 3/6] fix lint --- engine/execution/storehouse/executing_block_snapshot_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go index 8afaaa88744..af9182b7e32 100644 --- a/engine/execution/storehouse/executing_block_snapshot_test.go +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -3,11 +3,12 @@ package storehouse_test import ( "testing" + "github.com/stretchr/testify/require" + "github.com/onflow/flow-go/engine/execution/storehouse" "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/utils/unittest" - "github.com/stretchr/testify/require" ) func TestExtendingBlockSnapshot(t *testing.T) { From bfe8630b5612410e867757fd7b4fb912e87a8f37 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 16 Nov 2023 09:12:25 -0800 Subject: [PATCH 4/6] update interface --- engine/execution/storehouse.go | 2 +- .../storehouse/executing_block_snapshot.go | 16 +++-------- .../executing_block_snapshot_test.go | 27 +++++++++++++++---- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/engine/execution/storehouse.go b/engine/execution/storehouse.go index 5f20522e148..ab3ebf66e90 100644 --- a/engine/execution/storehouse.go +++ b/engine/execution/storehouse.go @@ -109,6 +109,6 @@ type WALReader interface { type ExtendableStorageSnapshot interface { snapshot.StorageSnapshot - Extend(newCommit flow.StateCommitment, updatedRegisters flow.RegisterEntries) ExtendableStorageSnapshot + Extend(newCommit flow.StateCommitment, updatedRegisters map[flow.RegisterID]flow.RegisterValue) ExtendableStorageSnapshot Commitment() flow.StateCommitment } diff --git a/engine/execution/storehouse/executing_block_snapshot.go b/engine/execution/storehouse/executing_block_snapshot.go index 4f8b77fb953..98c5088e34b 100644 --- a/engine/execution/storehouse/executing_block_snapshot.go +++ b/engine/execution/storehouse/executing_block_snapshot.go @@ -54,20 +54,12 @@ func (s *ExecutingBlockSnapshot) getFromUpdates(id flow.RegisterID) (flow.Regist } // Extend returns a new storage snapshot at the same block but but for a different state commitment, -// which contains the given trieUpdate +// which contains the given registerUpdates // Usually it's used to create a new storage snapshot at the next executed collection. -// The trieUpdate contains the register updates at the executed collection. -func (s *ExecutingBlockSnapshot) Extend(newCommit flow.StateCommitment, updatedRegisters flow.RegisterEntries) execution.ExtendableStorageSnapshot { - updates := make(map[flow.RegisterID]flow.RegisterValue) - - // add the old updates - // overwrite with new updates - for _, entry := range updatedRegisters { - updates[entry.Key] = entry.Value - } - +// The registerUpdates contains the register updates at the executed collection. +func (s *ExecutingBlockSnapshot) Extend(newCommit flow.StateCommitment, updates map[flow.RegisterID]flow.RegisterValue) execution.ExtendableStorageSnapshot { return &ExecutingBlockSnapshot{ - previous: s.previous, + previous: s, commitment: newCommit, registerUpdates: updates, } diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go index af9182b7e32..a82857fa945 100644 --- a/engine/execution/storehouse/executing_block_snapshot_test.go +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -39,14 +39,15 @@ func TestExtendingBlockSnapshot(t *testing.T) { reg1.Key: reg1.Value, reg2.Key: reg2.Value, } - snap1Commit := unittest.StateCommitmentFixture() - snap1 := storehouse.NewExecutingBlockSnapshot(base, snap1Commit) + // snap1: { key1: val1, key2: val2 } + snap1 := storehouse.NewExecutingBlockSnapshot(base, unittest.StateCommitmentFixture()) updatedReg2 := makeReg("key2", "val22") reg3 := makeReg("key3", "val3") - snap2Commit := unittest.StateCommitmentFixture() - snap2 := snap1.Extend(snap2Commit, flow.RegisterEntries{ - updatedReg2, reg3, + // snap2: { key1: val1, key2: val22, key3: val3 } + snap2 := snap1.Extend(unittest.StateCommitmentFixture(), map[flow.RegisterID]flow.RegisterValue{ + updatedReg2.Key: updatedReg2.Value, + reg3.Key: reg3.Value, }) // should get un-changed value @@ -64,6 +65,22 @@ func TestExtendingBlockSnapshot(t *testing.T) { value, err = snap2.Get(reg2.Key) require.NoError(t, err) require.Equal(t, updatedReg2.Value, value) + + updatedReg3 := makeReg("key3", "val33") + // snap3: { key1: val1, key2: val222, key3: val33 } + snap3 := snap1.Extend(unittest.StateCommitmentFixture(), map[flow.RegisterID]flow.RegisterValue{ + updatedReg3.Key: updatedReg3.Value, + }) + + // verify it's getting from the previous snapshot + value, err = snap3.Get(reg2.Key) + require.NoError(t, err) + require.Equal(t, updatedReg2.Value, value) + + value, err = snap3.Get(reg3.Key) + require.NoError(t, err) + require.Equal(t, updatedReg3.Value, value) + }) } From ad807321dba0ce3354126f76cabaeb6e867d2c61 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 16 Nov 2023 09:22:41 -0800 Subject: [PATCH 5/6] update tests --- .../executing_block_snapshot_test.go | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go index a82857fa945..b3283304cb6 100644 --- a/engine/execution/storehouse/executing_block_snapshot_test.go +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -53,7 +53,15 @@ func TestExtendingBlockSnapshot(t *testing.T) { // should get un-changed value value, err := snap2.Get(reg1.Key) require.NoError(t, err) - require.Equal(t, reg1.Value, value) + require.Equal(t, []byte("val1"), value) + + value, err = snap2.Get(reg2.Key) + require.NoError(t, err) + require.Equal(t, []byte("val22"), value) + + value, err = snap2.Get(reg3.Key) + require.NoError(t, err) + require.Equal(t, []byte("val3"), value) // should get nil for unknown register unknown := makeReg("unknown", "unknownV") @@ -61,26 +69,25 @@ func TestExtendingBlockSnapshot(t *testing.T) { require.NoError(t, err) require.Equal(t, []byte(nil), value) - // should get updated value - value, err = snap2.Get(reg2.Key) - require.NoError(t, err) - require.Equal(t, updatedReg2.Value, value) - + // create snap3 with reg3 updated + // snap3: { key1: val1, key2: val22, key3: val33 } updatedReg3 := makeReg("key3", "val33") - // snap3: { key1: val1, key2: val222, key3: val33 } - snap3 := snap1.Extend(unittest.StateCommitmentFixture(), map[flow.RegisterID]flow.RegisterValue{ + snap3 := snap2.Extend(unittest.StateCommitmentFixture(), map[flow.RegisterID]flow.RegisterValue{ updatedReg3.Key: updatedReg3.Value, }) - // verify it's getting from the previous snapshot + // verify all keys + value, err = snap3.Get(reg1.Key) + require.NoError(t, err) + require.Equal(t, []byte("val1"), value) + value, err = snap3.Get(reg2.Key) require.NoError(t, err) - require.Equal(t, updatedReg2.Value, value) + require.Equal(t, []byte("val22"), value) value, err = snap3.Get(reg3.Key) require.NoError(t, err) - require.Equal(t, updatedReg3.Value, value) - + require.Equal(t, []byte("val33"), value) }) } From 904bed008385733ca68160155350533bf6fe87cf Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Fri, 17 Nov 2023 16:30:48 -0800 Subject: [PATCH 6/6] fix lint --- engine/execution/storehouse/executing_block_snapshot_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/engine/execution/storehouse/executing_block_snapshot_test.go b/engine/execution/storehouse/executing_block_snapshot_test.go index b3283304cb6..616430ec858 100644 --- a/engine/execution/storehouse/executing_block_snapshot_test.go +++ b/engine/execution/storehouse/executing_block_snapshot_test.go @@ -90,7 +90,3 @@ func TestExtendingBlockSnapshot(t *testing.T) { require.Equal(t, []byte("val33"), value) }) } - -func makeReg(key string, value string) flow.RegisterEntry { - return unittest.MakeOwnerReg(key, value) -}