Merkle db cleanup view creation

x/merkledb/db.go

@@ -277,7 +277,7 @@ func (db *merkleDB) rebuild(ctx context.Context) error {
 
 	for it.Next() {
 		if len(currentOps) >= viewSizeLimit {
-			view, err := db.newUntrackedView(currentOps, true)
+			view, err := newTrieView(db, db, ViewChanges{BatchOps: currentOps, ConsumeBytes: true})
 			if err != nil {
 				return err
 			}
@@ -304,7 +304,7 @@ func (db *merkleDB) rebuild(ctx context.Context) error {
 	if err := it.Error(); err != nil {
 		return err
 	}
-	view, err := db.newUntrackedView(currentOps, true)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: currentOps, ConsumeBytes: true})
 	if err != nil {
 		return err
 	}
@@ -330,7 +330,7 @@ func (db *merkleDB) CommitChangeProof(ctx context.Context, proof *ChangeProof) e
 		}
 	}
 
-	view, err := db.newUntrackedView(ops, false)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: ops})
 	if err != nil {
 		return err
 	}
@@ -371,7 +371,7 @@ func (db *merkleDB) CommitRangeProof(ctx context.Context, start maybe.Maybe[[]by
 	}
 
 	// Don't need to lock [view] because nobody else has a reference to it.
-	view, err := db.newUntrackedView(ops, false)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: ops})
 	if err != nil {
 		return err
 	}
@@ -526,7 +526,7 @@ func (db *merkleDB) getProof(ctx context.Context, key []byte) (*Proof, error) {
 		return nil, database.ErrClosed
 	}
 
-	view, err := db.newUntrackedView(nil, true)
+	view, err := newTrieView(db, db, ViewChanges{})
 	if err != nil {
 		return nil, err
 	}
@@ -696,7 +696,7 @@ func (db *merkleDB) NewView(
 		return nil, database.ErrClosed
 	}
 
-	newView, err := newTrieView(db, db, db.root.clone(), changes)
+	newView, err := newTrieView(db, db, changes)
 	if err != nil {
 		return nil, err
 	}
@@ -709,21 +709,6 @@ func (db *merkleDB) NewView(
 	return newView, nil
 }
 
-// Returns a new view that isn't tracked in [db.childViews].
-// For internal use only, namely in methods that create short-lived views.
-// Assumes [db.lock] isn't held and [db.commitLock] is read locked.
-func (db *merkleDB) newUntrackedView(batchOps []database.BatchOp, consumeBytes bool) (*trieView, error) {
-	return newTrieView(
-		db,
-		db,
-		db.root.clone(),
-		ViewChanges{
-			BatchOps:     batchOps,
-			ConsumeBytes: consumeBytes,
-		},
-	)
-}
-
 func (db *merkleDB) Has(k []byte) (bool, error) {
 	db.lock.RLock()
 	defer db.lock.RUnlock()
@@ -853,13 +838,7 @@ func (db *merkleDB) PutContext(ctx context.Context, k, v []byte) error {
 		return database.ErrClosed
 	}
 
-	view, err := db.newUntrackedView(
-		[]database.BatchOp{{
-			Key:   k,
-			Value: v,
-		}},
-		false,
-	)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: []database.BatchOp{{Key: k, Value: v}}})
 	if err != nil {
 		return err
 	}
@@ -878,13 +857,14 @@ func (db *merkleDB) DeleteContext(ctx context.Context, key []byte) error {
 		return database.ErrClosed
 	}
 
-	view, err := db.newUntrackedView(
-		[]database.BatchOp{{
-			Key:    key,
-			Delete: true,
-		}},
-		true,
-	)
+	view, err := newTrieView(db, db,
+		ViewChanges{
+			BatchOps: []database.BatchOp{{
+				Key:    key,
+				Delete: true,
+			}},
+			ConsumeBytes: true,
+		})
 	if err != nil {
 		return err
 	}
@@ -901,7 +881,7 @@ func (db *merkleDB) commitBatch(ops []database.BatchOp) error {
 		return database.ErrClosed
 	}
 
-	view, err := db.newUntrackedView(ops, true)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: ops, ConsumeBytes: true})
 	if err != nil {
 		return err
 	}
@@ -1116,7 +1096,7 @@ func (db *merkleDB) VerifyChangeProof(
 	}
 
 	// Don't need to lock [view] because nobody else has a reference to it.
-	view, err := db.newUntrackedView(ops, true)
+	view, err := newTrieView(db, db, ViewChanges{BatchOps: ops, ConsumeBytes: true})
 	if err != nil {
 		return err
 	}
