From 795cfed5c091096fb914efea8a940c8bcc6042c9 Mon Sep 17 00:00:00 2001
From: Guillaume Ballet <gballet@gmail.com>
Date: Tue, 9 Jul 2019 16:33:46 +0200
Subject: [PATCH] eth: Include Felix' feedback

---
 eth/api.go      | 16 +++++++++++-----
 eth/api_test.go | 37 +++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/eth/api.go b/eth/api.go
index 70cd29fefa..f5bd11aeed 100644
--- a/eth/api.go
+++ b/eth/api.go
@@ -335,14 +335,18 @@ func (api *PrivateDebugAPI) GetBadBlocks(ctx context.Context) ([]*BadBlockArgs,
 	return results, nil
 }
 
+// AccountRangeResult returns a mapping from the hash of an account addresses
+// to its preimage. It will return the JSON null if no preimage is found.
+// Since a query can return a limited amount of results, a "next" field is
+// also present for paging.
 type AccountRangeResult struct {
-	Addresses []common.Address `json:"addresses"`
-	Next      common.Hash      `json:"next"`
+	Accounts map[common.Hash]*common.Address `json:"accounts"`
+	Next     common.Hash                     `json:"next"`
 }
 
 func accountRange(st state.Trie, start *common.Hash, maxResults int) (AccountRangeResult, error) {
 	it := trie.NewIterator(st.NodeIterator(start.Bytes()))
-	result := AccountRangeResult{Addresses: []common.Address{}, Next: common.Hash{}}
+	result := AccountRangeResult{Accounts: make(map[common.Hash]*common.Address), Next: common.Hash{}}
 
 	if maxResults > AccountRangeMaxResults {
 		maxResults = AccountRangeMaxResults
@@ -350,9 +354,11 @@ func accountRange(st state.Trie, start *common.Hash, maxResults int) (AccountRan
 
 	for i := 0; i < maxResults && it.Next(); i++ {
 		if preimage := st.GetKey(it.Key); preimage != nil {
-			result.Addresses = append(result.Addresses, common.BytesToAddress(preimage))
+			addr := &common.Address{}
+			addr.SetBytes(preimage)
+			result.Accounts[common.BytesToHash(it.Key)] = addr
 		} else {
-			return AccountRangeResult{}, fmt.Errorf("preimage not found for 0x%s", hex.EncodeToString(it.Key))
+			result.Accounts[common.BytesToHash(it.Key)] = nil
 		}
 	}
 
diff --git a/eth/api_test.go b/eth/api_test.go
index 6adc1c2915..19fcd9649d 100644
--- a/eth/api_test.go
+++ b/eth/api_test.go
@@ -38,13 +38,16 @@ func accountRangeTest(t *testing.T, trie *state.Trie, statedb *state.StateDB, st
 		t.Fatal(err)
 	}
 
-	if len(result.Addresses) != expectedNum {
-		t.Fatalf("expected %d results.  Got %d", expectedNum, len(result.Addresses))
+	if len(result.Accounts) != expectedNum {
+		t.Fatalf("expected %d results.  Got %d", expectedNum, len(result.Accounts))
 	}
 
-	for i := range result.Addresses {
-		if !statedb.Exist(result.Addresses[i]) {
-			t.Fatalf("account not found in state %s", result.Addresses[i].String())
+	for _, address := range result.Accounts {
+		if address == nil {
+			t.Fatalf("null address returned")
+		}
+		if !statedb.Exist(*address) {
+			t.Fatalf("account not found in state %s", address.Hex())
 		}
 	}
 
@@ -93,11 +96,25 @@ func TestAccountRange(t *testing.T) {
 	t.Logf("test pagination 2")
 	secondResult := accountRangeTest(t, &trie, state, &firstResult.Next, AccountRangeMaxResults, AccountRangeMaxResults)
 
-	for i := range firstResult.Addresses {
-		for j := range secondResult.Addresses {
-			if bytes.Equal(firstResult.Addresses[i].Bytes(), secondResult.Addresses[j].Bytes()) {
+	for h1, addr1 := range firstResult.Accounts {
+		for h2, addr2 := range secondResult.Accounts {
+			// Make sure that the hashes aren't the same
+			if bytes.Equal(h1.Bytes(), h2.Bytes()) {
 				t.Fatalf("pagination test failed:  results should not overlap")
 			}
+
+			// If either address is nil, then it makes no sense to compare
+			// them as they might be two different accounts.
+			if addr1 == nil || addr2 == nil {
+				continue
+			}
+
+			// Since the two hashes are different, they should not have
+			// the same preimage, but let's check anyway in case there
+			// is a bug in the (hash, addr) map generation code.
+			if bytes.Equal(addr1.Bytes(), addr2.Bytes()) {
+				t.Fatalf("pagination test failed: addresses should not repeat")
+			}
 		}
 	}
 }
@@ -123,8 +140,8 @@ func TestEmptyAccountRange(t *testing.T) {
 	if results.Next != common.HexToHash("0") {
 		t.Fatalf("Empty results should not return a second page")
 	}
-	if len(results.Addresses) != 0 {
-		t.Fatalf("Empty state should not return addresses: %v", results.Addresses)
+	if len(results.Accounts) != 0 {
+		t.Fatalf("Empty state should not return addresses: %v", results.Accounts)
 	}
 }