Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MerkleDB:Naming and comments cleanup #2274

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x/merkledb/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions x/merkledb/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions x/merkledb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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].
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion x/merkledb/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 3 additions & 3 deletions x/merkledb/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
92 changes: 46 additions & 46 deletions x/merkledb/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{
Expand Down Expand Up @@ -126,12 +126,12 @@ type Proof struct {
// This is a proof that [key] exists/doesn't exist.
Key Key

// Nothing if [Key] isn't in the trie.
// Otherwise the value corresponding to [Key].
// Nothing if [Key] isn't in the trie
// otherwise, the value corresponding to [Key].
dboehm-avalabs marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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].
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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
Expand Down
Loading
Loading