@@ -1226,7 +1206,7 @@ func (db *merkleDB) getHistoricalViewForRange(
 	// looking for the trie's current root id, so return the trie unmodified
 	if currentRootID == rootID {
 		// create an empty trie
-		return newTrieView(db, db, db.root.clone(), ViewChanges{})
+		return newTrieView(db, db, ViewChanges{})
 	}
 
 	changeHistory, err := db.history.getChangesToGetToRoot(rootID, start, end)

x/merkledb/db_test.go

@@ -458,11 +458,14 @@ func TestDatabaseNewUntrackedView(t *testing.T) {
 	require.NoError(err)
 
 	// Create a new untracked view.
-	view, err := db.newUntrackedView(
-		[]database.BatchOp{
-			{Key: []byte{1}, Value: []byte{1}},
+	view, err := newTrieView(
+		db,
+		db,
+		ViewChanges{
+			BatchOps: []database.BatchOp{
+				{Key: []byte{1}, Value: []byte{1}},
+			},
 		},
-		false,
 	)
 	require.NoError(err)
 	require.Empty(db.childViews)

x/merkledb/proof.go

@@ -878,5 +878,5 @@ func getStandaloneTrieView(ctx context.Context, ops []database.BatchOp) (*trieVi
 		return nil, err
 	}
 
-	return db.newUntrackedView(ops, true)
+	return newTrieView(db, db, ViewChanges{BatchOps: ops, ConsumeBytes: true})
 }

x/merkledb/trie_test.go

@@ -10,8 +10,6 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/ava-labs/avalanchego/utils/maybe"
-
 	"github.com/stretchr/testify/require"
 
 	"github.com/ava-labs/avalanchego/database"
@@ -565,19 +563,28 @@ func Test_Trie_HashCountOnBranch(t *testing.T) {
 	dbTrie, err := getBasicDB()
 	require.NoError(err)
 	require.NotNil(dbTrie)
-	trieIntf, err := dbTrie.NewView(context.Background(), ViewChanges{})
+	trieIntf, err := dbTrie.NewView(
+		context.Background(),
+		ViewChanges{
+			BatchOps: []database.BatchOp{
+				{Key: []byte("key12"), Value: []byte("value12")},
+			},
+		})
 	require.NoError(err)
 	trie := trieIntf.(*trieView)
 
-	// force a new node to generate with common prefix "key1" and have these two nodes as children
-	_, err = trie.insert(newPath([]byte("key12")), maybe.Some([]byte("value12")))
-	require.NoError(err)
-	_, err = trie.insert(newPath([]byte("key134")), maybe.Some([]byte("value134")))
+	// force a new branch node to generate with common prefix "key1" and have this node and the "key12" node as children
+	trieIntf, err = trie.NewView(context.Background(), ViewChanges{BatchOps: []database.BatchOp{
+		{Key: []byte("key134"), Value: []byte("value134")},
+	}})
 	require.NoError(err)
+	trie = trieIntf.(*trieView)
+	dbTrie.metrics.(*mockMetrics).hashCount = 0
+	require.NoError(trie.calculateNodeIDs(context.Background()))
+
 	// only hashes the new branch node, the new child node, and root
 	// shouldn't hash the existing node
-	require.NoError(trie.calculateNodeIDs(context.Background()))
-	require.Equal(int64(5), dbTrie.metrics.(*mockMetrics).hashCount)
+	require.Equal(int64(3), dbTrie.metrics.(*mockMetrics).hashCount)
 }
 
 func Test_Trie_HashCountOnDelete(t *testing.T) {
@@ -945,7 +952,7 @@ func TestNewViewOnCommittedView(t *testing.T) {
 	require.NoError(err)
 
 	// Create a view
-	view1Intf, err := db.NewView(context.Background(), ViewChanges{})
+	view1Intf, err := db.NewView(context.Background(), ViewChanges{BatchOps: []database.BatchOp{{Key: []byte{1}, Value: []byte{1}}}})
 	require.NoError(err)
 	require.IsType(&trieView{}, view1Intf)
 	view1 := view1Intf.(*trieView)
@@ -958,9 +965,6 @@ func TestNewViewOnCommittedView(t *testing.T) {
 	require.Contains(db.childViews, view1)
 	require.Equal(db, view1.parentTrie)
 
-	_, err = view1.insert(newPath([]byte{1}), maybe.Some([]byte{1}))
-	require.NoError(err)
-
 	// Commit the view
 	require.NoError(view1.CommitToDB(context.Background()))
 

x/merkledb/trieview.go

@@ -125,7 +125,7 @@ func (t *trieView) NewView(
 		return nil, err
 	}
 
-	newView, err := newTrieView(t.db, t, t.root.clone(), changes)
+	newView, err := newTrieView(t.db, t, changes)
 	if err != nil {
 		return nil, err
 	}
@@ -145,11 +145,14 @@ func (t *trieView) NewView(
 func newTrieView(
 	db *merkleDB,
 	parentTrie TrieView,
-	root *node,
 	changes ViewChanges,
 ) (*trieView, error) {
-	if root == nil {
-		return nil, ErrNoValidRoot
+	root, err := parentTrie.getEditableNode(RootPath)
+	if err != nil {
+		if err == database.ErrNotFound {
+			return nil, ErrNoValidRoot
+		}
+		return nil, err
 	}
 
 	newView := &trieView{