Clean up merkleDB interface and duplicate code

x/merkledb/db.go

@@ -179,7 +179,7 @@ type Config struct {
 	Tracer     trace.Tracer
 }
 
-// merkleDB can only be edited by committing changes from a trieView.
+// merkleDB can only be edited by committing changes from a view.
 type merkleDB struct {
 	// Must be held when reading/writing fields.
 	lock sync.RWMutex
@@ -216,7 +216,7 @@ type merkleDB struct {
 	rootID ids.ID
 
 	// Valid children of this trie.
-	childViews []*trieView
+	childViews []*view
 
 	// calculateNodeIDsSema controls the number of goroutines inside
 	// [calculateNodeIDsHelper] at any given time.
@@ -264,7 +264,7 @@ func newDatabase(
 		history:              newTrieHistory(int(config.HistoryLength)),
 		debugTracer:          getTracerIfEnabled(config.TraceLevel, DebugTrace, config.Tracer),
 		infoTracer:           getTracerIfEnabled(config.TraceLevel, InfoTrace, config.Tracer),
-		childViews:           make([]*trieView, 0, defaultPreallocationSize),
+		childViews:           make([]*view, 0, defaultPreallocationSize),
 		calculateNodeIDsSema: semaphore.NewWeighted(int64(rootGenConcurrency)),
 		tokenSize:            BranchFactorToTokenSize[config.BranchFactor],
 	}
@@ -462,13 +462,8 @@ func (db *merkleDB) PrefetchPaths(keys [][]byte) error {
 		return database.ErrClosed
 	}
 
-	// reuse the view so that it can keep repeated nodes in memory
-	tempView, err := newTrieView(db, db, ViewChanges{})
-	if err != nil {
-		return err
-	}
 	for _, key := range keys {
-		if err := db.prefetchPath(tempView, key); err != nil {
+		if err := db.prefetchPath(key); err != nil {
 			return err
 		}
 	}
@@ -483,16 +478,11 @@ func (db *merkleDB) PrefetchPath(key []byte) error {
 	if db.closed {
 		return database.ErrClosed
 	}
-	tempView, err := newTrieView(db, db, ViewChanges{})
-	if err != nil {
-		return err
-	}
-
-	return db.prefetchPath(tempView, key)
+	return db.prefetchPath(key)
 }
 
-func (db *merkleDB) prefetchPath(view *trieView, keyBytes []byte) error {
-	return view.visitPathToKey(ToKey(keyBytes), func(n *node) error {
+func (db *merkleDB) prefetchPath(keyBytes []byte) error {
+	return visitPathToKey(db, ToKey(keyBytes), func(n *node) error {
 		if !n.hasValue() {
 			// this value is already in the cache, so skip writing
 			// to avoid grabbing the cache write lock
@@ -607,22 +597,9 @@ func (db *merkleDB) getMerkleRoot() ids.ID {
 func (db *merkleDB) GetProof(ctx context.Context, key []byte) (*Proof, error) {
 	db.commitLock.RLock()
 	defer db.commitLock.RUnlock()
-
-	return db.getProof(ctx, key)
-}
-
-// Assumes [db.commitLock] is read locked.
-func (db *merkleDB) getProof(ctx context.Context, key []byte) (*Proof, error) {
-	if db.closed {
-		return nil, database.ErrClosed
-	}
-
-	view, err := newTrieView(db, db, ViewChanges{})
-	if err != nil {
-		return nil, err
-	}
-	// Don't need to lock [view] because nobody else has a reference to it.
-	return view.getProof(ctx, key)
+	_, span := db.infoTracer.Start(ctx, "MerkleDB.GetProof")
+	defer span.End()
+	return getProof(db, key)
 }
 
 func (db *merkleDB) GetRangeProof(
@@ -631,10 +608,9 @@ func (db *merkleDB) GetRangeProof(
 	end maybe.Maybe[[]byte],
 	maxLength int,
 ) (*RangeProof, error) {
-	db.commitLock.RLock()
-	defer db.commitLock.RUnlock()
-
-	return db.getRangeProofAtRoot(ctx, db.getMerkleRoot(), start, end, maxLength)
+	_, span := db.infoTracer.Start(ctx, "MerkleDB.GetRangeProof")
+	defer span.End()
+	return getRangeProof(db, start, end, maxLength)
 }
 
 func (db *merkleDB) GetRangeProofAtRoot(
@@ -647,18 +623,8 @@ func (db *merkleDB) GetRangeProofAtRoot(
 	db.commitLock.RLock()
 	defer db.commitLock.RUnlock()
 
-	return db.getRangeProofAtRoot(ctx, rootID, start, end, maxLength)
-}
-
-// Assumes [db.commitLock] is read locked.
-// Assumes [db.lock] is not held
-func (db *merkleDB) getRangeProofAtRoot(
-	ctx context.Context,
-	rootID ids.ID,
-	start maybe.Maybe[[]byte],
-	end maybe.Maybe[[]byte],
-	maxLength int,
-) (*RangeProof, error) {
+	_, span := db.infoTracer.Start(ctx, "MerkleDB.GetRangeProofAtRoot")
+	defer span.End()
 	switch {
 	case db.closed:
 		return nil, database.ErrClosed
@@ -668,11 +634,11 @@ func (db *merkleDB) getRangeProofAtRoot(
 		return nil, ErrEmptyProof
 	}
 
-	historicalView, err := db.getHistoricalViewForRange(rootID, start, end)
+	historicalTrie, err := db.getTrieAtRootForRange(rootID, start, end)
 	if err != nil {
 		return nil, err
 	}
-	return historicalView.GetRangeProof(ctx, start, end, maxLength)
+	return getRangeProof(historicalTrie, start, end, maxLength)
 }
 
 func (db *merkleDB) GetChangeProof(
@@ -683,6 +649,8 @@ func (db *merkleDB) GetChangeProof(
 	end maybe.Maybe[[]byte],
 	maxLength int,
 ) (*ChangeProof, error) {
+	_, span := db.infoTracer.Start(ctx, "MerkleDB.GetChangeProof")
+	defer span.End()
 	switch {
 	case start.HasValue() && end.HasValue() && bytes.Compare(start.Value(), end.Value()) == 1:
 		return nil, ErrStartAfterEnd
@@ -731,21 +699,21 @@ func (db *merkleDB) GetChangeProof(
 
 	// Since we hold [db.commitlock] we must still have sufficient
 	// history to recreate the trie at [endRootID].
-	historicalView, err := db.getHistoricalViewForRange(endRootID, start, largestKey)
+	historicalTrie, err := db.getTrieAtRootForRange(endRootID, start, largestKey)
 	if err != nil {
 		return nil, err
 	}
 
 	if largestKey.HasValue() {
-		endProof, err := historicalView.getProof(ctx, largestKey.Value())
+		endProof, err := getProof(historicalTrie, largestKey.Value())
 		if err != nil {
 			return nil, err
 		}
 		result.EndProof = endProof.Path
 	}
 
 	if start.HasValue() {
-		startProof, err := historicalView.getProof(ctx, start.Value())
+		startProof, err := getProof(historicalTrie, start.Value())
 		if err != nil {
 			return nil, err
 		}
@@ -782,7 +750,7 @@ func (db *merkleDB) GetChangeProof(
 func (db *merkleDB) NewView(
 	_ context.Context,
 	changes ViewChanges,
-) (TrieView, error) {
+) (View, error) {
 	// ensure the db doesn't change while creating the new view
 	db.commitLock.RLock()
 	defer db.commitLock.RUnlock()
@@ -916,7 +884,7 @@ func (db *merkleDB) commitBatch(ops []database.BatchOp) error {
 
 // commitChanges commits the changes in [trieToCommit] to [db].
 // Assumes [trieToCommit]'s node IDs have been calculated.
-func (db *merkleDB) commitChanges(ctx context.Context, trieToCommit *trieView) error {
+func (db *merkleDB) commitChanges(ctx context.Context, trieToCommit *view) error {
 	db.lock.Lock()
 	defer db.lock.Unlock()
 
@@ -1002,15 +970,15 @@ func (db *merkleDB) commitChanges(ctx context.Context, trieToCommit *trieView) e
 
 // moveChildViewsToDB removes any child views from the trieToCommit and moves them to the db
 // assumes [db.lock] is held
-func (db *merkleDB) moveChildViewsToDB(trieToCommit *trieView) {
+func (db *merkleDB) moveChildViewsToDB(trieToCommit *view) {
 	trieToCommit.validityTrackingLock.Lock()
 	defer trieToCommit.validityTrackingLock.Unlock()
 
 	for _, childView := range trieToCommit.childViews {
 		childView.updateParent(db)
 		db.childViews = append(db.childViews, childView)
 	}
-	trieToCommit.childViews = make([]*trieView, 0, defaultPreallocationSize)
+	trieToCommit.childViews = make([]*view, 0, defaultPreallocationSize)
 }
 
 // CommitToDB is a no-op for db since it is already in sync with itself.
@@ -1159,7 +1127,7 @@ func (db *merkleDB) VerifyChangeProof(
 
 // Invalidates and removes any child views that aren't [exception].
 // Assumes [db.lock] is held.
-func (db *merkleDB) invalidateChildrenExcept(exception *trieView) {
+func (db *merkleDB) invalidateChildrenExcept(exception *view) {
 	isTrackedView := false
 
 	for _, childView := range db.childViews {
@@ -1169,7 +1137,7 @@ func (db *merkleDB) invalidateChildrenExcept(exception *trieView) {
 			isTrackedView = true
 		}
 	}
-	db.childViews = make([]*trieView, 0, defaultPreallocationSize)
+	db.childViews = make([]*view, 0, defaultPreallocationSize)
 	if isTrackedView {
 		db.childViews = append(db.childViews, exception)
 	}
@@ -1217,24 +1185,22 @@ func (db *merkleDB) initializeRoot() error {
 // If [start] is Nothing, there's no lower bound on the range.
 // If [end] is Nothing, there's no upper bound on the range.
 // Assumes [db.commitLock] is read locked.
-func (db *merkleDB) getHistoricalViewForRange(
+// Assumes [db.lock] isn't held.
+func (db *merkleDB) getTrieAtRootForRange(
 	rootID ids.ID,
 	start maybe.Maybe[[]byte],
 	end maybe.Maybe[[]byte],
-) (*trieView, error) {
-	currentRootID := db.getMerkleRoot()
-
+) (Trie, error) {
 	// looking for the trie's current root id, so return the trie unmodified
-	if currentRootID == rootID {
-		// create an empty trie
-		return newTrieView(db, db, ViewChanges{})
+	if rootID == db.getMerkleRoot() {
+		return db, nil
 	}
 
 	changeHistory, err := db.history.getChangesToGetToRoot(rootID, start, end)
 	if err != nil {
 		return nil, err
 	}
-	return newHistoricalTrieView(db, changeHistory)
+	return newViewWithChanges(db, changeHistory)
 }
 
 // Returns all keys in range [start, end] that aren't in [keySet].
@@ -1327,6 +1293,10 @@ func (db *merkleDB) Clear() error {
 	return nil
 }
 
+func (db *merkleDB) getTokenSize() int {
+	return db.tokenSize
+}
+
 // Returns [key] prefixed by [prefix].
 // The returned []byte is taken from [bufferPool] and
 // should be returned to it when the caller is done with it.

x/merkledb/db_test.go

@@ -297,15 +297,15 @@ func Test_MerkleDB_Invalidate_Siblings_On_Commit(t *testing.T) {
 	sibling2, err := dbTrie.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
 
-	require.False(sibling1.(*trieView).isInvalid())
-	require.False(sibling2.(*trieView).isInvalid())
+	require.False(sibling1.(*view).isInvalid())
+	require.False(sibling2.(*view).isInvalid())
 
 	// Committing viewToCommit should invalidate siblings
 	require.NoError(viewToCommit.CommitToDB(context.Background()))
 
-	require.True(sibling1.(*trieView).isInvalid())
-	require.True(sibling2.(*trieView).isInvalid())
-	require.False(viewToCommit.(*trieView).isInvalid())
+	require.True(sibling1.(*view).isInvalid())
+	require.True(sibling2.(*view).isInvalid())
+	require.False(viewToCommit.(*view).isInvalid())
 }
 
 func Test_MerkleDB_CommitRangeProof_DeletesValuesInRange(t *testing.T) {
@@ -561,7 +561,7 @@ func TestDatabaseCommitChanges(t *testing.T) {
 	// Committing an invalid view should fail.
 	invalidView, err := db.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	invalidView.(*trieView).invalidate()
+	invalidView.(*view).invalidate()
 	err = invalidView.CommitToDB(context.Background())
 	require.ErrorIs(err, ErrInvalid)
 
@@ -582,22 +582,22 @@ func TestDatabaseCommitChanges(t *testing.T) {
 		},
 	)
 	require.NoError(err)
-	require.IsType(&trieView{}, view1Intf)
-	view1 := view1Intf.(*trieView)
+	require.IsType(&view{}, view1Intf)
+	view1 := view1Intf.(*view)
 	view1Root, err := view1.GetMerkleRoot(context.Background())
 	require.NoError(err)
 
 	// Make a second view
 	view2Intf, err := db.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	require.IsType(&trieView{}, view2Intf)
-	view2 := view2Intf.(*trieView)
+	require.IsType(&view{}, view2Intf)
+	view2 := view2Intf.(*view)
 
 	// Make a view atop a view
 	view3Intf, err := view1.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	require.IsType(&trieView{}, view3Intf)
-	view3 := view3Intf.(*trieView)
+	require.IsType(&view{}, view3Intf)
+	view3 := view3Intf.(*view)
 
 	// view3
 	//  |
@@ -646,18 +646,18 @@ func TestDatabaseInvalidateChildrenExcept(t *testing.T) {
 	// Create children
 	view1Intf, err := db.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	require.IsType(&trieView{}, view1Intf)
-	view1 := view1Intf.(*trieView)
+	require.IsType(&view{}, view1Intf)
+	view1 := view1Intf.(*view)
 
 	view2Intf, err := db.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	require.IsType(&trieView{}, view2Intf)
-	view2 := view2Intf.(*trieView)
+	require.IsType(&view{}, view2Intf)
+	view2 := view2Intf.(*view)
 
 	view3Intf, err := db.NewView(context.Background(), ViewChanges{})
 	require.NoError(err)
-	require.IsType(&trieView{}, view3Intf)
-	view3 := view3Intf.(*trieView)
+	require.IsType(&view{}, view3Intf)
+	view3 := view3Intf.(*view)
 
 	db.invalidateChildrenExcept(view1)
 
@@ -1255,7 +1255,7 @@ func TestGetRangeProofAtRootEmptyRootID(t *testing.T) {
 	db, err := getBasicDB()
 	require.NoError(err)
 
-	_, err = db.getRangeProofAtRoot(
+	_, err = db.GetRangeProofAtRoot(
 		context.Background(),
 		ids.Empty,
 		maybe.Nothing[[]byte](),