From 6eb67640988cbc27bec75cb6b7caaf49af8717de Mon Sep 17 00:00:00 2001 From: Draco Date: Sun, 24 Aug 2025 16:15:14 -0400 Subject: [PATCH 01/18] feat: added an in-memory mock implementation of blockdb for testing --- x/blockdb/memory_db.go | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 x/blockdb/memory_db.go diff --git a/x/blockdb/memory_db.go b/x/blockdb/memory_db.go new file mode 100644 index 000000000000..8d957a84f57d --- /dev/null +++ b/x/blockdb/memory_db.go @@ -0,0 +1,87 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package blockdb + +import "sync" + +// MemoryDatabase is an in-memory implementation of BlockDB +type MemoryDatabase struct { + mu sync.RWMutex + blocks map[BlockHeight]BlockData + closed bool +} + +// NewMemoryDatabase creates a new in-memory BlockDB +func NewMemoryDatabase() *MemoryDatabase { + return &MemoryDatabase{ + blocks: make(map[BlockHeight]BlockData), + } +} + +// WriteBlock stores a block in memory +func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { + m.mu.Lock() + defer m.mu.Unlock() + + if m.closed { + return ErrDatabaseClosed + } + + if len(block) == 0 { + return ErrBlockEmpty + } + + m.blocks[height] = block + + return nil +} + +// ReadBlock retrieves the full block data for the given height +func (m *MemoryDatabase) ReadBlock(height BlockHeight) (BlockData, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + if m.closed { + return nil, ErrDatabaseClosed + } + + block, exists := m.blocks[height] + if !exists { + return nil, ErrBlockNotFound + } + + return block, nil +} + +// HasBlock checks if a block exists at the given height +func (m *MemoryDatabase) HasBlock(height BlockHeight) (bool, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + if m.closed { + return false, ErrDatabaseClosed + } + + _, exists := m.blocks[height] + return exists, nil +} + +// Inspect returns details about the database +func (*MemoryDatabase) Inspect() (string, error) { + return "", nil +} + +// Close closes the in-memory database +func (m *MemoryDatabase) Close() error { + m.mu.Lock() + defer m.mu.Unlock() + + if m.closed { + return nil + } + + m.closed = true + m.blocks = nil + return nil +} From 5cf19484bcec759586865891cd58f1a83d5058e7 Mon Sep 17 00:00:00 2001 From: Draco Date: Thu, 28 Aug 2025 13:54:22 -0400 Subject: [PATCH 02/18] make copies of the block --- x/blockdb/memory_db.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/blockdb/memory_db.go b/x/blockdb/memory_db.go index 8d957a84f57d..b429d857cb27 100644 --- a/x/blockdb/memory_db.go +++ b/x/blockdb/memory_db.go @@ -32,7 +32,9 @@ func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { return ErrBlockEmpty } - m.blocks[height] = block + blockCopy := make([]byte, len(block)) + copy(blockCopy, block) + m.blocks[height] = blockCopy return nil } @@ -51,7 +53,9 @@ func (m *MemoryDatabase) ReadBlock(height BlockHeight) (BlockData, error) { return nil, ErrBlockNotFound } - return block, nil + blockCopy := make([]byte, len(block)) + copy(blockCopy, block) + return blockCopy, nil } // HasBlock checks if a block exists at the given height From d8d93e1895f3a236fb18ff2c89cdbb03ee603fed Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 2 Sep 2025 14:37:35 -0400 Subject: [PATCH 03/18] refactor memory database --- x/blockdb/memory_db.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/x/blockdb/memory_db.go b/x/blockdb/memory_db.go index b429d857cb27..e4393a85541f 100644 --- a/x/blockdb/memory_db.go +++ b/x/blockdb/memory_db.go @@ -12,13 +12,6 @@ type MemoryDatabase struct { closed bool } -// NewMemoryDatabase creates a new in-memory BlockDB -func NewMemoryDatabase() *MemoryDatabase { - return &MemoryDatabase{ - blocks: make(map[BlockHeight]BlockData), - } -} - // WriteBlock stores a block in memory func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { m.mu.Lock() @@ -28,6 +21,10 @@ func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { return ErrDatabaseClosed } + if m.blocks == nil { + m.blocks = make(map[BlockHeight]BlockData) + } + if len(block) == 0 { return ErrBlockEmpty } @@ -48,8 +45,8 @@ func (m *MemoryDatabase) ReadBlock(height BlockHeight) (BlockData, error) { return nil, ErrDatabaseClosed } - block, exists := m.blocks[height] - if !exists { + block, ok := m.blocks[height] + if !ok { return nil, ErrBlockNotFound } @@ -67,12 +64,15 @@ func (m *MemoryDatabase) HasBlock(height BlockHeight) (bool, error) { return false, ErrDatabaseClosed } - _, exists := m.blocks[height] - return exists, nil + _, ok := m.blocks[height] + return ok, nil } // Inspect returns details about the database -func (*MemoryDatabase) Inspect() (string, error) { +func (m *MemoryDatabase) Inspect() (string, error) { + if m.closed { + return "", ErrDatabaseClosed + } return "", nil } @@ -81,10 +81,6 @@ func (m *MemoryDatabase) Close() error { m.mu.Lock() defer m.mu.Unlock() - if m.closed { - return nil - } - m.closed = true m.blocks = nil return nil From c5e218c96b7981818d7214d563aa90718ff8d6b1 Mon Sep 17 00:00:00 2001 From: Draco Date: Fri, 5 Sep 2025 11:16:52 -0400 Subject: [PATCH 04/18] remove Inspect --- x/blockdb/memory_db.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/x/blockdb/memory_db.go b/x/blockdb/memory_db.go index e4393a85541f..d48bd49d47af 100644 --- a/x/blockdb/memory_db.go +++ b/x/blockdb/memory_db.go @@ -68,14 +68,6 @@ func (m *MemoryDatabase) HasBlock(height BlockHeight) (bool, error) { return ok, nil } -// Inspect returns details about the database -func (m *MemoryDatabase) Inspect() (string, error) { - if m.closed { - return "", ErrDatabaseClosed - } - return "", nil -} - // Close closes the in-memory database func (m *MemoryDatabase) Close() error { m.mu.Lock() From 7645992099318de21d52982860cf28e924e3d76b Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 9 Sep 2025 17:35:29 -0400 Subject: [PATCH 05/18] move memdb into package and add simple tests --- x/blockdb/{memory_db.go => memdb/database.go} | 34 ++++---- x/blockdb/memdb/database_test.go | 77 +++++++++++++++++++ 2 files changed, 96 insertions(+), 15 deletions(-) rename x/blockdb/{memory_db.go => memdb/database.go} (55%) create mode 100644 x/blockdb/memdb/database_test.go diff --git a/x/blockdb/memory_db.go b/x/blockdb/memdb/database.go similarity index 55% rename from x/blockdb/memory_db.go rename to x/blockdb/memdb/database.go index d48bd49d47af..3f4ade00d8e7 100644 --- a/x/blockdb/memory_db.go +++ b/x/blockdb/memdb/database.go @@ -1,32 +1,36 @@ // Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package blockdb +package memdb -import "sync" +import ( + "sync" -// MemoryDatabase is an in-memory implementation of BlockDB -type MemoryDatabase struct { + "github.com/ava-labs/avalanchego/x/blockdb" +) + +// Database is an in-memory implementation of BlockDB +type Database struct { mu sync.RWMutex - blocks map[BlockHeight]BlockData + blocks map[blockdb.BlockHeight]blockdb.BlockData closed bool } // WriteBlock stores a block in memory -func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { +func (m *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockData) error { m.mu.Lock() defer m.mu.Unlock() if m.closed { - return ErrDatabaseClosed + return blockdb.ErrDatabaseClosed } if m.blocks == nil { - m.blocks = make(map[BlockHeight]BlockData) + m.blocks = make(map[blockdb.BlockHeight]blockdb.BlockData) } if len(block) == 0 { - return ErrBlockEmpty + return blockdb.ErrBlockEmpty } blockCopy := make([]byte, len(block)) @@ -37,17 +41,17 @@ func (m *MemoryDatabase) WriteBlock(height BlockHeight, block BlockData) error { } // ReadBlock retrieves the full block data for the given height -func (m *MemoryDatabase) ReadBlock(height BlockHeight) (BlockData, error) { +func (m *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, error) { m.mu.RLock() defer m.mu.RUnlock() if m.closed { - return nil, ErrDatabaseClosed + return nil, blockdb.ErrDatabaseClosed } block, ok := m.blocks[height] if !ok { - return nil, ErrBlockNotFound + return nil, blockdb.ErrBlockNotFound } blockCopy := make([]byte, len(block)) @@ -56,12 +60,12 @@ func (m *MemoryDatabase) ReadBlock(height BlockHeight) (BlockData, error) { } // HasBlock checks if a block exists at the given height -func (m *MemoryDatabase) HasBlock(height BlockHeight) (bool, error) { +func (m *Database) HasBlock(height blockdb.BlockHeight) (bool, error) { m.mu.RLock() defer m.mu.RUnlock() if m.closed { - return false, ErrDatabaseClosed + return false, blockdb.ErrDatabaseClosed } _, ok := m.blocks[height] @@ -69,7 +73,7 @@ func (m *MemoryDatabase) HasBlock(height BlockHeight) (bool, error) { } // Close closes the in-memory database -func (m *MemoryDatabase) Close() error { +func (m *Database) Close() error { m.mu.Lock() defer m.mu.Unlock() diff --git a/x/blockdb/memdb/database_test.go b/x/blockdb/memdb/database_test.go new file mode 100644 index 000000000000..1f101d4c123b --- /dev/null +++ b/x/blockdb/memdb/database_test.go @@ -0,0 +1,77 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package memdb + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/x/blockdb" +) + +func TestOperationsAfterCloseReturnError(t *testing.T) { + db := &Database{} + + // Close database + require.NoError(t, db.Close()) + + height := blockdb.BlockHeight(1) + blockData := blockdb.BlockData("test block data") + + // All operations should fail after close + err := db.WriteBlock(height, blockData) + require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + + _, err = db.ReadBlock(height) + require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + + _, err = db.HasBlock(height) + require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) +} + +func TestWriteReadAndHasBlock(t *testing.T) { + db := &Database{} + + height := blockdb.BlockHeight(1) + blockData := blockdb.BlockData("test block data") + + // Write block + require.NoError(t, db.WriteBlock(height, blockData)) + + // Verify HasBlock returns true + exists, err := db.HasBlock(height) + require.NoError(t, err) + require.True(t, exists) + + // Read block back + retrievedBlock, err := db.ReadBlock(height) + require.NoError(t, err) + require.Equal(t, blockData, retrievedBlock) + + // Verify non-existent block + nonExistentHeight := blockdb.BlockHeight(999) + exists, err = db.HasBlock(nonExistentHeight) + require.NoError(t, err) + require.False(t, exists) +} + +func TestOverwritingBlockUpdatesData(t *testing.T) { + db := &Database{} + + height := blockdb.BlockHeight(1) + originalData := blockdb.BlockData("original data") + updatedData := blockdb.BlockData("updated data") + + // Write original block + require.NoError(t, db.WriteBlock(height, originalData)) + + // Overwrite with new data + require.NoError(t, db.WriteBlock(height, updatedData)) + + // Verify updated data + retrievedBlock, err := db.ReadBlock(height) + require.NoError(t, err) + require.Equal(t, updatedData, retrievedBlock) +} From 9997910aabecf35cf8f0d5df7d0e55e60660cdb1 Mon Sep 17 00:00:00 2001 From: Draco Date: Mon, 15 Sep 2025 10:50:33 -0400 Subject: [PATCH 06/18] fix naming & separate tests --- x/blockdb/memdb/database.go | 44 ++++++++++----------- x/blockdb/memdb/database_test.go | 65 +++++++++++++++++++++++++------- 2 files changed, 73 insertions(+), 36 deletions(-) diff --git a/x/blockdb/memdb/database.go b/x/blockdb/memdb/database.go index 3f4ade00d8e7..2dc6a6d1cf94 100644 --- a/x/blockdb/memdb/database.go +++ b/x/blockdb/memdb/database.go @@ -17,16 +17,16 @@ type Database struct { } // WriteBlock stores a block in memory -func (m *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockData) error { - m.mu.Lock() - defer m.mu.Unlock() +func (d *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockData) error { + d.mu.Lock() + defer d.mu.Unlock() - if m.closed { + if d.closed { return blockdb.ErrDatabaseClosed } - if m.blocks == nil { - m.blocks = make(map[blockdb.BlockHeight]blockdb.BlockData) + if d.blocks == nil { + d.blocks = make(map[blockdb.BlockHeight]blockdb.BlockData) } if len(block) == 0 { @@ -35,21 +35,21 @@ func (m *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockDat blockCopy := make([]byte, len(block)) copy(blockCopy, block) - m.blocks[height] = blockCopy + d.blocks[height] = blockCopy return nil } // ReadBlock retrieves the full block data for the given height -func (m *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, error) { - m.mu.RLock() - defer m.mu.RUnlock() +func (d *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, error) { + d.mu.RLock() + defer d.mu.RUnlock() - if m.closed { + if d.closed { return nil, blockdb.ErrDatabaseClosed } - block, ok := m.blocks[height] + block, ok := d.blocks[height] if !ok { return nil, blockdb.ErrBlockNotFound } @@ -60,24 +60,24 @@ func (m *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, err } // HasBlock checks if a block exists at the given height -func (m *Database) HasBlock(height blockdb.BlockHeight) (bool, error) { - m.mu.RLock() - defer m.mu.RUnlock() +func (d *Database) HasBlock(height blockdb.BlockHeight) (bool, error) { + d.mu.RLock() + defer d.mu.RUnlock() - if m.closed { + if d.closed { return false, blockdb.ErrDatabaseClosed } - _, ok := m.blocks[height] + _, ok := d.blocks[height] return ok, nil } // Close closes the in-memory database -func (m *Database) Close() error { - m.mu.Lock() - defer m.mu.Unlock() +func (d *Database) Close() error { + d.mu.Lock() + defer d.mu.Unlock() - m.closed = true - m.blocks = nil + d.closed = true + d.blocks = nil return nil } diff --git a/x/blockdb/memdb/database_test.go b/x/blockdb/memdb/database_test.go index 1f101d4c123b..8e8dcb7ae3df 100644 --- a/x/blockdb/memdb/database_test.go +++ b/x/blockdb/memdb/database_test.go @@ -20,18 +20,56 @@ func TestOperationsAfterCloseReturnError(t *testing.T) { height := blockdb.BlockHeight(1) blockData := blockdb.BlockData("test block data") - // All operations should fail after close - err := db.WriteBlock(height, blockData) - require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + tests := []struct { + name string + fn func() error + }{ + { + name: "WriteBlock", + fn: func() error { + return db.WriteBlock(height, blockData) + }, + }, + { + name: "ReadBlock", + fn: func() error { + _, err := db.ReadBlock(height) + return err + }, + }, + { + name: "HasBlock", + fn: func() error { + _, err := db.HasBlock(height) + return err + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.fn() + require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + }) + } +} + +func TestWriteAndReadBlock(t *testing.T) { + db := &Database{} - _, err = db.ReadBlock(height) - require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + height := blockdb.BlockHeight(1) + blockData := blockdb.BlockData("test block data") - _, err = db.HasBlock(height) - require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + // Write block + require.NoError(t, db.WriteBlock(height, blockData)) + + // Read block back + retrievedBlock, err := db.ReadBlock(height) + require.NoError(t, err) + require.Equal(t, blockData, retrievedBlock) } -func TestWriteReadAndHasBlock(t *testing.T) { +func TestHasBlockForExistingBlock(t *testing.T) { db := &Database{} height := blockdb.BlockHeight(1) @@ -44,15 +82,14 @@ func TestWriteReadAndHasBlock(t *testing.T) { exists, err := db.HasBlock(height) require.NoError(t, err) require.True(t, exists) +} - // Read block back - retrievedBlock, err := db.ReadBlock(height) - require.NoError(t, err) - require.Equal(t, blockData, retrievedBlock) +func TestHasBlockForNonExistentBlock(t *testing.T) { + db := &Database{} - // Verify non-existent block + // Verify non-existent block returns false nonExistentHeight := blockdb.BlockHeight(999) - exists, err = db.HasBlock(nonExistentHeight) + exists, err := db.HasBlock(nonExistentHeight) require.NoError(t, err) require.False(t, exists) } From 0ce9c2081436e2bcfcfed4499a10e162265f21bf Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 10:43:36 -0400 Subject: [PATCH 07/18] chore: implement database.HeightIndex --- x/blockdb/memdb/database.go | 17 +++++--- x/blockdb/memdb/database_test.go | 67 ++++++++++++++++---------------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/x/blockdb/memdb/database.go b/x/blockdb/memdb/database.go index 2dc6a6d1cf94..ccfb6fae0f28 100644 --- a/x/blockdb/memdb/database.go +++ b/x/blockdb/memdb/database.go @@ -6,9 +6,14 @@ package memdb import ( "sync" + "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/x/blockdb" ) +var ( + _ database.HeightIndex = (*Database)(nil) +) + // Database is an in-memory implementation of BlockDB type Database struct { mu sync.RWMutex @@ -16,8 +21,8 @@ type Database struct { closed bool } -// WriteBlock stores a block in memory -func (d *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockData) error { +// Put stores data in memory at the given height +func (d *Database) Put(height blockdb.BlockHeight, block blockdb.BlockData) error { d.mu.Lock() defer d.mu.Unlock() @@ -40,8 +45,8 @@ func (d *Database) WriteBlock(height blockdb.BlockHeight, block blockdb.BlockDat return nil } -// ReadBlock retrieves the full block data for the given height -func (d *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, error) { +// Get retrieves data at the given height +func (d *Database) Get(height blockdb.BlockHeight) (blockdb.BlockData, error) { d.mu.RLock() defer d.mu.RUnlock() @@ -59,8 +64,8 @@ func (d *Database) ReadBlock(height blockdb.BlockHeight) (blockdb.BlockData, err return blockCopy, nil } -// HasBlock checks if a block exists at the given height -func (d *Database) HasBlock(height blockdb.BlockHeight) (bool, error) { +// Has checks if data exists at the given height +func (d *Database) Has(height blockdb.BlockHeight) (bool, error) { d.mu.RLock() defer d.mu.RUnlock() diff --git a/x/blockdb/memdb/database_test.go b/x/blockdb/memdb/database_test.go index 8e8dcb7ae3df..8f1e053df3ff 100644 --- a/x/blockdb/memdb/database_test.go +++ b/x/blockdb/memdb/database_test.go @@ -25,22 +25,22 @@ func TestOperationsAfterCloseReturnError(t *testing.T) { fn func() error }{ { - name: "WriteBlock", + name: "Put", fn: func() error { - return db.WriteBlock(height, blockData) + return db.Put(height, blockData) }, }, { - name: "ReadBlock", + name: "Get", fn: func() error { - _, err := db.ReadBlock(height) + _, err := db.Get(height) return err }, }, { - name: "HasBlock", + name: "Has", fn: func() error { - _, err := db.HasBlock(height) + _, err := db.Has(height) return err }, }, @@ -54,47 +54,46 @@ func TestOperationsAfterCloseReturnError(t *testing.T) { } } -func TestWriteAndReadBlock(t *testing.T) { +func TestPut(t *testing.T) { db := &Database{} height := blockdb.BlockHeight(1) blockData := blockdb.BlockData("test block data") - - // Write block - require.NoError(t, db.WriteBlock(height, blockData)) - - // Read block back - retrievedBlock, err := db.ReadBlock(height) - require.NoError(t, err) - require.Equal(t, blockData, retrievedBlock) + require.NoError(t, db.Put(height, blockData)) } -func TestHasBlockForExistingBlock(t *testing.T) { +func TestGet(t *testing.T) { db := &Database{} height := blockdb.BlockHeight(1) blockData := blockdb.BlockData("test block data") + require.NoError(t, db.Put(height, blockData)) - // Write block - require.NoError(t, db.WriteBlock(height, blockData)) - - // Verify HasBlock returns true - exists, err := db.HasBlock(height) + // Read block back + retrievedBlock, err := db.Get(height) require.NoError(t, err) - require.True(t, exists) + require.Equal(t, blockData, retrievedBlock) } -func TestHasBlockForNonExistentBlock(t *testing.T) { - db := &Database{} - - // Verify non-existent block returns false - nonExistentHeight := blockdb.BlockHeight(999) - exists, err := db.HasBlock(nonExistentHeight) - require.NoError(t, err) - require.False(t, exists) +func TestHas(t *testing.T) { + t.Run("non-existent block", func(t *testing.T) { + db := &Database{} + exists, err := db.Has(blockdb.BlockHeight(1)) + require.NoError(t, err) + require.False(t, exists) + }) + + t.Run("existing block", func(t *testing.T) { + db := &Database{} + blockData := blockdb.BlockData("test block data") + require.NoError(t, db.Put(blockdb.BlockHeight(1), blockData)) + exists, err := db.Has(blockdb.BlockHeight(1)) + require.NoError(t, err) + require.True(t, exists) + }) } -func TestOverwritingBlockUpdatesData(t *testing.T) { +func TestPut_Overwrite(t *testing.T) { db := &Database{} height := blockdb.BlockHeight(1) @@ -102,13 +101,13 @@ func TestOverwritingBlockUpdatesData(t *testing.T) { updatedData := blockdb.BlockData("updated data") // Write original block - require.NoError(t, db.WriteBlock(height, originalData)) + require.NoError(t, db.Put(height, originalData)) // Overwrite with new data - require.NoError(t, db.WriteBlock(height, updatedData)) + require.NoError(t, db.Put(height, updatedData)) // Verify updated data - retrievedBlock, err := db.ReadBlock(height) + retrievedBlock, err := db.Get(height) require.NoError(t, err) require.Equal(t, updatedData, retrievedBlock) } From cde510e551afcb40e81edf0e3eee14bb7811b099 Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 10:53:18 -0400 Subject: [PATCH 08/18] feat: update to use database.HeightIndex and move out of blockdb --- database/heightindexdb/memdb/database.go | 87 ++++++++++++++++++ .../heightindexdb}/memdb/database_test.go | 32 +++---- x/blockdb/memdb/database.go | 88 ------------------- 3 files changed, 103 insertions(+), 104 deletions(-) create mode 100644 database/heightindexdb/memdb/database.go rename {x/blockdb => database/heightindexdb}/memdb/database_test.go (68%) delete mode 100644 x/blockdb/memdb/database.go diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go new file mode 100644 index 000000000000..ce4811827dd4 --- /dev/null +++ b/database/heightindexdb/memdb/database.go @@ -0,0 +1,87 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package memdb + +import ( + "sync" + + "github.com/ava-labs/avalanchego/database" +) + +var ( + _ database.HeightIndex = (*Database)(nil) +) + +// Database is an in-memory implementation of database.HeightIndex +type Database struct { + mu sync.RWMutex + data map[uint64][]byte + closed bool +} + +// Put stores data in memory at the given height +func (d *Database) Put(height uint64, data []byte) error { + d.mu.Lock() + defer d.mu.Unlock() + + if d.closed { + return database.ErrClosed + } + + if d.data == nil { + d.data = make(map[uint64][]byte) + } + + if len(data) == 0 { + return database.ErrNotFound + } + + dataCopy := make([]byte, len(data)) + copy(dataCopy, data) + d.data[height] = dataCopy + + return nil +} + +// Get retrieves data at the given height +func (d *Database) Get(height uint64) ([]byte, error) { + d.mu.RLock() + defer d.mu.RUnlock() + + if d.closed { + return nil, database.ErrClosed + } + + data, ok := d.data[height] + if !ok { + return nil, database.ErrNotFound + } + + dataCopy := make([]byte, len(data)) + copy(dataCopy, data) + return dataCopy, nil +} + +// Has checks if data exists at the given height +func (d *Database) Has(height uint64) (bool, error) { + d.mu.RLock() + defer d.mu.RUnlock() + + if d.closed { + return false, database.ErrClosed + } + + _, ok := d.data[height] + return ok, nil +} + +// Close closes the in-memory database +func (d *Database) Close() error { + d.mu.Lock() + defer d.mu.Unlock() + + d.closed = true + d.data = nil + return nil +} diff --git a/x/blockdb/memdb/database_test.go b/database/heightindexdb/memdb/database_test.go similarity index 68% rename from x/blockdb/memdb/database_test.go rename to database/heightindexdb/memdb/database_test.go index 8f1e053df3ff..cbd82366919b 100644 --- a/x/blockdb/memdb/database_test.go +++ b/database/heightindexdb/memdb/database_test.go @@ -8,17 +8,17 @@ import ( "github.com/stretchr/testify/require" - "github.com/ava-labs/avalanchego/x/blockdb" + "github.com/ava-labs/avalanchego/database" ) -func TestOperationsAfterCloseReturnError(t *testing.T) { +func TestOperationsAfterClose(t *testing.T) { db := &Database{} // Close database require.NoError(t, db.Close()) - height := blockdb.BlockHeight(1) - blockData := blockdb.BlockData("test block data") + height := uint64(1) + blockData := []byte("test block data") tests := []struct { name string @@ -49,7 +49,7 @@ func TestOperationsAfterCloseReturnError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := tt.fn() - require.ErrorIs(t, err, blockdb.ErrDatabaseClosed) + require.ErrorIs(t, err, database.ErrClosed) }) } } @@ -57,16 +57,16 @@ func TestOperationsAfterCloseReturnError(t *testing.T) { func TestPut(t *testing.T) { db := &Database{} - height := blockdb.BlockHeight(1) - blockData := blockdb.BlockData("test block data") + height := uint64(1) + blockData := []byte("test block data") require.NoError(t, db.Put(height, blockData)) } func TestGet(t *testing.T) { db := &Database{} - height := blockdb.BlockHeight(1) - blockData := blockdb.BlockData("test block data") + height := uint64(1) + blockData := []byte("test block data") require.NoError(t, db.Put(height, blockData)) // Read block back @@ -78,16 +78,16 @@ func TestGet(t *testing.T) { func TestHas(t *testing.T) { t.Run("non-existent block", func(t *testing.T) { db := &Database{} - exists, err := db.Has(blockdb.BlockHeight(1)) + exists, err := db.Has(uint64(1)) require.NoError(t, err) require.False(t, exists) }) t.Run("existing block", func(t *testing.T) { db := &Database{} - blockData := blockdb.BlockData("test block data") - require.NoError(t, db.Put(blockdb.BlockHeight(1), blockData)) - exists, err := db.Has(blockdb.BlockHeight(1)) + blockData := []byte("test block data") + require.NoError(t, db.Put(uint64(1), blockData)) + exists, err := db.Has(uint64(1)) require.NoError(t, err) require.True(t, exists) }) @@ -96,9 +96,9 @@ func TestHas(t *testing.T) { func TestPut_Overwrite(t *testing.T) { db := &Database{} - height := blockdb.BlockHeight(1) - originalData := blockdb.BlockData("original data") - updatedData := blockdb.BlockData("updated data") + height := uint64(1) + originalData := []byte("original data") + updatedData := []byte("updated data") // Write original block require.NoError(t, db.Put(height, originalData)) diff --git a/x/blockdb/memdb/database.go b/x/blockdb/memdb/database.go deleted file mode 100644 index ccfb6fae0f28..000000000000 --- a/x/blockdb/memdb/database.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package memdb - -import ( - "sync" - - "github.com/ava-labs/avalanchego/database" - "github.com/ava-labs/avalanchego/x/blockdb" -) - -var ( - _ database.HeightIndex = (*Database)(nil) -) - -// Database is an in-memory implementation of BlockDB -type Database struct { - mu sync.RWMutex - blocks map[blockdb.BlockHeight]blockdb.BlockData - closed bool -} - -// Put stores data in memory at the given height -func (d *Database) Put(height blockdb.BlockHeight, block blockdb.BlockData) error { - d.mu.Lock() - defer d.mu.Unlock() - - if d.closed { - return blockdb.ErrDatabaseClosed - } - - if d.blocks == nil { - d.blocks = make(map[blockdb.BlockHeight]blockdb.BlockData) - } - - if len(block) == 0 { - return blockdb.ErrBlockEmpty - } - - blockCopy := make([]byte, len(block)) - copy(blockCopy, block) - d.blocks[height] = blockCopy - - return nil -} - -// Get retrieves data at the given height -func (d *Database) Get(height blockdb.BlockHeight) (blockdb.BlockData, error) { - d.mu.RLock() - defer d.mu.RUnlock() - - if d.closed { - return nil, blockdb.ErrDatabaseClosed - } - - block, ok := d.blocks[height] - if !ok { - return nil, blockdb.ErrBlockNotFound - } - - blockCopy := make([]byte, len(block)) - copy(blockCopy, block) - return blockCopy, nil -} - -// Has checks if data exists at the given height -func (d *Database) Has(height blockdb.BlockHeight) (bool, error) { - d.mu.RLock() - defer d.mu.RUnlock() - - if d.closed { - return false, blockdb.ErrDatabaseClosed - } - - _, ok := d.blocks[height] - return ok, nil -} - -// Close closes the in-memory database -func (d *Database) Close() error { - d.mu.Lock() - defer d.mu.Unlock() - - d.closed = true - d.blocks = nil - return nil -} From be762aa9e3bb849ec60caddc7f7285bfc4e90c7b Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 11:19:11 -0400 Subject: [PATCH 09/18] fix lint issues --- database/heightindexdb/memdb/database.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go index ce4811827dd4..93800ff30fd8 100644 --- a/database/heightindexdb/memdb/database.go +++ b/database/heightindexdb/memdb/database.go @@ -9,9 +9,7 @@ import ( "github.com/ava-labs/avalanchego/database" ) -var ( - _ database.HeightIndex = (*Database)(nil) -) +var _ database.HeightIndex = (*Database)(nil) // Database is an in-memory implementation of database.HeightIndex type Database struct { From e70e213ea72c24ca9ab026b761c49efc0d2ae57c Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 18:05:00 -0400 Subject: [PATCH 10/18] create dbtest package for heightindexdb --- database/database.go | 19 +- database/heightindexdb/dbtest/dbtest.go | 240 ++++++++++++++++++ database/heightindexdb/memdb/database.go | 64 +++-- database/heightindexdb/memdb/database_test.go | 104 +------- 4 files changed, 297 insertions(+), 130 deletions(-) create mode 100644 database/heightindexdb/dbtest/dbtest.go diff --git a/database/database.go b/database/database.go index efa63fc87b46..d015a542d712 100644 --- a/database/database.go +++ b/database/database.go @@ -95,15 +95,24 @@ type Database interface { health.Checker } -// HeightIndex defines the interface for storing and retrieving entries by height. +// HeightIndex defines the interface for storing and retrieving values by height. type HeightIndex interface { - // Put inserts the entry into the store at the given height. - Put(height uint64, bytes []byte) error + // Put inserts the value into the database at the given height. + // + // If [value] is nil or an empty slice, it's retrieved value will be nil. + // + // Note: [value] is safe to read and modify after calling Put. + Put(height uint64, value []byte) error - // Get retrieves an entry by its height. + // Get retrieves a value by its height. + // Returns ErrNotFound if the key is not present in the database. + // + // Note: Get always returns a new copy of the data. Get(height uint64) ([]byte, error) - // Has checks if an entry exists at the given height. + // Has checks if a value exists at the given height. + // + // Return true even if the stored value is nil, or empty. Has(height uint64) (bool, error) // Close closes the database. diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go new file mode 100644 index 000000000000..7af80ea3a880 --- /dev/null +++ b/database/heightindexdb/dbtest/dbtest.go @@ -0,0 +1,240 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package dbtest + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/database" +) + +// Tests is a list of all database tests +var Tests = map[string]func(t *testing.T, newDB func() database.HeightIndex){ + "PutGet": TestPutGet, + "Has": TestHas, + "CloseAndPut": TestCloseAndPut, + "CloseAndGet": TestCloseAndGet, + "CloseAndHas": TestCloseAndHas, + "ModifyValueAfterPut": TestModifyValueAfterPut, + "ModifyValueAfterGet": TestModifyValueAfterGet, +} + +type putArgs struct { + height uint64 + data []byte +} + +func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { + tests := []struct { + name string + puts []putArgs + queryHeight uint64 + expected []byte + expectedErr error + }{ + { + name: "normal operation", + puts: []putArgs{ + {1, []byte("test data 1")}, + }, + queryHeight: 1, + expected: []byte("test data 1"), + }, + { + name: "not found error when getting on non-existing height", + puts: []putArgs{ + {1, []byte("test data")}, + }, + queryHeight: 2, + expected: nil, + expectedErr: database.ErrNotFound, + }, + { + name: "overwriting data on same height", + puts: []putArgs{ + {1, []byte("original data")}, + {1, []byte("overwritten data")}, + }, + queryHeight: 1, + expected: []byte("overwritten data"), + }, + { + name: "put and get nil data", + puts: []putArgs{ + {1, nil}, + }, + queryHeight: 1, + expected: nil, + }, + { + name: "put and get empty data", + puts: []putArgs{ + {1, []byte{}}, + }, + queryHeight: 1, + expected: nil, + }, + { + name: "put and get large data", + puts: []putArgs{ + {1, make([]byte, 1000)}, + }, + queryHeight: 1, + expected: make([]byte, 1000), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db := newDB() + defer db.Close() + + // Perform all puts + for _, write := range tt.puts { + require.NoError(t, db.Put(write.height, write.data)) + } + + // Query the specific height + retrievedData, err := db.Get(tt.queryHeight) + if tt.expectedErr != nil { + require.Equal(t, tt.expectedErr, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.expected, retrievedData) + } + }) + } +} + +func TestHas(t *testing.T, newDB func() database.HeightIndex) { + tests := []struct { + name string + puts []putArgs + queryHeight uint64 + expected bool + }{ + { + name: "non-existent item", + puts: []putArgs{}, + queryHeight: 1, + expected: false, + }, + { + name: "existing item with data", + puts: []putArgs{{1, []byte("test data")}}, + queryHeight: 1, + expected: true, + }, + { + name: "existing item with nil data", + puts: []putArgs{{1, nil}}, + queryHeight: 1, + expected: true, + }, + { + name: "existing item with empty bytes", + puts: []putArgs{{1, []byte{}}}, + queryHeight: 1, + expected: true, + }, + { + name: "has returns true on overridden height", + puts: []putArgs{ + {1, []byte("original data")}, + {1, []byte("overridden data")}, + }, + queryHeight: 1, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db := newDB() + defer db.Close() + + // Perform all puts + for _, write := range tt.puts { + require.NoError(t, db.Put(write.height, write.data)) + } + + exists, err := db.Has(tt.queryHeight) + require.NoError(t, err) + require.Equal(t, tt.expected, exists) + }) + } +} + +func TestCloseAndPut(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + require.NoError(t, db.Close()) + + // Try to put after close - should return error + err := db.Put(1, []byte("test")) + require.ErrorIs(t, err, database.ErrClosed) +} + +func TestCloseAndGet(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + require.NoError(t, db.Close()) + + // Try to get after close - should return error + _, err := db.Get(1) + require.ErrorIs(t, err, database.ErrClosed) +} + +func TestCloseAndHas(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + require.NoError(t, db.Close()) + + // Try to has after close - should return error + _, err := db.Has(1) + require.ErrorIs(t, err, database.ErrClosed) +} + +func TestModifyValueAfterPut(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + defer db.Close() + + originalData := []byte("original data") + modifiedData := []byte("modified data") + + // Put original data + require.NoError(t, db.Put(1, originalData)) + + // Modify the original data slice + copy(originalData, modifiedData) + + // Get the data back - should still be the original data, not the modified slice + retrievedData, err := db.Get(1) + require.NoError(t, err) + require.Equal(t, []byte("original data"), retrievedData) + require.NotEqual(t, modifiedData, retrievedData) +} + +func TestModifyValueAfterGet(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + defer db.Close() + + originalData := []byte("original data") + + // Put original data + require.NoError(t, db.Put(1, originalData)) + + // Get the data + retrievedData, err := db.Get(1) + require.NoError(t, err) + require.Equal(t, originalData, retrievedData) + + // Modify the retrieved data + copy(retrievedData, []byte("modified data")) + + // Get the data again - should still be the original data + retrievedData2, err := db.Get(1) + require.NoError(t, err) + require.Equal(t, originalData, retrievedData2) + require.NotEqual(t, retrievedData, retrievedData2) +} diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go index 93800ff30fd8..485eaa500c24 100644 --- a/database/heightindexdb/memdb/database.go +++ b/database/heightindexdb/memdb/database.go @@ -18,68 +18,80 @@ type Database struct { closed bool } +func New() *Database { + return &Database{ + data: make(map[uint64][]byte), + } +} + // Put stores data in memory at the given height -func (d *Database) Put(height uint64, data []byte) error { - d.mu.Lock() - defer d.mu.Unlock() +func (db *Database) Put(height uint64, data []byte) error { + db.mu.Lock() + defer db.mu.Unlock() - if d.closed { + if db.closed { return database.ErrClosed } - if d.data == nil { - d.data = make(map[uint64][]byte) + if db.data == nil { + db.data = make(map[uint64][]byte) } if len(data) == 0 { - return database.ErrNotFound + // don't save empty slice if data is nil or empty + db.data[height] = nil + } else { + dataCopy := make([]byte, len(data)) + copy(dataCopy, data) + db.data[height] = dataCopy } - dataCopy := make([]byte, len(data)) - copy(dataCopy, data) - d.data[height] = dataCopy - return nil } // Get retrieves data at the given height -func (d *Database) Get(height uint64) ([]byte, error) { - d.mu.RLock() - defer d.mu.RUnlock() +func (db *Database) Get(height uint64) ([]byte, error) { + db.mu.RLock() + defer db.mu.RUnlock() - if d.closed { + if db.closed { return nil, database.ErrClosed } - data, ok := d.data[height] + data, ok := db.data[height] if !ok { return nil, database.ErrNotFound } + // don't return empty slice if data is nil or empty + if data == nil { + return nil, nil + } + dataCopy := make([]byte, len(data)) copy(dataCopy, data) return dataCopy, nil } // Has checks if data exists at the given height -func (d *Database) Has(height uint64) (bool, error) { - d.mu.RLock() - defer d.mu.RUnlock() +func (db *Database) Has(height uint64) (bool, error) { + db.mu.RLock() + defer db.mu.RUnlock() - if d.closed { + if db.closed { return false, database.ErrClosed } - _, ok := d.data[height] + _, ok := db.data[height] return ok, nil } // Close closes the in-memory database -func (d *Database) Close() error { - d.mu.Lock() - defer d.mu.Unlock() +func (db *Database) Close() error { + db.mu.Lock() + defer db.mu.Unlock() - d.closed = true - d.data = nil + db.closed = true + db.data = nil return nil } diff --git a/database/heightindexdb/memdb/database_test.go b/database/heightindexdb/memdb/database_test.go index cbd82366919b..62dc9ef10d09 100644 --- a/database/heightindexdb/memdb/database_test.go +++ b/database/heightindexdb/memdb/database_test.go @@ -6,108 +6,14 @@ package memdb import ( "testing" - "github.com/stretchr/testify/require" - "github.com/ava-labs/avalanchego/database" + "github.com/ava-labs/avalanchego/database/heightindexdb/dbtest" ) -func TestOperationsAfterClose(t *testing.T) { - db := &Database{} - - // Close database - require.NoError(t, db.Close()) - - height := uint64(1) - blockData := []byte("test block data") - - tests := []struct { - name string - fn func() error - }{ - { - name: "Put", - fn: func() error { - return db.Put(height, blockData) - }, - }, - { - name: "Get", - fn: func() error { - _, err := db.Get(height) - return err - }, - }, - { - name: "Has", - fn: func() error { - _, err := db.Has(height) - return err - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.fn() - require.ErrorIs(t, err, database.ErrClosed) +func TestInterface(t *testing.T) { + for name, test := range dbtest.Tests { + t.Run(name, func(t *testing.T) { + test(t, func() database.HeightIndex { return New() }) }) } } - -func TestPut(t *testing.T) { - db := &Database{} - - height := uint64(1) - blockData := []byte("test block data") - require.NoError(t, db.Put(height, blockData)) -} - -func TestGet(t *testing.T) { - db := &Database{} - - height := uint64(1) - blockData := []byte("test block data") - require.NoError(t, db.Put(height, blockData)) - - // Read block back - retrievedBlock, err := db.Get(height) - require.NoError(t, err) - require.Equal(t, blockData, retrievedBlock) -} - -func TestHas(t *testing.T) { - t.Run("non-existent block", func(t *testing.T) { - db := &Database{} - exists, err := db.Has(uint64(1)) - require.NoError(t, err) - require.False(t, exists) - }) - - t.Run("existing block", func(t *testing.T) { - db := &Database{} - blockData := []byte("test block data") - require.NoError(t, db.Put(uint64(1), blockData)) - exists, err := db.Has(uint64(1)) - require.NoError(t, err) - require.True(t, exists) - }) -} - -func TestPut_Overwrite(t *testing.T) { - db := &Database{} - - height := uint64(1) - originalData := []byte("original data") - updatedData := []byte("updated data") - - // Write original block - require.NoError(t, db.Put(height, originalData)) - - // Overwrite with new data - require.NoError(t, db.Put(height, updatedData)) - - // Verify updated data - retrievedBlock, err := db.Get(height) - require.NoError(t, err) - require.Equal(t, updatedData, retrievedBlock) -} From 88f2394902c0ea570cfd0ecde7562c675a2a270c Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 18:18:56 -0400 Subject: [PATCH 11/18] update close behaviour on close --- database/database.go | 2 ++ database/heightindexdb/dbtest/dbtest.go | 10 ++++++++++ database/heightindexdb/memdb/database.go | 13 +++++++------ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/database/database.go b/database/database.go index d015a542d712..2eeb64a7aa15 100644 --- a/database/database.go +++ b/database/database.go @@ -116,5 +116,7 @@ type HeightIndex interface { Has(height uint64) (bool, error) // Close closes the database. + // + // Note: Calling Close after Close returns ErrClosed. io.Closer } diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index 7af80ea3a880..de3f6b33ec42 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -18,6 +18,7 @@ var Tests = map[string]func(t *testing.T, newDB func() database.HeightIndex){ "CloseAndPut": TestCloseAndPut, "CloseAndGet": TestCloseAndGet, "CloseAndHas": TestCloseAndHas, + "Close": TestClose, "ModifyValueAfterPut": TestModifyValueAfterPut, "ModifyValueAfterGet": TestModifyValueAfterGet, } @@ -238,3 +239,12 @@ func TestModifyValueAfterGet(t *testing.T, newDB func() database.HeightIndex) { require.Equal(t, originalData, retrievedData2) require.NotEqual(t, retrievedData, retrievedData2) } + +func TestClose(t *testing.T, newDB func() database.HeightIndex) { + db := newDB() + require.NoError(t, db.Close()) + + // Second close should return error + err := db.Close() + require.ErrorIs(t, err, database.ErrClosed) +} diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go index 485eaa500c24..20c43057d902 100644 --- a/database/heightindexdb/memdb/database.go +++ b/database/heightindexdb/memdb/database.go @@ -4,6 +4,7 @@ package memdb import ( + "slices" "sync" "github.com/ava-labs/avalanchego/database" @@ -41,9 +42,7 @@ func (db *Database) Put(height uint64, data []byte) error { // don't save empty slice if data is nil or empty db.data[height] = nil } else { - dataCopy := make([]byte, len(data)) - copy(dataCopy, data) - db.data[height] = dataCopy + db.data[height] = slices.Clone(data) } return nil @@ -68,9 +67,7 @@ func (db *Database) Get(height uint64) ([]byte, error) { return nil, nil } - dataCopy := make([]byte, len(data)) - copy(dataCopy, data) - return dataCopy, nil + return slices.Clone(data), nil } // Has checks if data exists at the given height @@ -91,6 +88,10 @@ func (db *Database) Close() error { db.mu.Lock() defer db.mu.Unlock() + if db.closed { + return database.ErrClosed + } + db.closed = true db.data = nil return nil From 0f9a5d951cefd93613fe9fa6b77abb3c332276c3 Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 23 Sep 2025 18:45:36 -0400 Subject: [PATCH 12/18] clarify Put behaviour --- database/database.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/database/database.go b/database/database.go index 2eeb64a7aa15..e4212ee6e442 100644 --- a/database/database.go +++ b/database/database.go @@ -99,7 +99,8 @@ type Database interface { type HeightIndex interface { // Put inserts the value into the database at the given height. // - // If [value] is nil or an empty slice, it's retrieved value will be nil. + // A nil or empty slice for [value] is allowed and will be returned as nil + // when retrieved via Get. // // Note: [value] is safe to read and modify after calling Put. Put(height uint64, value []byte) error From 1769a5c33de16a6968d5737e1d59abe80323d4ab Mon Sep 17 00:00:00 2001 From: Draco Date: Thu, 25 Sep 2025 15:39:29 -0400 Subject: [PATCH 13/18] fix test case --- database/heightindexdb/dbtest/dbtest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index de3f6b33ec42..04e3c46c6efc 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -101,7 +101,7 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { // Query the specific height retrievedData, err := db.Get(tt.queryHeight) if tt.expectedErr != nil { - require.Equal(t, tt.expectedErr, err) + require.ErrorIs(t, err, tt.expectedErr) } else { require.NoError(t, err) require.Equal(t, tt.expected, retrievedData) From 5454574527e96bb93dfe7bf838337e1aaaf99a93 Mon Sep 17 00:00:00 2001 From: Draco Date: Mon, 29 Sep 2025 14:04:17 -0400 Subject: [PATCH 14/18] fix: pr feedback --- database/heightindexdb/dbtest/dbtest.go | 129 +++++++----------- database/heightindexdb/memdb/database.go | 11 -- database/heightindexdb/memdb/database_test.go | 6 +- 3 files changed, 52 insertions(+), 94 deletions(-) diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index 04e3c46c6efc..d337092391c4 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -11,16 +11,19 @@ import ( "github.com/ava-labs/avalanchego/database" ) +type testCase struct { + Name string + Test func(t *testing.T, newDB func() database.HeightIndex) +} + // Tests is a list of all database tests -var Tests = map[string]func(t *testing.T, newDB func() database.HeightIndex){ - "PutGet": TestPutGet, - "Has": TestHas, - "CloseAndPut": TestCloseAndPut, - "CloseAndGet": TestCloseAndGet, - "CloseAndHas": TestCloseAndHas, - "Close": TestClose, - "ModifyValueAfterPut": TestModifyValueAfterPut, - "ModifyValueAfterGet": TestModifyValueAfterGet, +var Tests = []testCase{ + {"TestPutGet", TestPutGet}, + {"TestHas", TestHas}, + {"TestCloseAndPut", TestCloseAndPut}, + {"TestCloseAndGet", TestCloseAndGet}, + {"TestCloseAndHas", TestCloseAndHas}, + {"TestClose", TestClose}, } type putArgs struct { @@ -33,8 +36,8 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { name string puts []putArgs queryHeight uint64 - expected []byte - expectedErr error + want []byte + wantErr error }{ { name: "normal operation", @@ -42,7 +45,7 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, []byte("test data 1")}, }, queryHeight: 1, - expected: []byte("test data 1"), + want: []byte("test data 1"), }, { name: "not found error when getting on non-existing height", @@ -50,8 +53,7 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, []byte("test data")}, }, queryHeight: 2, - expected: nil, - expectedErr: database.ErrNotFound, + wantErr: database.ErrNotFound, }, { name: "overwriting data on same height", @@ -60,7 +62,7 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, []byte("overwritten data")}, }, queryHeight: 1, - expected: []byte("overwritten data"), + want: []byte("overwritten data"), }, { name: "put and get nil data", @@ -68,15 +70,15 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, nil}, }, queryHeight: 1, - expected: nil, + want: nil, }, { - name: "put and get empty data", + name: "put empty bytes and get nil", puts: []putArgs{ {1, []byte{}}, }, queryHeight: 1, - expected: nil, + want: nil, }, { name: "put and get large data", @@ -84,28 +86,39 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, make([]byte, 1000)}, }, queryHeight: 1, - expected: make([]byte, 1000), + want: make([]byte, 1000), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { db := newDB() - defer db.Close() + defer func() { + require.NoError(t, db.Close()) + }() // Perform all puts for _, write := range tt.puts { require.NoError(t, db.Put(write.height, write.data)) } + // modify the original value of the put data to ensure the saved + // value won't be changed after Get + if len(tt.puts) > int(tt.queryHeight) && tt.puts[tt.queryHeight].data != nil { + copy(tt.puts[tt.queryHeight].data, []byte("modified data")) + } + // Query the specific height retrievedData, err := db.Get(tt.queryHeight) - if tt.expectedErr != nil { - require.ErrorIs(t, err, tt.expectedErr) - } else { - require.NoError(t, err) - require.Equal(t, tt.expected, retrievedData) - } + require.ErrorIs(t, err, tt.wantErr) + require.Equal(t, tt.want, retrievedData) + + // modify the data returned from Get and ensure it won't change the + // data from a second Get + copy(retrievedData, []byte("modified data")) + newData, err := db.Get(tt.queryHeight) + require.ErrorIs(t, err, tt.wantErr) + require.Equal(t, tt.want, newData) }) } } @@ -115,31 +128,29 @@ func TestHas(t *testing.T, newDB func() database.HeightIndex) { name string puts []putArgs queryHeight uint64 - expected bool + want bool }{ { name: "non-existent item", - puts: []putArgs{}, queryHeight: 1, - expected: false, }, { name: "existing item with data", puts: []putArgs{{1, []byte("test data")}}, queryHeight: 1, - expected: true, + want: true, }, { name: "existing item with nil data", puts: []putArgs{{1, nil}}, queryHeight: 1, - expected: true, + want: true, }, { name: "existing item with empty bytes", puts: []putArgs{{1, []byte{}}}, queryHeight: 1, - expected: true, + want: true, }, { name: "has returns true on overridden height", @@ -148,23 +159,25 @@ func TestHas(t *testing.T, newDB func() database.HeightIndex) { {1, []byte("overridden data")}, }, queryHeight: 1, - expected: true, + want: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { db := newDB() - defer db.Close() + defer func() { + require.NoError(t, db.Close()) + }() // Perform all puts for _, write := range tt.puts { require.NoError(t, db.Put(write.height, write.data)) } - exists, err := db.Has(tt.queryHeight) + ok, err := db.Has(tt.queryHeight) require.NoError(t, err) - require.Equal(t, tt.expected, exists) + require.Equal(t, tt.want, ok) }) } } @@ -196,50 +209,6 @@ func TestCloseAndHas(t *testing.T, newDB func() database.HeightIndex) { require.ErrorIs(t, err, database.ErrClosed) } -func TestModifyValueAfterPut(t *testing.T, newDB func() database.HeightIndex) { - db := newDB() - defer db.Close() - - originalData := []byte("original data") - modifiedData := []byte("modified data") - - // Put original data - require.NoError(t, db.Put(1, originalData)) - - // Modify the original data slice - copy(originalData, modifiedData) - - // Get the data back - should still be the original data, not the modified slice - retrievedData, err := db.Get(1) - require.NoError(t, err) - require.Equal(t, []byte("original data"), retrievedData) - require.NotEqual(t, modifiedData, retrievedData) -} - -func TestModifyValueAfterGet(t *testing.T, newDB func() database.HeightIndex) { - db := newDB() - defer db.Close() - - originalData := []byte("original data") - - // Put original data - require.NoError(t, db.Put(1, originalData)) - - // Get the data - retrievedData, err := db.Get(1) - require.NoError(t, err) - require.Equal(t, originalData, retrievedData) - - // Modify the retrieved data - copy(retrievedData, []byte("modified data")) - - // Get the data again - should still be the original data - retrievedData2, err := db.Get(1) - require.NoError(t, err) - require.Equal(t, originalData, retrievedData2) - require.NotEqual(t, retrievedData, retrievedData2) -} - func TestClose(t *testing.T, newDB func() database.HeightIndex) { db := newDB() require.NoError(t, db.Close()) diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go index 20c43057d902..ccb334cdb079 100644 --- a/database/heightindexdb/memdb/database.go +++ b/database/heightindexdb/memdb/database.go @@ -19,12 +19,6 @@ type Database struct { closed bool } -func New() *Database { - return &Database{ - data: make(map[uint64][]byte), - } -} - // Put stores data in memory at the given height func (db *Database) Put(height uint64, data []byte) error { db.mu.Lock() @@ -62,11 +56,6 @@ func (db *Database) Get(height uint64) ([]byte, error) { return nil, database.ErrNotFound } - // don't return empty slice if data is nil or empty - if data == nil { - return nil, nil - } - return slices.Clone(data), nil } diff --git a/database/heightindexdb/memdb/database_test.go b/database/heightindexdb/memdb/database_test.go index 62dc9ef10d09..260c1a1b5518 100644 --- a/database/heightindexdb/memdb/database_test.go +++ b/database/heightindexdb/memdb/database_test.go @@ -11,9 +11,9 @@ import ( ) func TestInterface(t *testing.T) { - for name, test := range dbtest.Tests { - t.Run(name, func(t *testing.T) { - test(t, func() database.HeightIndex { return New() }) + for _, test := range dbtest.Tests { + t.Run("memdb_"+test.Name, func(t *testing.T) { + test.Test(t, func() database.HeightIndex { return &Database{} }) }) } } From 344b0e1f8e2f1cd2ed6aa34dfb7b65dc004706d4 Mon Sep 17 00:00:00 2001 From: Draco Date: Mon, 29 Sep 2025 14:12:58 -0400 Subject: [PATCH 15/18] update interface description --- database/database.go | 12 ++++++------ database/heightindexdb/dbtest/dbtest.go | 4 ++-- database/heightindexdb/memdb/database.go | 12 +----------- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/database/database.go b/database/database.go index e4212ee6e442..566a52bfd187 100644 --- a/database/database.go +++ b/database/database.go @@ -99,25 +99,25 @@ type Database interface { type HeightIndex interface { // Put inserts the value into the database at the given height. // - // A nil or empty slice for [value] is allowed and will be returned as nil - // when retrieved via Get. + // A nil or empty slice for [value] is accepted and will return their respective + // value if retrieved via Get. // - // Note: [value] is safe to read and modify after calling Put. + // [value] is safe to read and modify after calling Put. Put(height uint64, value []byte) error // Get retrieves a value by its height. // Returns ErrNotFound if the key is not present in the database. // - // Note: Get always returns a new copy of the data. + // Returned []byte is safe to read and modify after calling Get. Get(height uint64) ([]byte, error) // Has checks if a value exists at the given height. // - // Return true even if the stored value is nil, or empty. + // Returns true even if the stored value is nil or empty. Has(height uint64) (bool, error) // Close closes the database. // - // Note: Calling Close after Close returns ErrClosed. + // Calling Close after Close returns ErrClosed. io.Closer } diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index d337092391c4..3784f28027b3 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -73,12 +73,12 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { want: nil, }, { - name: "put empty bytes and get nil", + name: "put and get empty bytes", puts: []putArgs{ {1, []byte{}}, }, queryHeight: 1, - want: nil, + want: []byte{}, }, { name: "put and get large data", diff --git a/database/heightindexdb/memdb/database.go b/database/heightindexdb/memdb/database.go index ccb334cdb079..8f9ea900f60b 100644 --- a/database/heightindexdb/memdb/database.go +++ b/database/heightindexdb/memdb/database.go @@ -19,7 +19,6 @@ type Database struct { closed bool } -// Put stores data in memory at the given height func (db *Database) Put(height uint64, data []byte) error { db.mu.Lock() defer db.mu.Unlock() @@ -32,17 +31,10 @@ func (db *Database) Put(height uint64, data []byte) error { db.data = make(map[uint64][]byte) } - if len(data) == 0 { - // don't save empty slice if data is nil or empty - db.data[height] = nil - } else { - db.data[height] = slices.Clone(data) - } - + db.data[height] = slices.Clone(data) return nil } -// Get retrieves data at the given height func (db *Database) Get(height uint64) ([]byte, error) { db.mu.RLock() defer db.mu.RUnlock() @@ -59,7 +51,6 @@ func (db *Database) Get(height uint64) ([]byte, error) { return slices.Clone(data), nil } -// Has checks if data exists at the given height func (db *Database) Has(height uint64) (bool, error) { db.mu.RLock() defer db.mu.RUnlock() @@ -72,7 +63,6 @@ func (db *Database) Has(height uint64) (bool, error) { return ok, nil } -// Close closes the in-memory database func (db *Database) Close() error { db.mu.Lock() defer db.mu.Unlock() From bc432c31ea29071e8b5144133f31cc1451773b40 Mon Sep 17 00:00:00 2001 From: Draco Date: Mon, 29 Sep 2025 16:30:29 -0400 Subject: [PATCH 16/18] refactor testCase struct --- database/heightindexdb/dbtest/dbtest.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index 3784f28027b3..f57bbb49110b 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -11,13 +11,11 @@ import ( "github.com/ava-labs/avalanchego/database" ) -type testCase struct { +// Tests is a list of all database tests +var Tests = []struct { Name string Test func(t *testing.T, newDB func() database.HeightIndex) -} - -// Tests is a list of all database tests -var Tests = []testCase{ +}{ {"TestPutGet", TestPutGet}, {"TestHas", TestHas}, {"TestCloseAndPut", TestCloseAndPut}, From bbeb41fadfc3da6abdc87cbec504b4d09d28c0aa Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 30 Sep 2025 12:29:15 -0400 Subject: [PATCH 17/18] Change the return value req for nil or empty value puts --- database/database.go | 4 ++-- database/heightindexdb/dbtest/dbtest.go | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/database/database.go b/database/database.go index 566a52bfd187..592f041bc6e0 100644 --- a/database/database.go +++ b/database/database.go @@ -99,8 +99,8 @@ type Database interface { type HeightIndex interface { // Put inserts the value into the database at the given height. // - // A nil or empty slice for [value] is accepted and will return their respective - // value if retrieved via Get. + // If [value] is nil or an empty slice, then when it's retrieved + // it may be nil or an empty slice. // // [value] is safe to read and modify after calling Put. Put(height uint64, value []byte) error diff --git a/database/heightindexdb/dbtest/dbtest.go b/database/heightindexdb/dbtest/dbtest.go index f57bbb49110b..ce59e75d1888 100644 --- a/database/heightindexdb/dbtest/dbtest.go +++ b/database/heightindexdb/dbtest/dbtest.go @@ -4,6 +4,7 @@ package dbtest import ( + "bytes" "testing" "github.com/stretchr/testify/require" @@ -68,7 +69,6 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, nil}, }, queryHeight: 1, - want: nil, }, { name: "put and get empty bytes", @@ -76,7 +76,6 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { {1, []byte{}}, }, queryHeight: 1, - want: []byte{}, }, { name: "put and get large data", @@ -109,14 +108,14 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) { // Query the specific height retrievedData, err := db.Get(tt.queryHeight) require.ErrorIs(t, err, tt.wantErr) - require.Equal(t, tt.want, retrievedData) + require.True(t, bytes.Equal(tt.want, retrievedData)) // modify the data returned from Get and ensure it won't change the // data from a second Get copy(retrievedData, []byte("modified data")) newData, err := db.Get(tt.queryHeight) require.ErrorIs(t, err, tt.wantErr) - require.Equal(t, tt.want, newData) + require.True(t, bytes.Equal(tt.want, newData)) }) } } From ac5ca8fc97f08d44eb5c7f448c1d9e57521a9885 Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 30 Sep 2025 13:11:31 -0400 Subject: [PATCH 18/18] update doc wording --- database/database.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/database/database.go b/database/database.go index 592f041bc6e0..cc21840d0bf2 100644 --- a/database/database.go +++ b/database/database.go @@ -99,14 +99,14 @@ type Database interface { type HeightIndex interface { // Put inserts the value into the database at the given height. // - // If [value] is nil or an empty slice, then when it's retrieved - // it may be nil or an empty slice. + // If value is nil or an empty slice, then when it's retrieved it may be nil + // or an empty slice. // - // [value] is safe to read and modify after calling Put. + // value is safe to read and modify after calling Put. Put(height uint64, value []byte) error // Get retrieves a value by its height. - // Returns ErrNotFound if the key is not present in the database. + // Returns [ErrNotFound] if the key is not present in the database. // // Returned []byte is safe to read and modify after calling Get. Get(height uint64) ([]byte, error) @@ -118,6 +118,6 @@ type HeightIndex interface { // Close closes the database. // - // Calling Close after Close returns ErrClosed. + // Calling Close after Close returns [ErrClosed]. io.Closer }