From 6f88d6530a819a3a65c4b95681e6c52115365622 Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Thu, 19 Nov 2020 22:50:47 +0100
Subject: [PATCH] trie, rpc, cmd/geth: fix tests on 32-bit and windows + minor
 rpc fixes (#21871)

* trie: fix tests to work on 32-bit systems

* les: make test work on 32-bit platform

* cmd/geth: fix windows-issues on tests

* trie: improve balance

* cmd/geth: make account tests less verbose + less mem intense

* rpc: make debug-level log output less verbose

* cmd/geth: lint
---
 cmd/geth/accountcmd_test.go  | 29 +++++++++---------
 cmd/geth/les_test.go         | 35 +++++++++++++++-------
 cmd/geth/run_test.go         |  8 ++---
 internal/cmdtest/test_cmd.go | 15 ++++++----
 node/rpcstack.go             |  1 +
 rpc/endpoints.go             | 14 +++++++--
 trie/trie_test.go            | 57 ++++++++++++++++++++----------------
 7 files changed, 98 insertions(+), 61 deletions(-)

diff --git a/cmd/geth/accountcmd_test.go b/cmd/geth/accountcmd_test.go
index 6213e5195..2f15915b0 100644
--- a/cmd/geth/accountcmd_test.go
+++ b/cmd/geth/accountcmd_test.go
@@ -180,8 +180,8 @@ Fatal: could not decrypt key with given password
 func TestUnlockFlag(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t,
-		"--datadir", datadir, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a",
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "256", "--ipcdisable",
+		"--datadir", datadir, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a",
 		"js", "testdata/empty.js")
 	geth.Expect(`
 Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
@@ -204,8 +204,8 @@ Password: {{.InputLine "foobar"}}
 func TestUnlockFlagWrongPassword(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t,
-		"--datadir", datadir, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a")
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--datadir", datadir, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a")
 	defer geth.ExpectExit()
 	geth.Expect(`
 Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
@@ -223,9 +223,8 @@ Fatal: Failed to unlock account f466859ead1932d743d622cb74fc058882e8648a (could
 func TestUnlockFlagMultiIndex(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t,
-		"--datadir", datadir, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--unlock", "0,2",
-		"js", "testdata/empty.js")
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--datadir", datadir, "--unlock", "0,2", "js", "testdata/empty.js")
 	geth.Expect(`
 Unlocking account 0 | Attempt 1/3
 !! Unsupported terminal, password will be echoed.
@@ -250,8 +249,8 @@ Password: {{.InputLine "foobar"}}
 func TestUnlockFlagPasswordFile(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t,
-		"--datadir", datadir, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--password", "testdata/passwords.txt", "--unlock", "0,2",
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--datadir", datadir, "--password", "testdata/passwords.txt", "--unlock", "0,2",
 		"js", "testdata/empty.js")
 	geth.ExpectExit()
 
@@ -270,8 +269,8 @@ func TestUnlockFlagPasswordFile(t *testing.T) {
 func TestUnlockFlagPasswordFileWrongPassword(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t,
-		"--datadir", datadir, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--password", "testdata/wrong-passwords.txt", "--unlock", "0,2")
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--datadir", datadir, "--password", "testdata/wrong-passwords.txt", "--unlock", "0,2")
 	defer geth.ExpectExit()
 	geth.Expect(`
 Fatal: Failed to unlock account 0 (could not decrypt key with given password)
@@ -281,8 +280,8 @@ Fatal: Failed to unlock account 0 (could not decrypt key with given password)
 func TestUnlockFlagAmbiguous(t *testing.T) {
 	store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes")
 	geth := runGeth(t,
-		"--keystore", store, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a",
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--keystore", store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a",
 		"js", "testdata/empty.js")
 	defer geth.ExpectExit()
 
@@ -319,8 +318,8 @@ In order to avoid this warning, you need to remove the following duplicate key f
 func TestUnlockFlagAmbiguousWrongPassword(t *testing.T) {
 	store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes")
 	geth := runGeth(t,
-		"--keystore", store, "--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0",
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a")
+		"--nat", "none", "--nodiscover", "--maxpeers", "0", "--port", "0", "--nousb", "--cache", "128", "--ipcdisable",
+		"--keystore", store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a")
 	defer geth.ExpectExit()
 
 	// Helper for the expect template, returns absolute keystore path.
diff --git a/cmd/geth/les_test.go b/cmd/geth/les_test.go
index 242564651..e4fc2d4d0 100644
--- a/cmd/geth/les_test.go
+++ b/cmd/geth/les_test.go
@@ -2,10 +2,12 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -119,24 +121,36 @@ func ipcEndpoint(ipcPath, datadir string) string {
 	return ipcPath
 }
 
+// nextIPC ensures that each ipc pipe gets a unique name.
+// On linux, it works well to use ipc pipes all over the filesystem (in datadirs),
+// but windows require pipes to sit in "\\.\pipe\". Therefore, to run several
+// nodes simultaneously, we need to distinguish between them, which we do by
+// the pipe filename instead of folder.
+var nextIPC = uint32(0)
+
 func startGethWithIpc(t *testing.T, name string, args ...string) *gethrpc {
-	g := &gethrpc{name: name}
-	args = append([]string{"--networkid=42", "--port=0", "--nousb"}, args...)
+	ipcName := fmt.Sprintf("geth-%d.ipc", atomic.AddUint32(&nextIPC, 1))
+	args = append([]string{"--networkid=42", "--port=0", "--nousb", "--ipcpath", ipcName}, args...)
 	t.Logf("Starting %v with rpc: %v", name, args)
-	g.geth = runGeth(t, args...)
+
+	g := &gethrpc{
+		name: name,
+		geth: runGeth(t, args...),
+	}
 	// wait before we can attach to it. TODO: probe for it properly
 	time.Sleep(1 * time.Second)
 	var err error
-	ipcpath := ipcEndpoint("geth.ipc", g.geth.Datadir)
-	g.rpc, err = rpc.Dial(ipcpath)
-	if err != nil {
+	ipcpath := ipcEndpoint(ipcName, g.geth.Datadir)
+	if g.rpc, err = rpc.Dial(ipcpath); err != nil {
 		t.Fatalf("%v rpc connect to %v: %v", name, ipcpath, err)
 	}
 	return g
 }
 
 func initGeth(t *testing.T) string {
-	g := runGeth(t, "--nousb", "--networkid=42", "init", "./testdata/clique.json")
+	args := []string{"--nousb", "--networkid=42", "init", "./testdata/clique.json"}
+	t.Logf("Initializing geth: %v ", args)
+	g := runGeth(t, args...)
 	datadir := g.Datadir
 	g.WaitExit()
 	return datadir
@@ -144,15 +158,16 @@ func initGeth(t *testing.T) string {
 
 func startLightServer(t *testing.T) *gethrpc {
 	datadir := initGeth(t)
+	t.Logf("Importing keys to geth")
 	runGeth(t, "--nousb", "--datadir", datadir, "--password", "./testdata/password.txt", "account", "import", "./testdata/key.prv").WaitExit()
 	account := "0x02f0d131f1f97aef08aec6e3291b957d9efe7105"
-	server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1")
+	server := startGethWithIpc(t, "lightserver", "--allow-insecure-unlock", "--datadir", datadir, "--password", "./testdata/password.txt", "--unlock", account, "--mine", "--light.serve=100", "--light.maxpeers=1", "--nodiscover", "--nat=extip:127.0.0.1", "--verbosity=4")
 	return server
 }
 
 func startClient(t *testing.T, name string) *gethrpc {
 	datadir := initGeth(t)
-	return startGethWithIpc(t, name, "--datadir", datadir, "--nodiscover", "--syncmode=light", "--nat=extip:127.0.0.1")
+	return startGethWithIpc(t, name, "--datadir", datadir, "--nodiscover", "--syncmode=light", "--nat=extip:127.0.0.1", "--verbosity=4")
 }
 
 func TestPriorityClient(t *testing.T) {
@@ -175,7 +190,7 @@ func TestPriorityClient(t *testing.T) {
 	prioCli := startClient(t, "prioCli")
 	defer prioCli.killAndWait()
 	// 3_000_000_000 once we move to Go 1.13
-	tokens := 3000000000
+	tokens := uint64(3000000000)
 	lightServer.callRPC(nil, "les_addBalance", prioCli.getNodeInfo().ID, tokens)
 	prioCli.addPeer(lightServer)
 
diff --git a/cmd/geth/run_test.go b/cmd/geth/run_test.go
index f7b735b84..79b892c59 100644
--- a/cmd/geth/run_test.go
+++ b/cmd/geth/run_test.go
@@ -70,12 +70,12 @@ func runGeth(t *testing.T, args ...string) *testgeth {
 	tt := &testgeth{}
 	tt.TestCmd = cmdtest.NewTestCmd(t, tt)
 	for i, arg := range args {
-		switch {
-		case arg == "-datadir" || arg == "--datadir":
+		switch arg {
+		case "--datadir":
 			if i < len(args)-1 {
 				tt.Datadir = args[i+1]
 			}
-		case arg == "-etherbase" || arg == "--etherbase":
+		case "--etherbase":
 			if i < len(args)-1 {
 				tt.Etherbase = args[i+1]
 			}
@@ -84,7 +84,7 @@ func runGeth(t *testing.T, args ...string) *testgeth {
 	if tt.Datadir == "" {
 		tt.Datadir = tmpdir(t)
 		tt.Cleanup = func() { os.RemoveAll(tt.Datadir) }
-		args = append([]string{"-datadir", tt.Datadir}, args...)
+		args = append([]string{"--datadir", tt.Datadir}, args...)
 		// Remove the temporary datadir if something fails below.
 		defer func() {
 			if t.Failed() {
diff --git a/internal/cmdtest/test_cmd.go b/internal/cmdtest/test_cmd.go
index 0edfccec5..82ad9c15b 100644
--- a/internal/cmdtest/test_cmd.go
+++ b/internal/cmdtest/test_cmd.go
@@ -27,6 +27,7 @@ import (
 	"regexp"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"syscall"
 	"testing"
 	"text/template"
@@ -55,10 +56,13 @@ type TestCmd struct {
 	Err error
 }
 
+var id int32
+
 // Run exec's the current binary using name as argv[0] which will trigger the
 // reexec init function for that name (e.g. "geth-test" in cmd/geth/run_test.go)
 func (tt *TestCmd) Run(name string, args ...string) {
-	tt.stderr = &testlogger{t: tt.T}
+	id := atomic.AddInt32(&id, 1)
+	tt.stderr = &testlogger{t: tt.T, name: fmt.Sprintf("%d", id)}
 	tt.cmd = &exec.Cmd{
 		Path:   reexec.Self(),
 		Args:   append([]string{name}, args...),
@@ -238,16 +242,17 @@ func (tt *TestCmd) withKillTimeout(fn func()) {
 // testlogger logs all written lines via t.Log and also
 // collects them for later inspection.
 type testlogger struct {
-	t   *testing.T
-	mu  sync.Mutex
-	buf bytes.Buffer
+	t    *testing.T
+	mu   sync.Mutex
+	buf  bytes.Buffer
+	name string
 }
 
 func (tl *testlogger) Write(b []byte) (n int, err error) {
 	lines := bytes.Split(b, []byte("\n"))
 	for _, line := range lines {
 		if len(line) > 0 {
-			tl.t.Logf("(stderr) %s", line)
+			tl.t.Logf("(stderr:%v) %s", tl.name, line)
 		}
 	}
 	tl.mu.Lock()
diff --git a/node/rpcstack.go b/node/rpcstack.go
index 731e807ac..81e054ec9 100644
--- a/node/rpcstack.go
+++ b/node/rpcstack.go
@@ -448,6 +448,7 @@ func (is *ipcServer) start(apis []rpc.API) error {
 	}
 	listener, srv, err := rpc.StartIPCEndpoint(is.endpoint, apis)
 	if err != nil {
+		is.log.Warn("IPC opening failed", "url", is.endpoint, "error", err)
 		return err
 	}
 	is.log.Info("IPC endpoint opened", "url", is.endpoint)
diff --git a/rpc/endpoints.go b/rpc/endpoints.go
index 9fc070517..d78ebe285 100644
--- a/rpc/endpoints.go
+++ b/rpc/endpoints.go
@@ -18,6 +18,7 @@ package rpc
 
 import (
 	"net"
+	"strings"
 
 	"github.com/ethereum/go-ethereum/log"
 )
@@ -25,13 +26,22 @@ import (
 // StartIPCEndpoint starts an IPC endpoint.
 func StartIPCEndpoint(ipcEndpoint string, apis []API) (net.Listener, *Server, error) {
 	// Register all the APIs exposed by the services.
-	handler := NewServer()
+	var (
+		handler    = NewServer()
+		regMap     = make(map[string]struct{})
+		registered []string
+	)
 	for _, api := range apis {
 		if err := handler.RegisterName(api.Namespace, api.Service); err != nil {
+			log.Info("IPC registration failed", "namespace", api.Namespace, "error", err)
 			return nil, nil, err
 		}
-		log.Debug("IPC registered", "namespace", api.Namespace)
+		if _, ok := regMap[api.Namespace]; !ok {
+			registered = append(registered, api.Namespace)
+			regMap[api.Namespace] = struct{}{}
+		}
 	}
+	log.Debug("IPCs registered", "namespaces", strings.Join(registered, ","))
 	// All APIs registered, start the IPC listener.
 	listener, err := ipcListen(ipcEndpoint)
 	if err != nil {
diff --git a/trie/trie_test.go b/trie/trie_test.go
index 682dec157..ddbdcbbd5 100644
--- a/trie/trie_test.go
+++ b/trie/trie_test.go
@@ -594,21 +594,20 @@ func benchmarkCommitAfterHash(b *testing.B, onleaf LeafCallback) {
 
 func TestTinyTrie(t *testing.T) {
 	// Create a realistic account trie to hash
-	_, accounts := makeAccounts(10000)
+	_, accounts := makeAccounts(5)
 	trie := newEmpty()
 	trie.Update(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000001337"), accounts[3])
-	if exp, root := common.HexToHash("4fa6efd292cffa2db0083b8bedd23add2798ae73802442f52486e95c3df7111c"), trie.Hash(); exp != root {
-		t.Fatalf("1: got %x, exp %x", root, exp)
+	if exp, root := common.HexToHash("8c6a85a4d9fda98feff88450299e574e5378e32391f75a055d470ac0653f1005"), trie.Hash(); exp != root {
+		t.Errorf("1: got %x, exp %x", root, exp)
 	}
 	trie.Update(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000001338"), accounts[4])
-	if exp, root := common.HexToHash("cb5fb1213826dad9e604f095f8ceb5258fe6b5c01805ce6ef019a50699d2d479"), trie.Hash(); exp != root {
-		t.Fatalf("2: got %x, exp %x", root, exp)
+	if exp, root := common.HexToHash("ec63b967e98a5720e7f720482151963982890d82c9093c0d486b7eb8883a66b1"), trie.Hash(); exp != root {
+		t.Errorf("2: got %x, exp %x", root, exp)
 	}
 	trie.Update(common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000001339"), accounts[4])
-	if exp, root := common.HexToHash("ed7e06b4010057d8703e7b9a160a6d42cf4021f9020da3c8891030349a646987"), trie.Hash(); exp != root {
-		t.Fatalf("3: got %x, exp %x", root, exp)
+	if exp, root := common.HexToHash("0608c1d1dc3905fa22204c7a0e43644831c3b6d3def0f274be623a948197e64a"), trie.Hash(); exp != root {
+		t.Errorf("3: got %x, exp %x", root, exp)
 	}
-
 	checktr, _ := New(common.Hash{}, trie.db)
 	it := NewIterator(trie.NodeIterator(nil))
 	for it.Next() {
@@ -630,7 +629,7 @@ func TestCommitAfterHash(t *testing.T) {
 	trie.Hash()
 	trie.Commit(nil)
 	root := trie.Hash()
-	exp := common.HexToHash("e5e9c29bb50446a4081e6d1d748d2892c6101c1e883a1f77cf21d4094b697822")
+	exp := common.HexToHash("72f9d3f3fe1e1dd7b8936442e7642aef76371472d94319900790053c493f3fe6")
 	if exp != root {
 		t.Errorf("got %x, exp %x", root, exp)
 	}
@@ -646,19 +645,27 @@ func makeAccounts(size int) (addresses [][20]byte, accounts [][]byte) {
 	// Create a realistic account trie to hash
 	addresses = make([][20]byte, size)
 	for i := 0; i < len(addresses); i++ {
-		for j := 0; j < len(addresses[i]); j++ {
-			addresses[i][j] = byte(random.Intn(256))
-		}
+		data := make([]byte, 20)
+		random.Read(data)
+		copy(addresses[i][:], data)
 	}
 	accounts = make([][]byte, len(addresses))
 	for i := 0; i < len(accounts); i++ {
 		var (
-			nonce   = uint64(random.Int63())
-			balance = new(big.Int).Rand(random, new(big.Int).Exp(common.Big2, common.Big256, nil))
-			root    = emptyRoot
-			code    = crypto.Keccak256(nil)
+			nonce = uint64(random.Int63())
+			root  = emptyRoot
+			code  = crypto.Keccak256(nil)
 		)
-		accounts[i], _ = rlp.EncodeToBytes(&account{nonce, balance, root, code})
+		// The big.Rand function is not deterministic with regards to 64 vs 32 bit systems,
+		// and will consume different amount of data from the rand source.
+		//balance = new(big.Int).Rand(random, new(big.Int).Exp(common.Big2, common.Big256, nil))
+		// Therefore, we instead just read via byte buffer
+		numBytes := random.Uint32() % 33 // [0, 32] bytes
+		balanceBytes := make([]byte, numBytes)
+		random.Read(balanceBytes)
+		balance := new(big.Int).SetBytes(balanceBytes)
+		data, _ := rlp.EncodeToBytes(&account{nonce, balance, root, code})
+		accounts[i] = data
 	}
 	return addresses, accounts
 }
@@ -714,12 +721,12 @@ func TestCommitSequence(t *testing.T) {
 		expWriteSeqHash    []byte
 		expCallbackSeqHash []byte
 	}{
-		{20, common.FromHex("68c495e45209e243eb7e4f4e8ca8f9f7be71003bd9cafb8061b4534373740193"),
-			common.FromHex("01783213033d6b7781a641ab499e680d959336d025ac16f44d02f4f0c021bbf5")},
-		{200, common.FromHex("3b20d16c13c4bc3eb3b8d0ad7a169fef3b1600e056c0665895d03d3d2b2ff236"),
-			common.FromHex("fb8db0ec82e8f02729f11228940885b181c3047ab0d654ed0110291ca57111a8")},
-		{2000, common.FromHex("34eff3d1048bebdf77e9ae8bd939f2e7c742edc3dcd1173cff1aad9dbd20451a"),
-			common.FromHex("1c981604b1a9f8ffa40e0ae66b14830a87f5a4ed8345146a3912e6b2dcb05e63")},
+		{20, common.FromHex("873c78df73d60e59d4a2bcf3716e8bfe14554549fea2fc147cb54129382a8066"),
+			common.FromHex("ff00f91ac05df53b82d7f178d77ada54fd0dca64526f537034a5dbe41b17df2a")},
+		{200, common.FromHex("ba03d891bb15408c940eea5ee3d54d419595102648d02774a0268d892add9c8e"),
+			common.FromHex("f3cd509064c8d319bbdd1c68f511850a902ad275e6ed5bea11547e23d492a926")},
+		{2000, common.FromHex("f7a184f20df01c94f09537401d11e68d97ad0c00115233107f51b9c287ce60c7"),
+			common.FromHex("ff795ea898ba1e4cfed4a33b4cf5535a347a02cf931f88d88719faf810f9a1c9")},
 	} {
 		addresses, accounts := makeAccounts(tc.count)
 		// This spongeDb is used to check the sequence of disk-db-writes
@@ -740,10 +747,10 @@ func TestCommitSequence(t *testing.T) {
 			callbackSponge.Write(c[:])
 		})
 		if got, exp := s.sponge.Sum(nil), tc.expWriteSeqHash; !bytes.Equal(got, exp) {
-			t.Fatalf("test %d, disk write sequence wrong:\ngot %x exp %x\n", i, got, exp)
+			t.Errorf("test %d, disk write sequence wrong:\ngot %x exp %x\n", i, got, exp)
 		}
 		if got, exp := callbackSponge.Sum(nil), tc.expCallbackSeqHash; !bytes.Equal(got, exp) {
-			t.Fatalf("test %d, call back sequence wrong:\ngot: %x exp %x\n", i, got, exp)
+			t.Errorf("test %d, call back sequence wrong:\ngot: %x exp %x\n", i, got, exp)
 		}
 	}
 }
-- 
GitLab