From 646503208e6525e49ef4648a008f7577aa168abb Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 10 May 2022 16:37:24 +0200 Subject: [PATCH] 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 --- eth/protocols/snap/sort_test.go | 109 ++++++++++++++++++++++++++++++++ eth/protocols/snap/sync.go | 76 +++++++++++++++++++++- trie/sync.go | 6 +- 3 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 eth/protocols/snap/sort_test.go diff --git a/eth/protocols/snap/sort_test.go b/eth/protocols/snap/sort_test.go new file mode 100644 index 000000000000..c625be09ea54 --- /dev/null +++ b/eth/protocols/snap/sort_test.go @@ -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 . + +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) + } + } +} diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 5bbf5ee48214..415253c839be 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -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 +} diff --git a/trie/sync.go b/trie/sync.go index 7eaa35244e4e..db51dd4b036a 100644 --- a/trie/sync.go +++ b/trie/sync.go @@ -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) }