diff --git a/p2p/discover/v5_udp.go b/p2p/discover/v5_udp.go index 1c66602f8791..321c5bd2a818 100644 --- a/p2p/discover/v5_udp.go +++ b/p2p/discover/v5_udp.go @@ -24,7 +24,6 @@ import ( "errors" "fmt" "io" - "math" "net" "sync" "time" @@ -41,7 +40,6 @@ const ( lookupRequestLimit = 3 // max requests against a single node during lookup findnodeResultLimit = 16 // applies in FINDNODE handler totalNodesResponseLimit = 5 // applies in waitForNodes - nodesResponseItemLimit = 3 // applies in sendNodes respTimeoutV5 = 700 * time.Millisecond ) @@ -832,17 +830,29 @@ func packNodes(reqid []byte, nodes []*enode.Node) []*v5wire.Nodes { return []*v5wire.Nodes{{ReqID: reqid, Total: 1}} } - total := uint8(math.Ceil(float64(len(nodes)) / 3)) + // This limit represents the available space for nodes in output packets. Maximum + // packet size is 1280, and out of this ~80 bytes will be taken up by the packet + // frame. So limiting to 1000 bytes here leaves 200 bytes for other fields of the + // NODES message, which is a lot. + const sizeLimit = 1000 + var resp []*v5wire.Nodes for len(nodes) > 0 { - p := &v5wire.Nodes{ReqID: reqid, Total: total} - items := min(nodesResponseItemLimit, len(nodes)) - for i := 0; i < items; i++ { - p.Nodes = append(p.Nodes, nodes[i].Record()) + p := &v5wire.Nodes{ReqID: reqid} + size := uint64(0) + for len(nodes) > 0 { + r := nodes[0].Record() + if size += r.Size(); size > sizeLimit { + break + } + p.Nodes = append(p.Nodes, r) + nodes = nodes[1:] } - nodes = nodes[items:] resp = append(resp, p) } + for _, msg := range resp { + msg.Total = uint8(len(resp)) + } return resp } diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index ca63688afa13..ab0cb9a8211c 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -161,7 +161,7 @@ func TestUDPv5_findnodeHandling(t *testing.T) { defer test.close() // Create test nodes and insert them into the table. - nodes253 := nodesAtDistance(test.table.self().ID(), 253, 10) + nodes253 := nodesAtDistance(test.table.self().ID(), 253, 16) nodes249 := nodesAtDistance(test.table.self().ID(), 249, 4) nodes248 := nodesAtDistance(test.table.self().ID(), 248, 10) fillTable(test.table, wrapNodes(nodes253)) @@ -186,7 +186,7 @@ func TestUDPv5_findnodeHandling(t *testing.T) { // This request gets all the distance-253 nodes. test.packetIn(&v5wire.Findnode{ReqID: []byte{4}, Distances: []uint{253}}) - test.expectNodes([]byte{4}, 4, nodes253) + test.expectNodes([]byte{4}, 1, nodes253) // This request gets all the distance-249 nodes and some more at 248 because // the bucket at 249 is not full. @@ -194,7 +194,7 @@ func TestUDPv5_findnodeHandling(t *testing.T) { var nodes []*enode.Node nodes = append(nodes, nodes249...) nodes = append(nodes, nodes248[:10]...) - test.expectNodes([]byte{5}, 5, nodes) + test.expectNodes([]byte{5}, 1, nodes) } func (test *udpV5Test) expectNodes(wantReqID []byte, wantTotal uint8, wantNodes []*enode.Node) { @@ -208,9 +208,6 @@ func (test *udpV5Test) expectNodes(wantReqID []byte, wantTotal uint8, wantNodes if !bytes.Equal(p.ReqID, wantReqID) { test.t.Fatalf("wrong request ID %v in response, want %v", p.ReqID, wantReqID) } - if len(p.Nodes) > 3 { - test.t.Fatalf("too many nodes in response") - } if p.Total != wantTotal { test.t.Fatalf("wrong total response count %d, want %d", p.Total, wantTotal) } diff --git a/p2p/enr/enr.go b/p2p/enr/enr.go index 438c7b8a3b36..2b093b2f1ab1 100644 --- a/p2p/enr/enr.go +++ b/p2p/enr/enr.go @@ -96,6 +96,24 @@ type pair struct { v rlp.RawValue } +// Size returns the encoded size of the record. +func (r *Record) Size() uint64 { + if r.raw != nil { + return uint64(len(r.raw)) + } + return computeSize(r) +} + +func computeSize(r *Record) uint64 { + size := uint64(rlp.IntSize(r.seq)) + size += rlp.BytesSize(r.signature) + for _, p := range r.pairs { + size += rlp.StringSize(p.k) + size += uint64(len(p.v)) + } + return rlp.ListSize(size) +} + // Seq returns the sequence number. func (r *Record) Seq() uint64 { return r.seq diff --git a/p2p/enr/enr_test.go b/p2p/enr/enr_test.go index bf3f1047440e..b85ee209d591 100644 --- a/p2p/enr/enr_test.go +++ b/p2p/enr/enr_test.go @@ -169,6 +169,32 @@ func TestDirty(t *testing.T) { } } +func TestSize(t *testing.T) { + var r Record + + // Empty record size is 3 bytes. + // Unsigned records cannot be encoded, but they could, the encoding + // would be [ 0, 0 ] -> 0xC28080. + assert.Equal(t, uint64(3), r.Size()) + + // Add one attribute. The size increases to 5, the encoding + // would be [ 0, 0, "k", "v" ] -> 0xC58080C26B76. + r.Set(WithEntry("k", "v")) + assert.Equal(t, uint64(5), r.Size()) + + // Now add a signature. + nodeid := []byte{1, 2, 3, 4, 5, 6, 7, 8} + signTest(nodeid, &r) + assert.Equal(t, uint64(45), r.Size()) + enc, _ := rlp.EncodeToBytes(&r) + if r.Size() != uint64(len(enc)) { + t.Error("Size() not equal encoded length", len(enc)) + } + if r.Size() != computeSize(&r) { + t.Error("Size() not equal computed size", computeSize(&r)) + } +} + func TestSeq(t *testing.T) { var r Record @@ -268,8 +294,11 @@ func TestSignEncodeAndDecodeRandom(t *testing.T) { } require.NoError(t, signTest([]byte{5}, &r)) - _, err := rlp.EncodeToBytes(r) + + enc, err := rlp.EncodeToBytes(r) require.NoError(t, err) + require.Equal(t, uint64(len(enc)), r.Size()) + require.Equal(t, uint64(len(enc)), computeSize(&r)) for k, v := range pairs { desc := fmt.Sprintf("key %q", k) diff --git a/rlp/raw.go b/rlp/raw.go index f355efc144df..773aa7e614e8 100644 --- a/rlp/raw.go +++ b/rlp/raw.go @@ -28,13 +28,46 @@ type RawValue []byte var rawValueType = reflect.TypeOf(RawValue{}) +// StringSize returns the encoded size of a string. +func StringSize(s string) uint64 { + switch { + case len(s) == 0: + return 1 + case len(s) == 1: + if s[0] <= 0x7f { + return 1 + } else { + return 2 + } + default: + return uint64(headsize(uint64(len(s))) + len(s)) + } +} + +// BytesSize returns the encoded size of a byte slice. +func BytesSize(b []byte) uint64 { + switch { + case len(b) == 0: + return 1 + case len(b) == 1: + if b[0] <= 0x7f { + return 1 + } else { + return 2 + } + default: + return uint64(headsize(uint64(len(b))) + len(b)) + } +} + // ListSize returns the encoded size of an RLP list with the given // content size. func ListSize(contentSize uint64) uint64 { return uint64(headsize(contentSize)) + contentSize } -// IntSize returns the encoded size of the integer x. +// IntSize returns the encoded size of the integer x. Note: The return type of this +// function is 'int' for backwards-compatibility reasons. The result is always positive. func IntSize(x uint64) int { if x < 0x80 { return 1 diff --git a/rlp/raw_test.go b/rlp/raw_test.go index 46adff22c5da..2812ef74c622 100644 --- a/rlp/raw_test.go +++ b/rlp/raw_test.go @@ -283,3 +283,36 @@ func TestAppendUint64Random(t *testing.T) { t.Fatal(err) } } + +func TestBytesSize(t *testing.T) { + tests := []struct { + v []byte + size uint64 + }{ + {v: []byte{}, size: 1}, + {v: []byte{0x1}, size: 1}, + {v: []byte{0x7E}, size: 1}, + {v: []byte{0x7F}, size: 1}, + {v: []byte{0x80}, size: 2}, + {v: []byte{0xFF}, size: 2}, + {v: []byte{0xFF, 0xF0}, size: 3}, + {v: make([]byte, 55), size: 56}, + {v: make([]byte, 56), size: 58}, + } + + for _, test := range tests { + s := BytesSize(test.v) + if s != test.size { + t.Errorf("BytesSize(%#x) -> %d, want %d", test.v, s, test.size) + } + s = StringSize(string(test.v)) + if s != test.size { + t.Errorf("StringSize(%#x) -> %d, want %d", test.v, s, test.size) + } + // Sanity check: + enc, _ := EncodeToBytes(test.v) + if uint64(len(enc)) != test.size { + t.Errorf("len(EncodeToBytes(%#x)) -> %d, test says %d", test.v, len(enc), test.size) + } + } +}