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.

Sign up for GitHub

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

Jump to bottom

trie, rpc, cmd/geth: fix tests on 32-bit and windows + minor rpc fixes #21871

Merged
merged 7 commits into from
Nov 19, 2020
Merged

trie, rpc, cmd/geth: fix tests on 32-bit and windows + minor rpc fixes #21871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a69e10
trie: fix tests to work on 32-bit systems
holiman Nov 18, 2020
f22b786
les: make test work on 32-bit platform
holiman Nov 18, 2020
ef9fde5
cmd/geth: fix windows-issues on tests
holiman Nov 18, 2020
0a6f81f
trie: improve balance
holiman Nov 19, 2020
7796749
cmd/geth: make account tests less verbose + less mem intense
holiman Nov 19, 2020
4e776da
rpc: make debug-level log output less verbose
holiman Nov 19, 2020
fe99afc
cmd/geth: lint
holiman Nov 19, 2020
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
29 changes: 14 additions & 15 deletions cmd/geth/accountcmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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()

Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -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.
Expand Down
35 changes: 25 additions & 10 deletions cmd/geth/les_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package main

import (
"context"
"fmt"
"os"
"path/filepath"
"runtime"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -119,40 +121,53 @@ 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
}

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) {
Expand All @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions cmd/geth/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
Expand All @@ -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() {
Expand Down
15 changes: 10 additions & 5 deletions internal/cmdtest/test_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"regexp"
"strings"
"sync"
"sync/atomic"
"syscall"
"testing"
"text/template"
Expand Down Expand Up @@ -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...),
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions node/rpcstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 12 additions & 2 deletions rpc/endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,30 @@ package rpc

import (
"net"
"strings"

"github.com/ethereum/go-ethereum/log"
)

// 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 {
Expand Down
57 changes: 32 additions & 25 deletions trie/trie_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
}
Expand All @@ -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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Randomizing the number of digits and then randomizing the digits makes for a very uneven distribution. You could just always read and set 32 bytes as the big Int will be normalized to remove leading zeros. I do understand that this is just a test and that it really doesn't matter, but just wanted to point it out :-). Also, another alternative to big.Int.Rand() is crypto/rand.Int() that works on bytes, not words, so it would be deterministic in this regard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just always read and set 32 bytes as the big Int will be normalized to remove leading zeros.

Actually, that was my first replacement. However, the intent is to have rlp blobs of varying size, and thus have a lot of small (1-byte) values -- which basically would never ever happpen when using a random 32-byte array. So this is worse from a pure distribution perspective, but better in this particular instance.

balance := new(big.Int).SetBytes(balanceBytes)
data, _ := rlp.EncodeToBytes(&account{nonce, balance, root, code})
accounts[i] = data
}
return addresses, accounts
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
}
Expand Down