MerkleDB:Naming and comments cleanup (#2274)

Co-authored-by: Dan Laine <[email protected]>

x/merkledb/cache.go

@@ -48,7 +48,7 @@ func (c *onEvictCache[K, V]) Get(key K) (V, bool) {
 
 // Put an element into this cache. If this causes an element
 // to be evicted, calls [c.onEviction] on the evicted element
-// and returns the error from [c.onEviction]. Otherwise returns nil.
+// and returns the error from [c.onEviction]. Otherwise, returns nil.
 func (c *onEvictCache[K, V]) Put(key K, value V) error {
 	c.lock.Lock()
 	defer c.lock.Unlock()

x/merkledb/codec.go

@@ -83,7 +83,7 @@ func newCodec() encoderDecoder {
 	}
 }
 
-// Note that bytes.Buffer.Write always returns nil so we
+// Note that bytes.Buffer.Write always returns nil, so we
 // can ignore its return values in [codecImpl] methods.
 type codecImpl struct {
 	// Invariant: Every byte slice returned by [varIntPool] has
@@ -277,12 +277,12 @@ func (c *codecImpl) decodeMaybeByteSlice(src *bytes.Reader) (maybe.Maybe[[]byte]
 		return maybe.Nothing[[]byte](), err
 	}
 
-	bytes, err := c.decodeByteSlice(src)
+	rawBytes, err := c.decodeByteSlice(src)
 	if err != nil {
 		return maybe.Nothing[[]byte](), err
 	}
 
-	return maybe.Some(bytes), nil
+	return maybe.Some(rawBytes), nil
 }
 
 func (c *codecImpl) decodeByteSlice(src *bytes.Reader) ([]byte, error) {

x/merkledb/db.go

@@ -73,7 +73,7 @@ type ChangeProofer interface {
 		maxLength int,
 	) (*ChangeProof, error)
 
-	// Returns nil iff all of the following hold:
+	// Returns nil iff all the following hold:
 	//   - [start] <= [end].
 	//   - [proof] is non-empty.
 	//   - All keys in [proof.KeyValues] and [proof.DeletedKeys] are in [start, end].
@@ -175,7 +175,7 @@ type merkleDB struct {
 	// Should be held before taking [db.lock]
 	commitLock sync.RWMutex
 
-	// Contains all of the key-value pairs stored by this database,
+	// Contains all the key-value pairs stored by this database,
 	// including metadata, intermediate nodes and value nodes.
 	baseDB database.Database
 
@@ -495,11 +495,11 @@ func (db *merkleDB) GetValues(ctx context.Context, keys [][]byte) ([][]byte, []e
 	defer db.lock.RUnlock()
 
 	values := make([][]byte, len(keys))
-	errors := make([]error, len(keys))
+	getErrors := make([]error, len(keys))
 	for i, key := range keys {
-		values[i], errors[i] = db.getValueCopy(ToKey(key))
+		values[i], getErrors[i] = db.getValueCopy(ToKey(key))
 	}
-	return values, errors
+	return values, getErrors
 }
 
 // GetValue returns the value associated with [key].
@@ -778,7 +778,7 @@ func (db *merkleDB) Has(k []byte) (bool, error) {
 	}
 
 	_, err := db.getValueWithoutLock(ToKey(k))
-	if err == database.ErrNotFound {
+	if errors.Is(err, database.ErrNotFound) {
 		return false, nil
 	}
 	return err == nil, err
@@ -862,7 +862,7 @@ func (db *merkleDB) DeleteContext(ctx context.Context, key []byte) error {
 	return view.commitToDB(ctx)
 }
 
-// Assumes values inside of [ops] are safe to reference after the function
+// Assumes values inside [ops] are safe to reference after the function
 // returns. Assumes [db.lock] isn't held.
 func (db *merkleDB) commitBatch(ops []database.BatchOp) error {
 	db.commitLock.Lock()
@@ -1144,15 +1144,15 @@ func (db *merkleDB) initializeRootIfNeeded() (ids.ID, error) {
 	// check under both prefixes
 	var err error
 	db.root, err = db.intermediateNodeDB.Get(Key{})
-	if err == database.ErrNotFound {
+	if errors.Is(err, database.ErrNotFound) {
 		db.root, err = db.valueNodeDB.Get(Key{})
 	}
 	if err == nil {
 		// Root already exists, so calculate its id
 		db.root.calculateID(db.metrics)
 		return db.root.id, nil
 	}
-	if err != database.ErrNotFound {
+	if !errors.Is(err, database.ErrNotFound) {
 		return ids.Empty, err
 	}
 

x/merkledb/history.go

@@ -49,7 +49,7 @@ type changeSummaryAndInsertNumber struct {
 	insertNumber uint64
 }
 
-// Tracks all of the node and value changes that resulted in the rootID.
+// Tracks all the node and value changes that resulted in the rootID.
 type changeSummary struct {
 	rootID ids.ID
 	nodes  map[Key]*change[*node]

x/merkledb/key.go

@@ -68,7 +68,7 @@ func ToToken(val byte, tokenSize int) Key {
 }
 
 // Token returns the token at the specified index,
-// Assumes that bitindex + tokenSize doesn't cross a byte boundary
+// Assumes that bitIndex + tokenSize doesn't cross a byte boundary
 func (k Key) Token(bitIndex int, tokenSize int) byte {
 	storageByte := k.value[bitIndex/8]
 	// Shift the byte right to get the last bit to the rightmost position.
@@ -145,7 +145,7 @@ func (k Key) HasPrefix(prefix Key) bool {
 	}
 
 	// Note that this will never be an index OOB because len(prefix.value) > 0.
-	// If len(prefix.value) == 0 were true, [remainderTokens] would be 0 so we
+	// If len(prefix.value) == 0 were true, [remainderTokens] would be 0, so we
 	// would have returned above.
 	prefixWithoutPartialByte := prefix.value[:len(prefix.value)-1]
 	return strings.HasPrefix(k.value, prefixWithoutPartialByte)
@@ -167,7 +167,7 @@ func (k Key) Greater(other Key) bool {
 	return k.value > other.value || (k.value == other.value && k.length > other.length)
 }
 
-// Less returns true if current Key is less than other Key
+// Less will return true if current Key is less than other Key
 func (k Key) Less(other Key) bool {
 	return k.value < other.value || (k.value == other.value && k.length < other.length)
 }

x/merkledb/proof.go

@@ -50,7 +50,6 @@ var (
 	ErrNilProof                    = errors.New("proof is nil")
 	ErrNilValue                    = errors.New("value is nil")
 	ErrUnexpectedEndProof          = errors.New("end proof should be empty")
-	ErrInconsistentBranchFactor    = errors.New("all keys in proof nodes should have the same branch factor")
 )
 
 type ProofNode struct {
@@ -62,6 +61,7 @@ type ProofNode struct {
 	Children    map[byte]ids.ID
 }
 
+// ToProto converts the ProofNode into the protobuf version of a proof node
 // Assumes [node.Key.Key.length] <= math.MaxUint64.
 func (node *ProofNode) ToProto() *pb.ProofNode {
 	pbNode := &pb.ProofNode{
@@ -127,11 +127,11 @@ type Proof struct {
 	Key Key
 
 	// Nothing if [Key] isn't in the trie.
-	// Otherwise the value corresponding to [Key].
+	// Otherwise, the value corresponding to [Key].
 	Value maybe.Maybe[[]byte]
 }
 
-// Returns nil if the trie given in [proof] has root [expectedRootID].
+// Verify returns nil if the trie given in [proof] has root [expectedRootID].
 // That is, this is a valid proof that [proof.Key] exists/doesn't exist
 // in the trie with root [expectedRootID].
 func (proof *Proof) Verify(ctx context.Context, expectedRootID ids.ID, tokenSize int) error {
@@ -172,11 +172,11 @@ func (proof *Proof) Verify(ctx context.Context, expectedRootID ids.ID, tokenSize
 	}
 
 	// Insert all proof nodes.
-	// [provenPath] is the path that we are proving exists, or the path
-	// that is where the path we are proving doesn't exist should be.
-	provenPath := maybe.Some(proof.Path[len(proof.Path)-1].Key)
+	// [provenKey] is the key that we are proving exists, or the key
+	// that is the next key along the node path, proving that [proof.Key] doesn't exist in the trie.
+	provenKey := maybe.Some(proof.Path[len(proof.Path)-1].Key)
 
-	if err = addPathInfo(view, proof.Path, provenPath, provenPath); err != nil {
+	if err = addPathInfo(view, proof.Path, provenKey, provenKey); err != nil {
 		return err
 	}
 
@@ -240,7 +240,7 @@ type KeyValue struct {
 	Value []byte
 }
 
-// A proof that a given set of key-value pairs are in a trie.
+// RangeProof is a proof that a given set of key-value pairs are in a trie.
 type RangeProof struct {
 	// Invariant: At least one of [StartProof], [EndProof], [KeyValues] is non-empty.
 
@@ -302,21 +302,21 @@ func (proof *RangeProof) Verify(
 	}
 
 	// [proof] allegedly provides and proves all key-value
-	// pairs in [smallestProvenPath, largestProvenPath].
-	// If [smallestProvenPath] is Nothing, [proof] should
-	// provide and prove all keys < [largestProvenPath].
-	// If [largestProvenPath] is Nothing, [proof] should
-	// provide and prove all keys > [smallestProvenPath].
+	// pairs in [smallestProvenKey, largestProvenKey].
+	// If [smallestProvenKey] is Nothing, [proof] should
+	// provide and prove all keys < [largestProvenKey].
+	// If [largestProvenKey] is Nothing, [proof] should
+	// provide and prove all keys > [smallestProvenKey].
 	// If both are Nothing, [proof] should prove the entire trie.
-	smallestProvenPath := maybe.Bind(start, ToKey)
+	smallestProvenKey := maybe.Bind(start, ToKey)
 
-	largestProvenPath := maybe.Bind(end, ToKey)
+	largestProvenKey := maybe.Bind(end, ToKey)
 
 	if len(proof.KeyValues) > 0 {
 		// If [proof] has key-value pairs, we should insert children
-		// greater than [largestProvenPath] to ancestors of the node containing
-		// [largestProvenPath] so that we get the expected root ID.
-		largestProvenPath = maybe.Some(ToKey(proof.KeyValues[len(proof.KeyValues)-1].Key))
+		// greater than [largestProvenKey] to ancestors of the node containing
+		// [largestProvenKey] so that we get the expected root ID.
+		largestProvenKey = maybe.Some(ToKey(proof.KeyValues[len(proof.KeyValues)-1].Key))
 	}
 
 	// The key-value pairs (allegedly) proven by [proof].
@@ -327,27 +327,27 @@ func (proof *RangeProof) Verify(
 
 	// Ensure that the start proof is valid and contains values that
 	// match the key/values that were sent.
-	if err := verifyProofPath(proof.StartProof, smallestProvenPath); err != nil {
+	if err := verifyProofPath(proof.StartProof, smallestProvenKey); err != nil {
 		return err
 	}
 	if err := verifyAllRangeProofKeyValuesPresent(
 		proof.StartProof,
-		smallestProvenPath,
-		largestProvenPath,
+		smallestProvenKey,
+		largestProvenKey,
 		keyValues,
 	); err != nil {
 		return err
 	}
 
 	// Ensure that the end proof is valid and contains values that
 	// match the key/values that were sent.
-	if err := verifyProofPath(proof.EndProof, largestProvenPath); err != nil {
+	if err := verifyProofPath(proof.EndProof, largestProvenKey); err != nil {
 		return err
 	}
 	if err := verifyAllRangeProofKeyValuesPresent(
 		proof.EndProof,
-		smallestProvenPath,
-		largestProvenPath,
+		smallestProvenKey,
+		largestProvenKey,
 		keyValues,
 	); err != nil {
 		return err
@@ -369,24 +369,24 @@ func (proof *RangeProof) Verify(
 	}
 
 	// For all the nodes along the edges of the proof, insert children
-	// < [smallestProvenPath] and > [largestProvenPath]
+	// < [smallestProvenKey] and > [largestProvenKey]
 	// into the trie so that we get the expected root ID (if this proof is valid).
-	// By inserting all children < [smallestProvenPath], we prove that there are no keys
-	// > [smallestProvenPath] but less than the first key given.
+	// By inserting all children < [smallestProvenKey], we prove that there are no keys
+	// > [smallestProvenKey] but less than the first key given.
 	// That is, the peer who gave us this proof is not omitting nodes.
 	if err := addPathInfo(
 		view,
 		proof.StartProof,
-		smallestProvenPath,
-		largestProvenPath,
+		smallestProvenKey,
+		largestProvenKey,
 	); err != nil {
 		return err
 	}
 	if err := addPathInfo(
 		view,
 		proof.EndProof,
-		smallestProvenPath,
-		largestProvenPath,
+		smallestProvenKey,
+		largestProvenKey,
 	); err != nil {
 		return err
 	}
@@ -462,13 +462,13 @@ func (proof *RangeProof) UnmarshalProto(pbProof *pb.RangeProof) error {
 func verifyAllRangeProofKeyValuesPresent(proof []ProofNode, start maybe.Maybe[Key], end maybe.Maybe[Key], keysValues map[Key][]byte) error {
 	for i := 0; i < len(proof); i++ {
 		var (
-			node     = proof[i]
-			nodePath = node.Key
+			node    = proof[i]
+			nodeKey = node.Key
 		)
 
 		// Skip keys that cannot have a value (enforced by [verifyProofPath]).
-		if !nodePath.hasPartialByte() && (start.IsNothing() || !nodePath.Less(start.Value())) && (end.IsNothing() || !nodePath.Greater(end.Value())) {
-			value, ok := keysValues[nodePath]
+		if !nodeKey.hasPartialByte() && (start.IsNothing() || !nodeKey.Less(start.Value())) && (end.IsNothing() || !nodeKey.Greater(end.Value())) {
+			value, ok := keysValues[nodeKey]
 			if !ok && node.ValueOrHash.HasValue() {
 				// We didn't get a key-value pair for this key, but the proof node has a value.
 				return ErrProofNodeHasUnincludedValue
@@ -488,7 +488,7 @@ type KeyChange struct {
 	Value maybe.Maybe[[]byte]
 }
 
-// A change proof proves that a set of key-value changes occurred
+// ChangeProof proves that a set of key-value changes occurred
 // between two trie roots, where each key-value pair's key is
 // between some lower and upper bound (inclusive).
 type ChangeProof struct {
@@ -622,8 +622,8 @@ func (proof *ChangeProof) UnmarshalProto(pbProof *pb.ChangeProof) error {
 }
 
 // Verifies that all values present in the [proof]:
-// - Are nothing when deleted, not in the db, or the node has path partial byte length
-// - if the node's path is within the key range, that has a value that matches the value passed in the change list or in the db
+// - Are nothing when deleted, not in the db, or the node has key partial byte length
+// - if the node's key is within the key range, that has a value that matches the value passed in the change list or in the db
 func verifyAllChangeProofKeyValuesPresent(
 	ctx context.Context,
 	db MerkleDB,
@@ -634,19 +634,19 @@ func verifyAllChangeProofKeyValuesPresent(
 ) error {
 	for i := 0; i < len(proof); i++ {
 		var (
-			node     = proof[i]
-			nodePath = node.Key
+			node    = proof[i]
+			nodeKey = node.Key
 		)
 
 		// Check the value of any node with a key that is within the range.
 		// Skip keys that cannot have a value (enforced by [verifyProofPath]).
-		if !nodePath.hasPartialByte() && (start.IsNothing() || !nodePath.Less(start.Value())) && (end.IsNothing() || !nodePath.Greater(end.Value())) {
-			value, ok := keysValues[nodePath]
+		if !nodeKey.hasPartialByte() && (start.IsNothing() || !nodeKey.Less(start.Value())) && (end.IsNothing() || !nodeKey.Greater(end.Value())) {
+			value, ok := keysValues[nodeKey]
 			if !ok {
 				// This value isn't in the list of key-value pairs we got.
-				dbValue, err := db.GetValue(ctx, nodePath.Bytes())
+				dbValue, err := db.GetValue(ctx, nodeKey.Bytes())
 				if err != nil {
-					if err != database.ErrNotFound {
+					if !errors.Is(err, database.ErrNotFound) {
 						return err
 					}
 					// This key isn't in the database so proof node should have Nothing.
@@ -669,7 +669,7 @@ func (proof *ChangeProof) Empty() bool {
 		len(proof.StartProof) == 0 && len(proof.EndProof) == 0
 }
 
-// Exactly one of [ChangeProof] or [RangeProof] is non-nil.
+// ChangeOrRangeProof has exactly one of [ChangeProof] or [RangeProof] is non-nil.
 type ChangeOrRangeProof struct {
 	ChangeProof *ChangeProof
 	RangeProof  *RangeProof