From 040159c9aae829efbb686167222323e4d9df04c0 Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Tue, 21 Oct 2025 11:37:53 +0200 Subject: [PATCH 1/2] fix: writer_with_verify_hash tries to get hash from block that hasn't been written --- writer_with_verify_hash.go | 7 ++-- writer_with_verify_hash_test.go | 61 ++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/writer_with_verify_hash.go b/writer_with_verify_hash.go index 76a9bcc..1fafcfe 100644 --- a/writer_with_verify_hash.go +++ b/writer_with_verify_hash.go @@ -45,6 +45,11 @@ func NewWriterWithVerifyHash[T any](writer Writer[T], blockHashGetter BlockHashG } func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { + // Skip if block is already written + if b.Number <= w.Writer.BlockNum() { + return nil + } + // Skip validation if block is first block if b.Number == 1 { if err := w.Writer.Write(ctx, b); err != nil { @@ -67,7 +72,6 @@ func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { // Validate parent hash 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()) } @@ -75,7 +79,6 @@ func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { // Write block err := w.Writer.Write(ctx, b) if err != nil { - w.prevHash = common.Hash{} return fmt.Errorf("failed to write block: %w", err) } diff --git a/writer_with_verify_hash_test.go b/writer_with_verify_hash_test.go index 4aae1e2..e839634 100644 --- a/writer_with_verify_hash_test.go +++ b/writer_with_verify_hash_test.go @@ -92,6 +92,7 @@ func TestWriterWithVerifyHash_Write_FirstBlock(t *testing.T) { } // Mock expectations + mockWriter.On("BlockNum").Return(uint64(0)) // No blocks written yet mockWriter.On("Write", ctx, block).Return(nil) err := writer.Write(ctx, block) @@ -138,8 +139,11 @@ func TestWriterWithVerifyHash_Write_SuccessfulSequence(t *testing.T) { } // Mock expectations + mockWriter.On("BlockNum").Return(uint64(0)).Once() // Before block 1 mockWriter.On("Write", ctx, block1).Return(nil) + mockWriter.On("BlockNum").Return(uint64(1)).Once() // Before block 2 mockWriter.On("Write", ctx, block2).Return(nil) + mockWriter.On("BlockNum").Return(uint64(2)).Once() // Before block 3 mockWriter.On("Write", ctx, block3).Return(nil) // Write blocks in sequence @@ -178,6 +182,7 @@ func TestWriterWithVerifyHash_Write_WithBlockHashGetter(t *testing.T) { } // Mock expectations + mockWriter.On("BlockNum").Return(uint64(2)) // Block 2 is the last written block mockGetter.On("GetHash", ctx, uint64(2)).Return(prevHash, nil) mockWriter.On("Write", ctx, block3).Return(nil) @@ -212,13 +217,16 @@ func TestWriterWithVerifyHash_Write_ParentHashMismatch(t *testing.T) { Data: 44, } + // Mock expectations + mockWriter.On("BlockNum").Return(uint64(1)) // Block 1 is the last written block + 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) + // prevHash should remain unchanged after error (not reset) + assert.Equal(t, common.BytesToHash([]byte{0x01}), writerImpl.prevHash) // Mock should not be called since validation fails first mockWriter.AssertNotCalled(t, "Write") @@ -243,6 +251,7 @@ func TestWriterWithVerifyHash_Write_BlockHashGetterError(t *testing.T) { getterError := errors.New("failed to get block hash") // Mock expectations + mockWriter.On("BlockNum").Return(uint64(2)) // Block 2 is the last written block mockGetter.On("GetHash", ctx, uint64(2)).Return(common.Hash{}, getterError) err := writer.Write(ctx, block) @@ -273,6 +282,7 @@ func TestWriterWithVerifyHash_Write_UnderlyingWriterError(t *testing.T) { writeError := errors.New("write failed") // Mock expectations + mockWriter.On("BlockNum").Return(uint64(0)) // No blocks written yet mockWriter.On("Write", ctx, block).Return(writeError) err := writer.Write(ctx, block) @@ -281,7 +291,7 @@ func TestWriterWithVerifyHash_Write_UnderlyingWriterError(t *testing.T) { assert.Contains(t, err.Error(), "failed to write block") assert.ErrorIs(t, err, writeError) - // Check that prevHash is reset to empty on write error + // prevHash should remain unchanged after error (not reset) writerImpl := writer.(*writerWithVerifyHash[int]) assert.Equal(t, common.Hash{}, writerImpl.prevHash) @@ -308,6 +318,7 @@ func TestWriterWithVerifyHash_Write_ParentHashMismatchAfterGetter(t *testing.T) } // Mock expectations - getter returns correct hash, but block has wrong parent + mockWriter.On("BlockNum").Return(uint64(2)) // Block 2 is the last written block mockGetter.On("GetHash", ctx, uint64(2)).Return(actualPrevHash, nil) err := writer.Write(ctx, block) @@ -315,9 +326,9 @@ func TestWriterWithVerifyHash_Write_ParentHashMismatchAfterGetter(t *testing.T) require.Error(t, err) assert.Contains(t, err.Error(), "parent hash mismatch") - // Check that prevHash is reset to empty on error + // prevHash should be set to the fetched value (actualPrevHash), not reset writerImpl := writer.(*writerWithVerifyHash[int]) - assert.Equal(t, common.Hash{}, writerImpl.prevHash) + assert.Equal(t, actualPrevHash, writerImpl.prevHash) mockGetter.AssertExpectations(t) mockWriter.AssertNotCalled(t, "Write") // Should not call write if validation fails @@ -368,7 +379,7 @@ func TestWriterWithVerifyHash_InterfaceCompliance(t *testing.T) { } func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { - // Test that the writer can recover after an error by resetting prevHash + // Test that the writer can recover after an error by manually resetting prevHash mockWriter := &mockWriter[int]{} mockGetter := &mockBlockHashGetter{} @@ -385,12 +396,13 @@ func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { Data: 42, } + mockWriter.On("BlockNum").Return(uint64(0)).Once() 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) + // Then try to write a block with wrong parent hash (should error but NOT reset prevHash) block2Wrong := Block[int]{ Hash: common.BytesToHash([]byte{0x02}), Parent: common.BytesToHash([]byte{0x99}), // Wrong parent @@ -398,9 +410,13 @@ func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { Data: 43, } + mockWriter.On("BlockNum").Return(uint64(1)).Once() err = writer.Write(ctx, block2Wrong) require.Error(t, err) - assert.Equal(t, common.Hash{}, writerImpl.prevHash) // Should be reset + assert.Equal(t, block1.Hash, writerImpl.prevHash) // Should NOT be reset, remains as block1.Hash + + // Manually reset prevHash to simulate recovery (e.g., after recreating the writer) + writerImpl.prevHash = common.Hash{} // Now write a new sequence starting from block 3 (requiring hash lookup) block2Hash := common.BytesToHash([]byte{0x02}) @@ -411,6 +427,7 @@ func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { Data: 44, } + mockWriter.On("BlockNum").Return(uint64(2)).Once() mockGetter.On("GetHash", ctx, uint64(2)).Return(block2Hash, nil) mockWriter.On("Write", ctx, block3).Return(nil) @@ -422,6 +439,34 @@ func TestWriterWithVerifyHash_Write_ResetAfterError(t *testing.T) { mockGetter.AssertExpectations(t) } +func TestWriterWithVerifyHash_Write_SkipAlreadyWritten(t *testing.T) { + mockWriter := &mockWriter[int]{} + mockGetter := &mockBlockHashGetter{} + + writer := NewWriterWithVerifyHash[int](mockWriter, mockGetter.GetHash) + + ctx := context.Background() + + block := Block[int]{ + Hash: common.BytesToHash([]byte{0x05}), + Parent: common.BytesToHash([]byte{0x04}), + Number: 5, + Data: 42, + } + + // Mock: BlockNum returns 10, meaning blocks up to 10 are already written + mockWriter.On("BlockNum").Return(uint64(10)) + + // Try to write block 5 - should be skipped since BlockNum() returns 10 + err := writer.Write(ctx, block) + + require.NoError(t, err) + mockWriter.AssertExpectations(t) + // Write should not be called since block is already written + mockWriter.AssertNotCalled(t, "Write") + mockGetter.AssertNotCalled(t, "GetHash") +} + 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 From 478e2ce498977cea4c9eefe2ed423c8fd00a1216 Mon Sep 17 00:00:00 2001 From: Marcin Gorzynski Date: Tue, 21 Oct 2025 14:58:22 +0200 Subject: [PATCH 2/2] return specific error on hash mismatch --- writer_with_verify_hash.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/writer_with_verify_hash.go b/writer_with_verify_hash.go index 1fafcfe..c54b113 100644 --- a/writer_with_verify_hash.go +++ b/writer_with_verify_hash.go @@ -2,11 +2,14 @@ package ethwal import ( "context" + "errors" "fmt" "github.com/0xsequence/ethkit/go-ethereum/common" ) +var ErrParentHashMismatch = errors.New("parent hash mismatch") + type BlockHashGetter func(ctx context.Context, blockNum uint64) (common.Hash, error) func BlockHashGetterFromReader[T any](options Options) BlockHashGetter { @@ -72,8 +75,7 @@ func (w *writerWithVerifyHash[T]) Write(ctx context.Context, b Block[T]) error { // Validate parent hash if b.Parent != w.prevHash { - return fmt.Errorf("parent hash mismatch, expected %s, got %s", - w.prevHash.String(), b.Parent.String()) + return fmt.Errorf("%w, expected %s, got %s", ErrParentHashMismatch, b.Parent.String(), w.prevHash.String()) } // Write block