From dbf757bda7b3c1369d28b690e26e5786e04cc6ed Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Fri, 26 Sep 2025 10:16:55 +0200 Subject: [PATCH 1/4] Add Parent hash to Block and provide Writer that verifies correctness --- block.go | 1 + writer_with_indexer_test.go | 59 ----- writer_with_verify_hash.go | 70 +++++ writer_with_verify_hash_test.go | 442 ++++++++++++++++++++++++++++++++ 4 files changed, 513 insertions(+), 59 deletions(-) delete mode 100644 writer_with_indexer_test.go create mode 100644 writer_with_verify_hash.go create mode 100644 writer_with_verify_hash_test.go diff --git a/block.go b/block.go index 807dbdf..ec267c6 100644 --- a/block.go +++ b/block.go @@ -6,6 +6,7 @@ import ( type Block[T any] struct { Hash common.Hash `json:"blockHash"` + Parent common.Hash `json:"parentHash"` Number uint64 `json:"blockNum"` TS uint64 `json:"blockTS"` // unix ts Data T `json:"blockData"` diff --git a/writer_with_indexer_test.go b/writer_with_indexer_test.go deleted file mode 100644 index 3db6a0d..0000000 --- a/writer_with_indexer_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package ethwal - -import ( - "context" - "os" - "path" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestWriterWithIndexer(t *testing.T) { - defer func() { - _ = os.RemoveAll(testPath) - }() - - blocks := generateMixedIntBlocks() - - indexes := generateMixedIntIndexes() - - indexer, err := NewIndexer(context.Background(), IndexerOptions[[]int]{ - Dataset: Dataset{ - Path: testPath, - }, - Indexes: indexes, - }) - require.NoError(t, err) - - w, err := NewWriter[[]int](Options{ - Dataset: Dataset{ - Path: testPath, - }, - NewCompressor: NewZSTDCompressor, - NewEncoder: NewCBOREncoder, - }) - require.NoError(t, err) - - wi, err := NewWriterWithIndexer(w, indexer) - require.NoError(t, err) - - for _, block := range blocks { - err := wi.Write(context.Background(), block) - require.NoError(t, err) - } - - err = wi.RollFile(context.Background()) - require.NoError(t, err) - - err = wi.Close(context.Background()) - require.NoError(t, err) - - indexDirEntries, err := os.ReadDir(path.Join(testPath, ".indexes")) - require.NoError(t, err) - require.Len(t, indexDirEntries, 4) - - ethwalDirEntries, err := os.ReadDir(testPath) - require.NoError(t, err) - require.Len(t, ethwalDirEntries, 3) -} diff --git a/writer_with_verify_hash.go b/writer_with_verify_hash.go new file mode 100644 index 0000000..8c0a403 --- /dev/null +++ b/writer_with_verify_hash.go @@ -0,0 +1,70 @@ +package ethwal + +import ( + "context" + "fmt" + + "github.com/0xsequence/ethkit/go-ethereum/common" +) + +type BlockHashGetter func(ctx context.Context, blockNum uint64) (common.Hash, error) + +func BlockGetterFromReader[T any](options Options) BlockHashGetter { + return func(ctx context.Context, blockNum uint64) (common.Hash, error) { + reader, err := NewReader[T](options) + if err != nil { + return common.Hash{}, fmt.Errorf("failed to create reader: %w", err) + } + defer reader.Close() + + err = reader.Seek(ctx, blockNum-1) + if err != nil { + return common.Hash{}, fmt.Errorf("failed to seek to block %d: %w", blockNum, err) + } + + block, err := reader.Read(ctx) + if err != nil { + return common.Hash{}, fmt.Errorf("failed to read block %d: %w", blockNum, err) + } + return block.Hash, nil + } +} + +type writerWithVerifyHash[T any] struct { + Writer[T] + + blockHashGetter BlockHashGetter + + prevHash common.Hash +} + +var _ Writer[any] = (*writerWithVerifyHash[any])(nil) + +func NewWriterWithVerifyHash[T any](writer Writer[T], blockHashGetter BlockHashGetter) Writer[T] { + return &writerWithVerifyHash[T]{Writer: writer, blockHashGetter: blockHashGetter} +} + +func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { + var err error + if w.prevHash == (common.Hash{}) && b.Number > 1 { + w.prevHash, err = w.blockHashGetter(ctx, b.Number-1) + if err != nil { + return err + } + } + + if b.Parent != w.prevHash { + w.prevHash = common.Hash{} + return fmt.Errorf("parent hash mismatch, expected %s, got %s", + w.prevHash.String(), b.Parent.String()) + } + + err = w.Writer.Write(ctx, b) + if err != nil { + w.prevHash = common.Hash{} + return err + } + + w.prevHash = b.Hash + return nil +} diff --git a/writer_with_verify_hash_test.go b/writer_with_verify_hash_test.go new file mode 100644 index 0000000..1985b96 --- /dev/null +++ b/writer_with_verify_hash_test.go @@ -0,0 +1,442 @@ +package ethwal + +import ( + "context" + "errors" + "testing" + + "github.com/0xsequence/ethkit/go-ethereum/common" + "github.com/0xsequence/ethwal/storage" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Mock writer for testing +type mockWriter[T any] struct { + mock.Mock +} + +func (m *mockWriter[T]) FileSystem() storage.FS { + args := m.Called() + return args.Get(0).(storage.FS) +} + +func (m *mockWriter[T]) Write(ctx context.Context, b Block[T]) error { + args := m.Called(ctx, b) + return args.Error(0) +} + +func (m *mockWriter[T]) BlockNum() uint64 { + args := m.Called() + return args.Get(0).(uint64) +} + +func (m *mockWriter[T]) RollFile(ctx context.Context) error { + args := m.Called(ctx) + return args.Error(0) +} + +func (m *mockWriter[T]) Close(ctx context.Context) error { + args := m.Called(ctx) + return args.Error(0) +} + +func (m *mockWriter[T]) Options() Options { + args := m.Called() + return args.Get(0).(Options) +} + +func (m *mockWriter[T]) SetOptions(opt Options) { + m.Called(opt) +} + +// Mock block hash getter for testing +type mockBlockHashGetter struct { + mock.Mock +} + +func (m *mockBlockHashGetter) GetHash(ctx context.Context, blockNum uint64) (common.Hash, error) { + args := m.Called(ctx, blockNum) + return args.Get(0).(common.Hash), args.Error(1) +} + +func TestNewWriterWithVerifyHash(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + require.NotNil(t, writer) + + // Check that it returns a writerWithVerifyHash instance + writerImpl, ok := writer.(*writerWithVerifyHash[int]) + require.True(t, ok) + require.Equal(t, mockWriter, writerImpl.Writer) + require.NotNil(t, writerImpl.blockHashGetter) + require.Equal(t, common.Hash{}, writerImpl.prevHash) +} + +func TestWriterWithVerifyHash_Write_FirstBlock(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x01}), + Parent: common.Hash{}, // First block has empty parent + Number: 1, + Data: 42, + } + + // Mock expectations + mockWriter.On("Write", ctx, block).Return(nil) + + err := writer.Write(ctx, block) + + require.NoError(t, err) + mockWriter.AssertExpectations(t) + mockGetter.AssertNotCalled(t, "GetHash") // Should not call getter for block 1 + + // Check that prevHash is updated + writerImpl := writer.(*writerWithVerifyHash[int]) + assert.Equal(t, block.Hash, writerImpl.prevHash) +} + +func TestWriterWithVerifyHash_Write_SuccessfulSequence(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + // First block + block1 := Block[int]{ + Hash: common.BytesToHash([]byte{0x01}), + Parent: common.Hash{}, + Number: 1, + Data: 42, + } + + // Second block with correct parent + block2 := Block[int]{ + Hash: common.BytesToHash([]byte{0x02}), + Parent: block1.Hash, + Number: 2, + Data: 43, + } + + // Third block with correct parent + block3 := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: block2.Hash, + Number: 3, + Data: 44, + } + + // Mock expectations + mockWriter.On("Write", ctx, block1).Return(nil) + mockWriter.On("Write", ctx, block2).Return(nil) + mockWriter.On("Write", ctx, block3).Return(nil) + + // Write blocks in sequence + err := writer.Write(ctx, block1) + require.NoError(t, err) + + err = writer.Write(ctx, block2) + require.NoError(t, err) + + err = writer.Write(ctx, block3) + require.NoError(t, err) + + mockWriter.AssertExpectations(t) + mockGetter.AssertNotCalled(t, "GetHash") // Should not call getter when writing sequentially + + // Check final prevHash state + writerImpl := writer.(*writerWithVerifyHash[int]) + assert.Equal(t, block3.Hash, writerImpl.prevHash) +} + +func TestWriterWithVerifyHash_Write_WithBlockHashGetter(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + // Starting from block 3 (should fetch previous hash) + prevHash := common.BytesToHash([]byte{0x02}) + block3 := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: prevHash, + Number: 3, + Data: 44, + } + + // Mock expectations + mockGetter.On("GetHash", ctx, uint64(2)).Return(prevHash, nil) + mockWriter.On("Write", ctx, block3).Return(nil) + + err := writer.Write(ctx, block3) + + require.NoError(t, err) + mockWriter.AssertExpectations(t) + mockGetter.AssertExpectations(t) + + // Check that prevHash is updated + writerImpl := writer.(*writerWithVerifyHash[int]) + assert.Equal(t, block3.Hash, writerImpl.prevHash) +} + +func TestWriterWithVerifyHash_Write_ParentHashMismatch(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + // Set up a previous hash first + writerImpl := writer.(*writerWithVerifyHash[int]) + writerImpl.prevHash = common.BytesToHash([]byte{0x01}) + + // Block with wrong parent hash + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: common.BytesToHash([]byte{0x99}), // Wrong parent + Number: 2, + Data: 44, + } + + err := writer.Write(ctx, block) + + require.Error(t, err) + assert.Contains(t, err.Error(), "parent hash mismatch") + + // Check that prevHash is reset to empty on error + assert.Equal(t, common.Hash{}, writerImpl.prevHash) + + // Mock should not be called since validation fails first + mockWriter.AssertNotCalled(t, "Write") + mockGetter.AssertNotCalled(t, "GetHash") +} + +func TestWriterWithVerifyHash_Write_BlockHashGetterError(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: common.BytesToHash([]byte{0x02}), + Number: 3, + Data: 44, + } + + getterError := errors.New("failed to get block hash") + + // Mock expectations + mockGetter.On("GetHash", ctx, uint64(2)).Return(common.Hash{}, getterError) + + err := writer.Write(ctx, block) + + require.Error(t, err) + assert.Equal(t, getterError, err) + + mockGetter.AssertExpectations(t) + mockWriter.AssertNotCalled(t, "Write") // Should not call write if getter fails +} + +func TestWriterWithVerifyHash_Write_UnderlyingWriterError(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x01}), + Parent: common.Hash{}, + Number: 1, + Data: 42, + } + + writeError := errors.New("write failed") + + // Mock expectations + mockWriter.On("Write", ctx, block).Return(writeError) + + err := writer.Write(ctx, block) + + require.Error(t, err) + assert.Equal(t, writeError, err) + + // Check that prevHash is reset to empty on write error + writerImpl := writer.(*writerWithVerifyHash[int]) + assert.Equal(t, common.Hash{}, writerImpl.prevHash) + + mockWriter.AssertExpectations(t) +} + +func TestWriterWithVerifyHash_Write_ParentHashMismatchAfterGetter(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + // Block that requires fetching previous hash + actualPrevHash := common.BytesToHash([]byte{0x02}) + wrongParentHash := common.BytesToHash([]byte{0x99}) + + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: wrongParentHash, // Wrong parent hash + Number: 3, + Data: 44, + } + + // Mock expectations - getter returns correct hash, but block has wrong parent + mockGetter.On("GetHash", ctx, uint64(2)).Return(actualPrevHash, nil) + + err := writer.Write(ctx, block) + + require.Error(t, err) + assert.Contains(t, err.Error(), "parent hash mismatch") + + // Check that prevHash is reset to empty on error + writerImpl := writer.(*writerWithVerifyHash[int]) + assert.Equal(t, common.Hash{}, writerImpl.prevHash) + + mockGetter.AssertExpectations(t) + mockWriter.AssertNotCalled(t, "Write") // Should not call write if validation fails +} + +func TestWriterWithVerifyHash_InterfaceCompliance(t *testing.T) { + // Test that all interface methods are properly delegated + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + // Test FileSystem delegation + expectedFS := &struct{ storage.FS }{} + mockWriter.On("FileSystem").Return(expectedFS) + fs := writer.FileSystem() + assert.Equal(t, expectedFS, fs) + + // Test BlockNum delegation + mockWriter.On("BlockNum").Return(uint64(42)) + blockNum := writer.BlockNum() + assert.Equal(t, uint64(42), blockNum) + + // Test RollFile delegation + mockWriter.On("RollFile", ctx).Return(nil) + err := writer.RollFile(ctx) + assert.NoError(t, err) + + // Test Close delegation + mockWriter.On("Close", ctx).Return(nil) + err = writer.Close(ctx) + assert.NoError(t, err) + + // Test Options delegation + expectedOptions := Options{Dataset: Dataset{Name: "test"}} + mockWriter.On("Options").Return(expectedOptions) + options := writer.Options() + assert.Equal(t, expectedOptions, options) + + // Test SetOptions delegation + newOptions := Options{Dataset: Dataset{Name: "new-test"}} + mockWriter.On("SetOptions", newOptions).Return() + writer.SetOptions(newOptions) + + mockWriter.AssertExpectations(t) +} + +func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { + // Test that the writer can recover after an error by resetting prevHash + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + writerImpl := writer.(*writerWithVerifyHash[int]) + + // First, write a successful block + block1 := Block[int]{ + Hash: common.BytesToHash([]byte{0x01}), + Parent: common.Hash{}, + Number: 1, + Data: 42, + } + + mockWriter.On("Write", ctx, block1).Return(nil) + err := writer.Write(ctx, block1) + require.NoError(t, err) + assert.Equal(t, block1.Hash, writerImpl.prevHash) + + // Then try to write a block with wrong parent hash (should error and reset) + block2Wrong := Block[int]{ + Hash: common.BytesToHash([]byte{0x02}), + Parent: common.BytesToHash([]byte{0x99}), // Wrong parent + Number: 2, + Data: 43, + } + + err = writer.Write(ctx, block2Wrong) + require.Error(t, err) + assert.Equal(t, common.Hash{}, writerImpl.prevHash) // Should be reset + + // Now write a new sequence starting from block 3 (requiring hash lookup) + block2Hash := common.BytesToHash([]byte{0x02}) + block3 := Block[int]{ + Hash: common.BytesToHash([]byte{0x03}), + Parent: block2Hash, + Number: 3, + Data: 44, + } + + mockGetter.On("GetHash", ctx, uint64(2)).Return(block2Hash, nil) + mockWriter.On("Write", ctx, block3).Return(nil) + + err = writer.Write(ctx, block3) + require.NoError(t, err) + assert.Equal(t, block3.Hash, writerImpl.prevHash) + + mockWriter.AssertExpectations(t) + mockGetter.AssertExpectations(t) +} + +func TestBlockGetterFromReader(t *testing.T) { + // This test validates that the BlockGetterFromReader helper function can be created + // The detailed functionality testing is covered in the integration tests above + options := Options{ + Dataset: Dataset{ + Name: "test-wal", + Path: testPath, + Version: defaultDatasetVersion, + }, + NewEncoder: NewJSONEncoder, + NewDecoder: NewJSONDecoder, + } + + getter := BlockGetterFromReader[int](options) + require.NotNil(t, getter) + + // Test that calling it with invalid block returns error + _, err := getter(context.Background(), 99) + require.Error(t, err) +} From 8f582271e1215292242f9b8237476576d1aaeb1f Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Fri, 26 Sep 2025 10:23:02 +0200 Subject: [PATCH 2/4] rename --- writer_with_verify_hash.go | 2 +- writer_with_verify_hash_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/writer_with_verify_hash.go b/writer_with_verify_hash.go index 8c0a403..a7bd0f0 100644 --- a/writer_with_verify_hash.go +++ b/writer_with_verify_hash.go @@ -9,7 +9,7 @@ import ( type BlockHashGetter func(ctx context.Context, blockNum uint64) (common.Hash, error) -func BlockGetterFromReader[T any](options Options) BlockHashGetter { +func BlockHashGetterFromReader[T any](options Options) BlockHashGetter { return func(ctx context.Context, blockNum uint64) (common.Hash, error) { reader, err := NewReader[T](options) if err != nil { diff --git a/writer_with_verify_hash_test.go b/writer_with_verify_hash_test.go index 1985b96..8b282f6 100644 --- a/writer_with_verify_hash_test.go +++ b/writer_with_verify_hash_test.go @@ -433,7 +433,7 @@ func TestBlockGetterFromReader(t *testing.T) { NewDecoder: NewJSONDecoder, } - getter := BlockGetterFromReader[int](options) + getter := BlockHashGetterFromReader[int](options) require.NotNil(t, getter) // Test that calling it with invalid block returns error From 49bfa485792baa09f8659a4b3c10a64cdf99e82e Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Fri, 26 Sep 2025 10:24:19 +0200 Subject: [PATCH 3/4] restore tests --- writer_with_indexer_test.go | 59 +++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 writer_with_indexer_test.go diff --git a/writer_with_indexer_test.go b/writer_with_indexer_test.go new file mode 100644 index 0000000..3db6a0d --- /dev/null +++ b/writer_with_indexer_test.go @@ -0,0 +1,59 @@ +package ethwal + +import ( + "context" + "os" + "path" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestWriterWithIndexer(t *testing.T) { + defer func() { + _ = os.RemoveAll(testPath) + }() + + blocks := generateMixedIntBlocks() + + indexes := generateMixedIntIndexes() + + indexer, err := NewIndexer(context.Background(), IndexerOptions[[]int]{ + Dataset: Dataset{ + Path: testPath, + }, + Indexes: indexes, + }) + require.NoError(t, err) + + w, err := NewWriter[[]int](Options{ + Dataset: Dataset{ + Path: testPath, + }, + NewCompressor: NewZSTDCompressor, + NewEncoder: NewCBOREncoder, + }) + require.NoError(t, err) + + wi, err := NewWriterWithIndexer(w, indexer) + require.NoError(t, err) + + for _, block := range blocks { + err := wi.Write(context.Background(), block) + require.NoError(t, err) + } + + err = wi.RollFile(context.Background()) + require.NoError(t, err) + + err = wi.Close(context.Background()) + require.NoError(t, err) + + indexDirEntries, err := os.ReadDir(path.Join(testPath, ".indexes")) + require.NoError(t, err) + require.Len(t, indexDirEntries, 4) + + ethwalDirEntries, err := os.ReadDir(testPath) + require.NoError(t, err) + require.Len(t, ethwalDirEntries, 3) +} From 7bd463c0dc916abf3e44a4892e0e581cefd5677c Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Fri, 26 Sep 2025 10:27:53 +0200 Subject: [PATCH 4/4] wrap errors --- writer_with_verify_hash.go | 4 ++-- writer_with_verify_hash_test.go | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/writer_with_verify_hash.go b/writer_with_verify_hash.go index a7bd0f0..f865b74 100644 --- a/writer_with_verify_hash.go +++ b/writer_with_verify_hash.go @@ -49,7 +49,7 @@ func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { if w.prevHash == (common.Hash{}) && b.Number > 1 { w.prevHash, err = w.blockHashGetter(ctx, b.Number-1) if err != nil { - return err + return fmt.Errorf("failed to get block hash: %w", err) } } @@ -62,7 +62,7 @@ func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { err = w.Writer.Write(ctx, b) if err != nil { w.prevHash = common.Hash{} - return err + return fmt.Errorf("failed to write block: %w", err) } w.prevHash = b.Hash diff --git a/writer_with_verify_hash_test.go b/writer_with_verify_hash_test.go index 8b282f6..25c7bad 100644 --- a/writer_with_verify_hash_test.go +++ b/writer_with_verify_hash_test.go @@ -248,7 +248,8 @@ func TestWriterWithVerifyHash_Write_BlockHashGetterError(t *testing.T) { err := writer.Write(ctx, block) require.Error(t, err) - assert.Equal(t, getterError, err) + assert.Contains(t, err.Error(), "failed to get block hash") + assert.ErrorIs(t, err, getterError) mockGetter.AssertExpectations(t) mockWriter.AssertNotCalled(t, "Write") // Should not call write if getter fails @@ -277,7 +278,8 @@ func TestWriterWithVerifyHash_Write_UnderlyingWriterError(t *testing.T) { err := writer.Write(ctx, block) require.Error(t, err) - assert.Equal(t, writeError, err) + assert.Contains(t, err.Error(), "failed to write block") + assert.ErrorIs(t, err, writeError) // Check that prevHash is reset to empty on write error writerImpl := writer.(*writerWithVerifyHash[int])