panic: invalid argument to Int63n (light mainnet node)

[joeytwiddle]

System information

Geth version: geth-linux-amd64-1.9.23-8c2f2715 but also observed in 1.9.22
OS & Version: Amazon Linux 4.14.193-149.317.amzn2.x86_64
Go version: go1.13.14 linux/amd64

Actual behaviour

It runs ok for a while, but then crashes with:

panic: invalid argument to Int63n

goroutine 47 [running]:
math/rand.(*Rand).Int63n(0xc000090150, 0x8000000000000001, 0x0)
        math/rand/rand.go:111 +0x11d
math/rand.Int63n(...)
        math/rand/rand.go:327
github.com/ethereum/go-ethereum/les/utils.(*WeightedRandomSelect).Choose(0xc003344920, 0xc000e78da8, 0xc000e78d78)
        github.com/ethereum/go-ethereum/les/utils/weighted_select.go:86 +0x4f
github.com/ethereum/go-ethereum/les.(*requestDistributor).nextRequest(0xc000470bd0, 0x0, 0x0, 0x0, 0x0)
        github.com/ethereum/go-ethereum/les/distributor.go:242 +0x7b2
github.com/ethereum/go-ethereum/les.(*requestDistributor).loop(0xc000470bd0)
        github.com/ethereum/go-ethereum/les/distributor.go:145 +0xc5
created by github.com/ethereum/go-ethereum/les.newRequestDistributor
        github.com/ethereum/go-ethereum/les/distributor.go:89 +0x14c

Steps to reproduce the behaviour

I'm running a light geth node on mainnet with:

./geth --syncmode light \
  --datadir "$HOME/.ethereum-mainnet" \
  --port 20202 \
  --rpc --rpcaddr xx.xx.xx.xx --rpcport 28545 --rpcapi personal,web3,eth \
  --ws --wsaddr xx.xx.xx.xx --wsport 28546 --wsapi personal,web3,eth

Possibly relevant, I am also running a light testnet node on the same machine, and have not yet experienced the same problem. (Although that node struggles to find peers, so that could be the reason.)

(Note to self: I have now wiped the config folder and restarted, to see if that helps.)

[holiman]

@zsfelfoldi repro:

diff --git a/les/utils/weighted_select_test.go b/les/utils/weighted_select_test.go
index 3e1c0ad987..c5913ed9d7 100644
--- a/les/utils/weighted_select_test.go
+++ b/les/utils/weighted_select_test.go
@@ -66,3 +66,12 @@ func TestWeightedRandomSelect(t *testing.T) {
 	testFn(100000)
 	testFn(1000000)
 }
+
+// TestOOB tests values which doesn't fit in int64
+func TestOOB(t *testing.T) {
+	w := NewWeightedRandomSelect(func(i interface{}) uint64 {
+		return 5
+	})
+	w.root.sumWeight = uint64(0xffffffffffffffee)
+	w.Choose()
+}

It's easy to just fix by just making it ceiling to math.MaxInt64, however, there's an additional panic(nil) in func (n *wrsNode) choose(val uint64) that get's hit instead.

[holiman]

Thanks for reporting, @joeytwiddle

[holiman]

@zsfelfoldi actually, the later panic is probably just because my testcase is bad. Potential fix:

diff --git a/les/utils/weighted_select.go b/les/utils/weighted_select.go
index d6db3c0e65..15f6eae5e3 100644
--- a/les/utils/weighted_select.go
+++ b/les/utils/weighted_select.go
@@ -17,6 +17,7 @@
 package utils
 
 import (
+	"math"
 	"math/rand"
 )
 
@@ -83,6 +84,9 @@ func (w *WeightedRandomSelect) Choose() WrsItem {
 		if w.root.sumWeight == 0 {
 			return nil
 		}
+		if int64(w.root.sumWeight) < 0{
+			w.root.sumWeight = math.MaxInt64
+		}
 		val := uint64(rand.Int63n(int64(w.root.sumWeight)))
 		choice, lastWeight := w.root.choose(val)
 		weight := w.wfn(choice)

[holiman]

Or perhaps even better:

diff --git a/les/utils/weighted_select.go b/les/utils/weighted_select.go
index d6db3c0e65..d5169dbbfd 100644
--- a/les/utils/weighted_select.go
+++ b/les/utils/weighted_select.go
@@ -83,7 +83,7 @@ func (w *WeightedRandomSelect) Choose() WrsItem {
 		if w.root.sumWeight == 0 {
 			return nil
 		}
-		val := uint64(rand.Int63n(int64(w.root.sumWeight)))
+		val := rand.Uint64() % w.root.sumWeight
 		choice, lastWeight := w.root.choose(val)
 		weight := w.wfn(choice)
 		if weight != lastWeight {