all: simplify and fix database iteration with prefix/start (#20808)

* core/state/snapshot: start fixing disk iterator seek

* ethdb, rawdb, leveldb, memorydb: implement iterators with prefix and start

* les, core/state/snapshot: iterator fixes

* all: remove two iterator methods

* all: rename Iteratee.NewIteratorWith -> NewIterator

* ethdb: fix review concerns

core/state/iterator_test.go

@@ -51,7 +51,7 @@ func TestNodeIteratorCoverage(t *testing.T) {
 			t.Errorf("state entry not reported %x", hash)
 		}
 	}
-	it := db.TrieDB().DiskDB().(ethdb.Database).NewIterator()
+	it := db.TrieDB().DiskDB().(ethdb.Database).NewIterator(nil, nil)
 	for it.Next() {
 		key := it.Key()
 		if bytes.HasPrefix(key, []byte("secure-key-")) {

core/state/snapshot/disklayer_test.go

@@ -18,11 +18,15 @@ package snapshot
 
 import (
 	"bytes"
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/VictoriaMetrics/fastcache"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/rawdb"
+	"github.com/ethereum/go-ethereum/ethdb"
+	"github.com/ethereum/go-ethereum/ethdb/leveldb"
 	"github.com/ethereum/go-ethereum/ethdb/memorydb"
 )
 
@@ -432,4 +436,76 @@ func TestDiskPartialMerge(t *testing.T) {
 // This test case is a tiny specialized case of TestDiskPartialMerge, which tests
 // some very specific cornercases that random tests won't ever trigger.
 func TestDiskMidAccountPartialMerge(t *testing.T) {
+	// TODO(@karalabe) ?
+}
+
+// TestDiskSeek tests that seek-operations work on the disk layer
+func TestDiskSeek(t *testing.T) {
+	// Create some accounts in the disk layer
+	var db ethdb.Database
+
+	if dir, err := ioutil.TempDir("", "disklayer-test"); err != nil {
+		t.Fatal(err)
+	} else {
+		defer os.RemoveAll(dir)
+		diskdb, err := leveldb.New(dir, 256, 0, "")
+		if err != nil {
+			t.Fatal(err)
+		}
+		db = rawdb.NewDatabase(diskdb)
+	}
+	// Fill even keys [0,2,4...]
+	for i := 0; i < 0xff; i += 2 {
+		acc := common.Hash{byte(i)}
+		rawdb.WriteAccountSnapshot(db, acc, acc[:])
+	}
+	// Add an 'higher' key, with incorrect (higher) prefix
+	highKey := []byte{rawdb.SnapshotAccountPrefix[0] + 1}
+	db.Put(highKey, []byte{0xff, 0xff})
+
+	baseRoot := randomHash()
+	rawdb.WriteSnapshotRoot(db, baseRoot)
+
+	snaps := &Tree{
+		layers: map[common.Hash]snapshot{
+			baseRoot: &diskLayer{
+				diskdb: db,
+				cache:  fastcache.New(500 * 1024),
+				root:   baseRoot,
+			},
+		},
+	}
+	// Test some different seek positions
+	type testcase struct {
+		pos    byte
+		expkey byte
+	}
+	var cases = []testcase{
+		{0xff, 0x55}, // this should exit immediately without checking key
+		{0x01, 0x02},
+		{0xfe, 0xfe},
+		{0xfd, 0xfe},
+		{0x00, 0x00},
+	}
+	for i, tc := range cases {
+		it, err := snaps.AccountIterator(baseRoot, common.Hash{tc.pos})
+		if err != nil {
+			t.Fatalf("case %d, error: %v", i, err)
+		}
+		count := 0
+		for it.Next() {
+			k, v, err := it.Hash()[0], it.Account()[0], it.Error()
+			if err != nil {
+				t.Fatalf("test %d, item %d, error: %v", i, count, err)
+			}
+			// First item in iterator should have the expected key
+			if count == 0 && k != tc.expkey {
+				t.Fatalf("test %d, item %d, got %v exp %v", i, count, k, tc.expkey)
+			}
+			count++
+			if v != k {
+				t.Fatalf("test %d, item %d, value wrong, got %v exp %v", i, count, v, k)
+			}
+		}
+	}
 }

core/state/snapshot/iterator.go

@@ -148,10 +148,10 @@ type diskAccountIterator struct {
 
 // AccountIterator creates an account iterator over a disk layer.
 func (dl *diskLayer) AccountIterator(seek common.Hash) AccountIterator {
-	// TODO: Fix seek position, or remove seek parameter
+	pos := common.TrimRightZeroes(seek[:])
 	return &diskAccountIterator{
 		layer: dl,
-		it:    dl.diskdb.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix),
+		it:    dl.diskdb.NewIterator(rawdb.SnapshotAccountPrefix, pos),
 	}
 }
 

core/state/snapshot/wipe.go

@@ -92,7 +92,7 @@ func wipeKeyRange(db ethdb.KeyValueStore, kind string, prefix []byte, keylen int
 	// Iterate over the key-range and delete all of them
 	start, logged := time.Now(), time.Now()
 
-	it := db.NewIteratorWithStart(prefix)
+	it := db.NewIterator(prefix, nil)
 	for it.Next() {
 		// Skip any keys with the correct prefix but wrong lenth (trie nodes)
 		key := it.Key()
@@ -113,7 +113,8 @@ func wipeKeyRange(db ethdb.KeyValueStore, kind string, prefix []byte, keylen int
 				return err
 			}
 			batch.Reset()
-			it = db.NewIteratorWithStart(key)
+			seekPos := key[len(prefix):]
+			it = db.NewIterator(prefix, seekPos)
 
 			if time.Since(logged) > 8*time.Second {
 				log.Info("Deleting state snapshot leftovers", "kind", kind, "wiped", items, "elapsed", common.PrettyDuration(time.Since(start)))

core/state/snapshot/wipe_test.go

@@ -60,7 +60,7 @@ func TestWipe(t *testing.T) {
 	// Sanity check that all the keys are present
 	var items int
 
-	it := db.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix)
+	it := db.NewIterator(rawdb.SnapshotAccountPrefix, nil)
 	defer it.Release()
 
 	for it.Next() {
@@ -69,7 +69,7 @@ func TestWipe(t *testing.T) {
 			items++
 		}
 	}
-	it = db.NewIteratorWithPrefix(rawdb.SnapshotStoragePrefix)
+	it = db.NewIterator(rawdb.SnapshotStoragePrefix, nil)
 	defer it.Release()
 
 	for it.Next() {
@@ -88,7 +88,7 @@ func TestWipe(t *testing.T) {
 	<-wipeSnapshot(db, true)
 
 	// Iterate over the database end ensure no snapshot information remains
-	it = db.NewIteratorWithPrefix(rawdb.SnapshotAccountPrefix)
+	it = db.NewIterator(rawdb.SnapshotAccountPrefix, nil)
 	defer it.Release()
 
 	for it.Next() {
@@ -97,7 +97,7 @@ func TestWipe(t *testing.T) {
 			t.Errorf("snapshot entry remained after wipe: %x", key)
 		}
 	}
-	it = db.NewIteratorWithPrefix(rawdb.SnapshotStoragePrefix)
+	it = db.NewIterator(rawdb.SnapshotStoragePrefix, nil)
 	defer it.Release()
 
 	for it.Next() {
@@ -112,7 +112,7 @@ func TestWipe(t *testing.T) {
 	// Iterate over the database and ensure miscellaneous items are present
 	items = 0
 
-	it = db.NewIterator()
+	it = db.NewIterator(nil, nil)
 	defer it.Release()
 
 	for it.Next() {

core/state/statedb_test.go

@@ -60,7 +60,7 @@ func TestUpdateLeaks(t *testing.T) {
 	}
 
 	// Ensure that no data was leaked into the database
-	it := db.NewIterator()
+	it := db.NewIterator(nil, nil)
 	for it.Next() {
 		t.Errorf("State leaked into database: %x -> %x", it.Key(), it.Value())
 	}
@@ -118,7 +118,7 @@ func TestIntermediateLeaks(t *testing.T) {
 		t.Errorf("can not commit trie %v to persistent database", finalRoot.Hex())
 	}
 
-	it := finalDb.NewIterator()
+	it := finalDb.NewIterator(nil, nil)
 	for it.Next() {
 		key, fvalue := it.Key(), it.Value()
 		tvalue, err := transDb.Get(key)
@@ -131,7 +131,7 @@ func TestIntermediateLeaks(t *testing.T) {
 	}
 	it.Release()
 
-	it = transDb.NewIterator()
+	it = transDb.NewIterator(nil, nil)
 	for it.Next() {
 		key, tvalue := it.Key(), it.Value()
 		fvalue, err := finalDb.Get(key)