From 6834c074542f73a2c6ff0ee6d0e2b4062ff9fa3e Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Mon, 18 Nov 2024 20:05:40 +0100 Subject: [PATCH 1/5] Copy slice when setting block hash list --- fvm/evm/handler/blockHashList.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fvm/evm/handler/blockHashList.go b/fvm/evm/handler/blockHashList.go index 86ea5738b5a..bf1d564c2a2 100644 --- a/fvm/evm/handler/blockHashList.go +++ b/fvm/evm/handler/blockHashList.go @@ -84,6 +84,7 @@ func (bhl *BlockHashList) Push(height uint64, bh gethCommon.Hash) error { if !bhl.IsEmpty() && height != bhl.height+1 { return fmt.Errorf("out of the order block hash, expected: %d, got: %d", bhl.height+1, height) } + // updates the block hash stored at index err := bhl.updateBlockHashAt(bhl.tail, bh) if err != nil { @@ -147,19 +148,22 @@ func (bhl *BlockHashList) updateBlockHashAt(idx int, bh gethCommon.Hash) error { // fetch the bucket bucketNumber := idx / hashCountPerBucket bucket, err := bhl.fetchBucket(bucketNumber) + cpy := make([]byte, len(bucket)) + copy(cpy, bucket) + if err != nil { return err } // update the block hash start := (idx % hashCountPerBucket) * hashEncodingSize end := start + hashEncodingSize - copy(bucket[start:end], bh.Bytes()) + copy(cpy[start:end], bh.Bytes()) // store bucket return bhl.backend.SetValue( bhl.rootAddress[:], []byte(fmt.Sprintf(blockHashListBucketKeyFormat, bucketNumber)), - bucket, + cpy, ) } From 291d0cafaabcb5a6517f6d0dda65b1fd997cea2e Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 19 Nov 2024 16:15:08 +0100 Subject: [PATCH 2/5] add test --- fvm/evm/handler/blockHashList.go | 7 ++- fvm/fvm_test.go | 101 +++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/fvm/evm/handler/blockHashList.go b/fvm/evm/handler/blockHashList.go index bf1d564c2a2..91eefded24e 100644 --- a/fvm/evm/handler/blockHashList.go +++ b/fvm/evm/handler/blockHashList.go @@ -148,12 +148,13 @@ func (bhl *BlockHashList) updateBlockHashAt(idx int, bh gethCommon.Hash) error { // fetch the bucket bucketNumber := idx / hashCountPerBucket bucket, err := bhl.fetchBucket(bucketNumber) - cpy := make([]byte, len(bucket)) - copy(cpy, bucket) - if err != nil { return err } + + cpy := make([]byte, len(bucket)) + copy(cpy, bucket) + // update the block hash start := (idx % hashCountPerBucket) * hashEncodingSize end := start + hashEncodingSize diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index b186d0e1c14..96176f8f2d2 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3,6 +3,7 @@ package fvm_test import ( "context" "crypto/rand" + "encoding/binary" "encoding/hex" "fmt" "math" @@ -38,6 +39,7 @@ import ( envMock "github.com/onflow/flow-go/fvm/environment/mock" "github.com/onflow/flow-go/fvm/errors" "github.com/onflow/flow-go/fvm/evm/events" + "github.com/onflow/flow-go/fvm/evm/handler" "github.com/onflow/flow-go/fvm/evm/stdlib" "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/flow-go/fvm/meter" @@ -3587,3 +3589,102 @@ func Test_MinimumRequiredVersion(t *testing.T) { require.Equal(t, cadenceVersion1.String(), getVersion(ctx, snapshotTree)) })) } + +func Test_BlockHashListShouldWriteOnPush(t *testing.T) { + + chain := flow.Emulator.Chain() + sc := systemcontracts.SystemContractsForChain(chain.ChainID()) + + push := func(bhl *handler.BlockHashList, height uint64) { + buffer := make([]byte, 32) + pos := 0 + + // encode capacity + binary.BigEndian.PutUint64(buffer[pos:], height) + err := bhl.Push(height, [32]byte(buffer)) + require.NoError(t, err) + } + + t.Run("block hash list write on push", newVMTest(). + withContextOptions( + fvm.WithChain(chain), + fvm.WithEVMEnabled(true), + ). + run(func( + t *testing.T, + vm fvm.VM, + chain flow.Chain, + ctx fvm.Context, + snapshotTree snapshot.SnapshotTree, + ) { + capacity := 256 + + ts := state.NewTransactionState(snapshotTree, state.DefaultParameters()) + accounts := environment.NewAccounts(ts) + envMeter := environment.NewMeter(ts) + + valueStore := environment.NewValueStore( + tracing.NewMockTracerSpan(), + envMeter, + accounts, + ) + + bhl, err := handler.NewBlockHashList(valueStore, sc.EVMStorage.Address, capacity) + require.NoError(t, err) + + // fill the block hash list + height := uint64(0) + for ; height < uint64(capacity); height++ { + push(bhl, height) + } + + es, err := ts.FinalizeMainTransaction() + require.NoError(t, err) + snapshotTree = snapshotTree.Append(es) + + // end of test setup + + ts = state.NewTransactionState(snapshotTree, state.DefaultParameters()) + accounts = environment.NewAccounts(ts) + envMeter = environment.NewMeter(ts) + + valueStore = environment.NewValueStore( + tracing.NewMockTracerSpan(), + envMeter, + accounts, + ) + + bhl, err = handler.NewBlockHashList(valueStore, sc.EVMStorage.Address, capacity) + require.NoError(t, err) + + // after we push the changes should be applied and the first block hash in the bucket should be capacity+1 instead of 0 + push(bhl, height) + + es, err = ts.FinalizeMainTransaction() + require.NoError(t, err) + + require.Len(t, es.WriteSet, 2) + newBlockHashListBucket, ok := es.WriteSet[flow.NewRegisterID(sc.EVMStorage.Address, "BlockHashListBucket0")] + require.True(t, ok) + // full expected block hash list bucket split by individual block hashes + // first block hash is the capacity+1 instead of 0 (00 00 00 00 00 00 01 00) + expectedBlockHashListBucket, err := hex.DecodeString( + "0000000000000100000000000000000000000000000000000000000000000000" + + "0000000000000001000000000000000000000000000000000000000000000000" + + "0000000000000002000000000000000000000000000000000000000000000000" + + "0000000000000003000000000000000000000000000000000000000000000000" + + "0000000000000004000000000000000000000000000000000000000000000000" + + "0000000000000005000000000000000000000000000000000000000000000000" + + "0000000000000006000000000000000000000000000000000000000000000000" + + "0000000000000007000000000000000000000000000000000000000000000000" + + "0000000000000008000000000000000000000000000000000000000000000000" + + "0000000000000009000000000000000000000000000000000000000000000000" + + "000000000000000a000000000000000000000000000000000000000000000000" + + "000000000000000b000000000000000000000000000000000000000000000000" + + "000000000000000c000000000000000000000000000000000000000000000000" + + "000000000000000d000000000000000000000000000000000000000000000000" + + "000000000000000e000000000000000000000000000000000000000000000000" + + "000000000000000f000000000000000000000000000000000000000000000000") + require.Equal(t, expectedBlockHashListBucket, newBlockHashListBucket) + })) +} From 99d46cf9102a09c15cf1ea7eb7332daf5cce5529 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 19 Nov 2024 16:21:45 +0100 Subject: [PATCH 3/5] more comments --- fvm/fvm_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 96176f8f2d2..22c31ca3324 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3619,6 +3619,8 @@ func Test_BlockHashListShouldWriteOnPush(t *testing.T) { ) { capacity := 256 + // for the setup we make sure all the block hash list buckets exist + ts := state.NewTransactionState(snapshotTree, state.DefaultParameters()) accounts := environment.NewAccounts(ts) envMeter := environment.NewMeter(ts) @@ -3663,6 +3665,7 @@ func Test_BlockHashListShouldWriteOnPush(t *testing.T) { es, err = ts.FinalizeMainTransaction() require.NoError(t, err) + // the write set should have both block metadata and block hash list bucket require.Len(t, es.WriteSet, 2) newBlockHashListBucket, ok := es.WriteSet[flow.NewRegisterID(sc.EVMStorage.Address, "BlockHashListBucket0")] require.True(t, ok) From 349350bf59e6cc60bafc7e6d96eab4820ca51a0b Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 19 Nov 2024 16:44:23 +0100 Subject: [PATCH 4/5] fix lint --- fvm/fvm_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 22c31ca3324..10c7fc5ae29 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3688,6 +3688,7 @@ func Test_BlockHashListShouldWriteOnPush(t *testing.T) { "000000000000000d000000000000000000000000000000000000000000000000" + "000000000000000e000000000000000000000000000000000000000000000000" + "000000000000000f000000000000000000000000000000000000000000000000") + require.NoError(t, err) require.Equal(t, expectedBlockHashListBucket, newBlockHashListBucket) })) } From 755ec67b068e3f8efc34edc21de91a4d30b2e995 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 19 Nov 2024 18:58:25 +0100 Subject: [PATCH 5/5] fix comment --- fvm/fvm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 10c7fc5ae29..eedc9e50c8b 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3599,7 +3599,7 @@ func Test_BlockHashListShouldWriteOnPush(t *testing.T) { buffer := make([]byte, 32) pos := 0 - // encode capacity + // encode height as block hash binary.BigEndian.PutUint64(buffer[pos:], height) err := bhl.Push(height, [32]byte(buffer)) require.NoError(t, err)