From 9d890b22821dc644c3a32616adf96ba6e8f2c5e3 Mon Sep 17 00:00:00 2001
From: Roy Crihfield <roy@manteia.ltd>
Date: Tue, 29 Aug 2023 17:00:18 +0800
Subject: [PATCH] doc comments

---
 iterator.go        | 10 +++++++---
 iterator_test.go   |  9 +++++----
 tracker/tracker.go | 11 ++++++-----
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/iterator.go b/iterator.go
index e29509a..31cde21 100644
--- a/iterator.go
+++ b/iterator.go
@@ -40,9 +40,13 @@ func (it *PrefixBoundIterator) Next(descend bool) bool {
 	if !it.NodeIterator.Next(descend) {
 		return false
 	}
-	// stop if underlying iterator went past upper bound
-	cmp := bytes.Compare(it.Path(), it.EndPath)
-	return cmp <= 0
+	// Stop if underlying iterator went past upper bound.
+	// Note: this results in a single node of overlap between binned iterators. The more correct
+	// behavior would be to make this a strict less-than, so that iterators cover mutually disjoint
+	// subtries. Unfortunately, the NodeIterator constructor takes a compact path, meaning
+	// odd-length paths must be padded with a 0, so e.g. [8] becomes [8, 0], which means we would
+	// skip [8]. So, we use <= here to cover that node for the "next" bin.
+	return bytes.Compare(it.Path(), it.EndPath) <= 0
 }
 
 // NewPrefixBoundIterator returns an iterator with an upper bound value (hex path prefix)
diff --git a/iterator_test.go b/iterator_test.go
index 4547a33..1c0de58 100644
--- a/iterator_test.go
+++ b/iterator_test.go
@@ -87,12 +87,13 @@ func TestIterator(t *testing.T) {
 			for b := uint(0); b < nbins; b++ {
 				for it := iters[b]; it.Next(true); ix++ {
 					if !bytes.Equal(allPaths[ix], it.Path()) {
-						t.Fatalf("wrong path value\nexpected:\t%v\nactual:\t\t%v",
-							allPaths[ix], it.Path())
+						t.Fatalf("wrong path value in bin %d (index %d)\nexpected:\t%v\nactual:\t\t%v",
+							b, ix, allPaths[ix], it.Path())
 					}
 				}
-				// if the last node path was even-length, it will be duplicated
-				if len(allPaths[ix-1])&0b1 == 0 {
+				// if the last node path for the previous bin was even-length, the next iterator
+				// will seek to the same node and it will be duplicated (see comment in Next()).
+				if len(allPaths[ix-1])&1 == 0 {
 					ix--
 				}
 			}
diff --git a/tracker/tracker.go b/tracker/tracker.go
index 8fedf72..593bc78 100644
--- a/tracker/tracker.go
+++ b/tracker/tracker.go
@@ -46,8 +46,8 @@ func (tr *Tracker) CaptureSignal(cancelCtx context.CancelFunc) {
 	go func() {
 		sig := <-sigChan
 		log.Error("Signal received (%v), stopping", "signal", sig)
-		// cancel context on receiving a signal
-		// on ctx cancellation, all the iterators complete processing of their current node before stopping
+		// Cancel context on receiving a signal. On cancellation, all tracked iterators complete
+		// processing of their current node before stopping.
 		cancelCtx()
 	}()
 }
@@ -90,8 +90,9 @@ func (tr *Tracker) dump() error {
 	return out.WriteAll(rows)
 }
 
-// Restore attempts to read iterator state from file
-// if file doesn't exist, returns an empty slice with no error
+// Restore attempts to read iterator state from the recovery file.
+// If the file doesn't exist, returns an empty slice with no error.
+// Restored iterators are constructed in the same order as in the returned slice.
 func (tr *Tracker) Restore(makeIterator iter.IteratorConstructor) ([]trie.NodeIterator, error) {
 	file, err := os.Open(tr.recoveryFile)
 	if err != nil {
@@ -185,7 +186,7 @@ func (it *Iterator) Next(descend bool) bool {
 }
 
 // Subtracts 1 from the last byte in a path slice, carrying if needed.
-// Does nothing, returning false, for all-zero inputs.
+// Does nothing, returning false, for all-zero inputs (underflow).
 func decrementPath(path []byte) bool {
 	// check for all zeros
 	allzero := true