refactor(heightindexdb): remove safety of byte slice modifications in Put and Get

Author: DracoLi

database/database.go

@@ -102,13 +102,13 @@ type HeightIndex interface {
 	// 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.
+	// The provided value is not safe to 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.
 	//
-	// Returned []byte is safe to read and modify after calling Get.
+	// The returned byte slice is not safe to modify.
 	Get(height uint64) ([]byte, error)
 
 	// Has checks if a value exists at the given height.

database/heightindexdb/dbtest/dbtest.go

@@ -99,23 +99,10 @@ func TestPutGet(t *testing.T, newDB func() database.HeightIndex) {
 				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)
 			require.ErrorIs(t, err, tt.wantErr)
 			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.True(t, bytes.Equal(tt.want, newData))
 		})
 	}
 }

database/heightindexdb/memdb/database.go

@@ -4,7 +4,6 @@
 package memdb
 
 import (
-	"slices"
 	"sync"
 
 	"github.com/ava-labs/avalanchego/database"
@@ -31,7 +30,7 @@ func (db *Database) Put(height uint64, data []byte) error {
 		db.data = make(map[uint64][]byte)
 	}
 
-	db.data[height] = slices.Clone(data)
+	db.data[height] = data
 	return nil
 }
 
@@ -48,7 +47,7 @@ func (db *Database) Get(height uint64) ([]byte, error) {
 		return nil, database.ErrNotFound
 	}
 
-	return slices.Clone(data), nil
+	return data, nil
 }
 
 func (db *Database) Has(height uint64) (bool, error) {

x/blockdb/cache_db.go

@@ -4,7 +4,6 @@
 package blockdb
 
 import (
-	"slices"
 	"sync"
 
 	"go.uber.org/zap"
@@ -46,13 +45,13 @@ func (c *cacheDB) Get(height BlockHeight) (BlockData, error) {
 	}
 
 	if cached, ok := c.cache.Get(height); ok {
-		return slices.Clone(cached), nil
+		return cached, nil
 	}
 	data, err := c.db.Get(height)
 	if err != nil {
 		return nil, err
 	}
-	c.cache.Put(height, slices.Clone(data))
+	c.cache.Put(height, data)
 	return data, nil
 }
 
@@ -69,7 +68,7 @@ func (c *cacheDB) Put(height BlockHeight, data BlockData) error {
 		return err
 	}
 
-	c.cache.Put(height, slices.Clone(data))
+	c.cache.Put(height, data)
 	return nil
 }
 

x/blockdb/cache_db_test.go

@@ -4,7 +4,6 @@
 package blockdb
 
 import (
-	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -57,44 +56,6 @@ func TestCacheHas(t *testing.T) {
 	require.True(t, has)
 }
 
-func TestCachePutStoresClone(t *testing.T) {
-	db := newCacheDatabase(t, DefaultConfig())
-	height := uint64(40)
-	block := randomBlock(t)
-	clone := slices.Clone(block)
-	require.NoError(t, db.Put(height, clone))
-
-	// Modify the original block after Put
-	clone[0] = 99
-
-	// Cache should have the original unmodified data
-	cached, ok := db.cache.Get(height)
-	require.True(t, ok)
-	require.Equal(t, block, cached)
-}
-
-func TestCacheGetReturnsClone(t *testing.T) {
-	db := newCacheDatabase(t, DefaultConfig())
-	height := uint64(50)
-	block := randomBlock(t)
-	require.NoError(t, db.Put(height, block))
-
-	// Get the block and modify the returned data
-	data, err := db.Get(height)
-	require.NoError(t, err)
-	data[0] = 99
-
-	// Cache should still have the original unmodified data
-	cached, ok := db.cache.Get(height)
-	require.True(t, ok)
-	require.Equal(t, block, cached)
-
-	// Second Get should also return original data
-	data, err = db.Get(height)
-	require.NoError(t, err)
-	require.Equal(t, block, data)
-}
-
 func TestCachePutOverridesSameHeight(t *testing.T) {
 	db := newCacheDatabase(t, DefaultConfig())
 	height := uint64(60)