eth/protocols/snap: sort trienode heal requests by path (#24779)

* sort snap trienode heal requests

* eth/protocols/snap: remove debug code

* eth/protocols/snap: simplify sort, generate pathsets later

* eth/protocols/snap: review concern

* eth/protocols/snap: renamings

* eth/protocols/snap: add comments in Merge

* eth/protocols/snap: remove variable 'last' in Merge

* eth/protocols/snap: fix lint flaws in test

Co-authored-by: Felix Lange <fjl@twurst.com>
This commit is contained in:
Martin Holst Swende 2022-05-10 16:37:24 +02:00 committed by GitHub
parent 52eb87d87c
commit 646503208e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 187 additions and 4 deletions

@ -0,0 +1,109 @@
// Copyright 2022 The go-ethereum Authors
// This file is part of the go-ethereum library.
//
// The go-ethereum library is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// The go-ethereum library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
package snap
import (
"bytes"
"fmt"
"testing"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/trie"
)
func hexToNibbles(s string) []byte {
if len(s) >= 2 && s[0] == '0' && s[1] == 'x' {
s = s[2:]
}
var s2 []byte
for _, ch := range []byte(s) {
s2 = append(s2, '0')
s2 = append(s2, ch)
}
return common.Hex2Bytes(string(s2))
}
func TestRequestSorting(t *testing.T) {
// - Path 0x9 -> {0x19}
// - Path 0x99 -> {0x0099}
// - Path 0x01234567890123456789012345678901012345678901234567890123456789019 -> {0x0123456789012345678901234567890101234567890123456789012345678901, 0x19}
// - Path 0x012345678901234567890123456789010123456789012345678901234567890199 -> {0x0123456789012345678901234567890101234567890123456789012345678901, 0x0099}
var f = func(path string) (trie.SyncPath, TrieNodePathSet, common.Hash) {
data := hexToNibbles(path)
sp := trie.NewSyncPath(data)
tnps := TrieNodePathSet([][]byte(sp))
hash := common.Hash{}
return sp, tnps, hash
}
var (
hashes []common.Hash
paths []trie.SyncPath
pathsets []TrieNodePathSet
)
for _, x := range []string{
"0x9",
"0x012345678901234567890123456789010123456789012345678901234567890195",
"0x012345678901234567890123456789010123456789012345678901234567890197",
"0x012345678901234567890123456789010123456789012345678901234567890196",
"0x99",
"0x012345678901234567890123456789010123456789012345678901234567890199",
"0x01234567890123456789012345678901012345678901234567890123456789019",
"0x0123456789012345678901234567890101234567890123456789012345678901",
"0x01234567890123456789012345678901012345678901234567890123456789010",
"0x01234567890123456789012345678901012345678901234567890123456789011",
} {
sp, tnps, hash := f(x)
hashes = append(hashes, hash)
paths = append(paths, sp)
pathsets = append(pathsets, tnps)
}
_, paths, pathsets = sortByAccountPath(hashes, paths)
{
var b = new(bytes.Buffer)
for i := 0; i < len(paths); i++ {
fmt.Fprintf(b, "\n%d. paths %x", i, paths[i])
}
want := `
0. paths [0099]
1. paths [0123456789012345678901234567890101234567890123456789012345678901 00]
2. paths [0123456789012345678901234567890101234567890123456789012345678901 0095]
3. paths [0123456789012345678901234567890101234567890123456789012345678901 0096]
4. paths [0123456789012345678901234567890101234567890123456789012345678901 0097]
5. paths [0123456789012345678901234567890101234567890123456789012345678901 0099]
6. paths [0123456789012345678901234567890101234567890123456789012345678901 10]
7. paths [0123456789012345678901234567890101234567890123456789012345678901 11]
8. paths [0123456789012345678901234567890101234567890123456789012345678901 19]
9. paths [19]`
if have := b.String(); have != want {
t.Errorf("have:%v\nwant:%v\n", have, want)
}
}
{
var b = new(bytes.Buffer)
for i := 0; i < len(pathsets); i++ {
fmt.Fprintf(b, "\n%d. pathset %x", i, pathsets[i])
}
want := `
0. pathset [0099]
1. pathset [0123456789012345678901234567890101234567890123456789012345678901 00 0095 0096 0097 0099 10 11 19]
2. pathset [19]`
if have := b.String(); have != want {
t.Errorf("have:%v\nwant:%v\n", have, want)
}
}
}

@ -1331,12 +1331,13 @@ func (s *Syncer) assignTrienodeHealTasks(success chan *trienodeHealResponse, fai
hashes = append(hashes, hash)
paths = append(paths, pathset)
pathsets = append(pathsets, [][]byte(pathset)) // TODO(karalabe): group requests by account hash
if len(hashes) >= cap {
break
}
}
// Group requests by account hash
hashes, paths, pathsets = sortByAccountPath(hashes, paths)
req := &trienodeHealRequest{
peer: idle,
id: reqid,
@ -2908,3 +2909,76 @@ func (s *capacitySort) Swap(i, j int) {
s.ids[i], s.ids[j] = s.ids[j], s.ids[i]
s.caps[i], s.caps[j] = s.caps[j], s.caps[i]
}
// healRequestSort implements the Sort interface, allowing sorting trienode
// heal requests, which is a prerequisite for merging storage-requests.
type healRequestSort struct {
hashes []common.Hash
paths []trie.SyncPath
}
func (t *healRequestSort) Len() int {
return len(t.hashes)
}
func (t *healRequestSort) Less(i, j int) bool {
a := t.paths[i]
b := t.paths[j]
switch bytes.Compare(a[0], b[0]) {
case -1:
return true
case 1:
return false
}
// identical first part
if len(a) < len(b) {
return true
}
if len(b) < len(a) {
return false
}
if len(a) == 2 {
return bytes.Compare(a[1], b[1]) < 0
}
return false
}
func (t *healRequestSort) Swap(i, j int) {
t.hashes[i], t.hashes[j] = t.hashes[j], t.hashes[i]
t.paths[i], t.paths[j] = t.paths[j], t.paths[i]
}
// Merge merges the pathsets, so that several storage requests concerning the
// same account are merged into one, to reduce bandwidth.
// OBS: This operation is moot if t has not first been sorted.
func (t *healRequestSort) Merge() []TrieNodePathSet {
var result []TrieNodePathSet
for _, path := range t.paths {
pathset := TrieNodePathSet([][]byte(path))
if len(path) == 1 {
// It's an account reference.
result = append(result, pathset)
} else {
// It's a storage reference.
end := len(result) - 1
if len(result) == 0 || !bytes.Equal(pathset[0], result[end][0]) {
// The account doesn't doesn't match last, create a new entry.
result = append(result, pathset)
} else {
// It's the same account as the previous one, add to the storage
// paths of that request.
result[end] = append(result[end], pathset[1])
}
}
}
return result
}
// sortByAccountPath takes hashes and paths, and sorts them. After that, it generates
// the TrieNodePaths and merges paths which belongs to the same account path.
func sortByAccountPath(hashes []common.Hash, paths []trie.SyncPath) ([]common.Hash, []trie.SyncPath, []TrieNodePathSet) {
n := &healRequestSort{hashes, paths}
sort.Sort(n)
pathsets := n.Merge()
return n.hashes, n.paths, pathsets
}

@ -71,9 +71,9 @@ type request struct {
// - Path 0x012345678901234567890123456789010123456789012345678901234567890199 -> {0x0123456789012345678901234567890101234567890123456789012345678901, 0x0099}
type SyncPath [][]byte
// newSyncPath converts an expanded trie path from nibble form into a compact
// NewSyncPath converts an expanded trie path from nibble form into a compact
// version that can be sent over the network.
func newSyncPath(path []byte) SyncPath {
func NewSyncPath(path []byte) SyncPath {
// If the hash is from the account trie, append a single item, if it
// is from the a storage trie, append a tuple. Note, the length 64 is
// clashing between account leaf and storage root. It's fine though
@ -238,7 +238,7 @@ func (s *Sync) Missing(max int) (nodes []common.Hash, paths []SyncPath, codes []
hash := item.(common.Hash)
if req, ok := s.nodeReqs[hash]; ok {
nodeHashes = append(nodeHashes, hash)
nodePaths = append(nodePaths, newSyncPath(req.path))
nodePaths = append(nodePaths, NewSyncPath(req.path))
} else {
codeHashes = append(codeHashes, hash)
}