From b1ef0bfe031ea1a02cacaa1be814422ef598ca8a Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 19 Jun 2023 17:38:57 -0400 Subject: [PATCH] eth: use slices package for sorting (#27490) Also adds Hash.Less method for sorting purposes. --------- Co-authored-by: Felix Lange --- common/types.go | 5 +++ eth/api_debug_test.go | 12 ++----- eth/gasprice/feehistory.go | 25 +++++---------- eth/gasprice/gasprice.go | 53 +++++++++--------------------- eth/protocols/snap/sync_test.go | 57 ++++++++++++++++----------------- eth/tracers/api_test.go | 13 ++------ 6 files changed, 61 insertions(+), 104 deletions(-) diff --git a/common/types.go b/common/types.go index 31515f52fa..93fb090454 100644 --- a/common/types.go +++ b/common/types.go @@ -65,6 +65,11 @@ func BigToHash(b *big.Int) Hash { return BytesToHash(b.Bytes()) } // If b is larger than len(h), b will be cropped from the left. func HexToHash(s string) Hash { return BytesToHash(FromHex(s)) } +// Less compares two hashes. +func (h Hash) Less(other Hash) bool { + return bytes.Compare(h[:], other[:]) < 0 +} + // Bytes gets the byte representation of the underlying hash. func (h Hash) Bytes() []byte { return h[:] } diff --git a/eth/api_debug_test.go b/eth/api_debug_test.go index dec1b34ddc..c7f0d4ac90 100644 --- a/eth/api_debug_test.go +++ b/eth/api_debug_test.go @@ -21,7 +21,6 @@ import ( "fmt" "math/big" "reflect" - "sort" "testing" "github.com/davecgh/go-spew/spew" @@ -31,6 +30,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/trie" + "golang.org/x/exp/slices" ) var dumper = spew.ConfigState{Indent: " "} @@ -58,12 +58,6 @@ func accountRangeTest(t *testing.T, trie *state.Trie, statedb *state.StateDB, st return result } -type resultHash []common.Hash - -func (h resultHash) Len() int { return len(h) } -func (h resultHash) Swap(i, j int) { h[i], h[j] = h[j], h[i] } -func (h resultHash) Less(i, j int) bool { return bytes.Compare(h[i].Bytes(), h[j].Bytes()) < 0 } - func TestAccountRange(t *testing.T) { t.Parallel() @@ -97,7 +91,7 @@ func TestAccountRange(t *testing.T) { firstResult := accountRangeTest(t, &trie, state, common.Hash{}, AccountRangeMaxResults, AccountRangeMaxResults) secondResult := accountRangeTest(t, &trie, state, common.BytesToHash(firstResult.Next), AccountRangeMaxResults, AccountRangeMaxResults) - hList := make(resultHash, 0) + hList := make([]common.Hash, 0) for addr1 := range firstResult.Accounts { // If address is empty, then it makes no sense to compare // them as they might be two different accounts. @@ -111,7 +105,7 @@ func TestAccountRange(t *testing.T) { } // Test to see if it's possible to recover from the middle of the previous // set and get an even split between the first and second sets. - sort.Sort(hList) + slices.SortFunc(hList, common.Hash.Less) middleH := hList[AccountRangeMaxResults/2] middleResult := accountRangeTest(t, &trie, state, middleH, AccountRangeMaxResults, AccountRangeMaxResults) missing, infirst, insecond := 0, 0, 0 diff --git a/eth/gasprice/feehistory.go b/eth/gasprice/feehistory.go index ee0b0ebcca..c048377062 100644 --- a/eth/gasprice/feehistory.go +++ b/eth/gasprice/feehistory.go @@ -23,7 +23,6 @@ import ( "fmt" "math" "math/big" - "sort" "sync/atomic" "github.com/ethereum/go-ethereum/common" @@ -31,6 +30,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rpc" + "golang.org/x/exp/slices" ) var ( @@ -69,20 +69,9 @@ type processedFees struct { } // txGasAndReward is sorted in ascending order based on reward -type ( - txGasAndReward struct { - gasUsed uint64 - reward *big.Int - } - sortGasAndReward []txGasAndReward -) - -func (s sortGasAndReward) Len() int { return len(s) } -func (s sortGasAndReward) Swap(i, j int) { - s[i], s[j] = s[j], s[i] -} -func (s sortGasAndReward) Less(i, j int) bool { - return s[i].reward.Cmp(s[j].reward) < 0 +type txGasAndReward struct { + gasUsed uint64 + reward *big.Int } // processBlock takes a blockFees structure with the blockNumber, the header and optionally @@ -117,12 +106,14 @@ func (oracle *Oracle) processBlock(bf *blockFees, percentiles []float64) { return } - sorter := make(sortGasAndReward, len(bf.block.Transactions())) + sorter := make([]txGasAndReward, len(bf.block.Transactions())) for i, tx := range bf.block.Transactions() { reward, _ := tx.EffectiveGasTip(bf.block.BaseFee()) sorter[i] = txGasAndReward{gasUsed: bf.receipts[i].GasUsed, reward: reward} } - sort.Stable(sorter) + slices.SortStableFunc(sorter, func(a, b txGasAndReward) bool { + return a.reward.Cmp(b.reward) < 0 + }) var txIndex int sumGasUsed := sorter[0].gasUsed diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index a3dd83d79f..9eccd72428 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -19,7 +19,6 @@ package gasprice import ( "context" "math/big" - "sort" "sync" "github.com/ethereum/go-ethereum/common" @@ -30,6 +29,7 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" + "golang.org/x/exp/slices" ) const sampleNumber = 3 // Number of transactions sampled in a block @@ -208,7 +208,7 @@ func (oracle *Oracle) SuggestTipCap(ctx context.Context) (*big.Int, error) { } price := lastPrice if len(results) > 0 { - sort.Sort(bigIntArray(results)) + slices.SortFunc(results, func(a, b *big.Int) bool { return a.Cmp(b) < 0 }) price = results[(len(results)-1)*oracle.percentile/100] } if price.Cmp(oracle.maxPrice) > 0 { @@ -227,30 +227,6 @@ type results struct { err error } -type txSorter struct { - txs []*types.Transaction - baseFee *big.Int -} - -func newSorter(txs []*types.Transaction, baseFee *big.Int) *txSorter { - return &txSorter{ - txs: txs, - baseFee: baseFee, - } -} - -func (s *txSorter) Len() int { return len(s.txs) } -func (s *txSorter) Swap(i, j int) { - s.txs[i], s.txs[j] = s.txs[j], s.txs[i] -} -func (s *txSorter) Less(i, j int) bool { - // It's okay to discard the error because a tx would never be - // accepted into a block with an invalid effective tip. - tip1, _ := s.txs[i].EffectiveGasTip(s.baseFee) - tip2, _ := s.txs[j].EffectiveGasTip(s.baseFee) - return tip1.Cmp(tip2) < 0 -} - // getBlockValues calculates the lowest transaction gas price in a given block // and sends it to the result channel. If the block is empty or all transactions // are sent by the miner itself(it doesn't make any sense to include this kind of @@ -267,14 +243,21 @@ func (oracle *Oracle) getBlockValues(ctx context.Context, blockNum uint64, limit signer := types.MakeSigner(oracle.backend.ChainConfig(), block.Number(), block.Time()) // Sort the transaction by effective tip in ascending sort. - txs := make([]*types.Transaction, len(block.Transactions())) - copy(txs, block.Transactions()) - sorter := newSorter(txs, block.BaseFee()) - sort.Sort(sorter) + txs := block.Transactions() + sortedTxs := make([]*types.Transaction, len(txs)) + copy(sortedTxs, txs) + baseFee := block.BaseFee() + slices.SortFunc(sortedTxs, func(a, b *types.Transaction) bool { + // It's okay to discard the error because a tx would never be + // accepted into a block with an invalid effective tip. + tip1, _ := a.EffectiveGasTip(baseFee) + tip2, _ := b.EffectiveGasTip(baseFee) + return tip1.Cmp(tip2) < 0 + }) var prices []*big.Int - for _, tx := range sorter.txs { - tip, _ := tx.EffectiveGasTip(block.BaseFee()) + for _, tx := range sortedTxs { + tip, _ := tx.EffectiveGasTip(baseFee) if ignoreUnder != nil && tip.Cmp(ignoreUnder) == -1 { continue } @@ -291,9 +274,3 @@ func (oracle *Oracle) getBlockValues(ctx context.Context, blockNum uint64, limit case <-quit: } } - -type bigIntArray []*big.Int - -func (s bigIntArray) Len() int { return len(s) } -func (s bigIntArray) Less(i, j int) bool { return s[i].Cmp(s[j]) < 0 } -func (s bigIntArray) Swap(i, j int) { s[i], s[j] = s[j], s[i] } diff --git a/eth/protocols/snap/sync_test.go b/eth/protocols/snap/sync_test.go index 23af204162..6efe0c8325 100644 --- a/eth/protocols/snap/sync_test.go +++ b/eth/protocols/snap/sync_test.go @@ -22,7 +22,6 @@ import ( "encoding/binary" "fmt" "math/big" - "sort" "sync" "testing" "time" @@ -38,6 +37,7 @@ import ( "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/trie/trienode" "golang.org/x/crypto/sha3" + "golang.org/x/exp/slices" ) func TestHashing(t *testing.T) { @@ -127,9 +127,9 @@ type testPeer struct { remote *Syncer logger log.Logger accountTrie *trie.Trie - accountValues entrySlice + accountValues []*kv storageTries map[common.Hash]*trie.Trie - storageValues map[common.Hash]entrySlice + storageValues map[common.Hash][]*kv accountRequestHandler accountHandlerFunc storageRequestHandler storageHandlerFunc @@ -1321,12 +1321,9 @@ type kv struct { k, v []byte } -// Some helpers for sorting -type entrySlice []*kv - -func (p entrySlice) Len() int { return len(p) } -func (p entrySlice) Less(i, j int) bool { return bytes.Compare(p[i].k, p[j].k) < 0 } -func (p entrySlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } +func (k *kv) less(other *kv) bool { + return bytes.Compare(k.k, other.k) < 0 +} func key32(i uint64) []byte { key := make([]byte, 32) @@ -1367,11 +1364,11 @@ func getCodeByHash(hash common.Hash) []byte { } // makeAccountTrieNoStorage spits out a trie, along with the leafs -func makeAccountTrieNoStorage(n int) (string, *trie.Trie, entrySlice) { +func makeAccountTrieNoStorage(n int) (string, *trie.Trie, []*kv) { var ( db = trie.NewDatabase(rawdb.NewMemoryDatabase()) accTrie = trie.NewEmpty(db) - entries entrySlice + entries []*kv ) for i := uint64(1); i <= uint64(n); i++ { value, _ := rlp.EncodeToBytes(&types.StateAccount{ @@ -1385,7 +1382,7 @@ func makeAccountTrieNoStorage(n int) (string, *trie.Trie, entrySlice) { accTrie.MustUpdate(elem.k, elem.v) entries = append(entries, elem) } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) // Commit the state changes into db and re-create the trie // for accessing later. @@ -1399,9 +1396,9 @@ func makeAccountTrieNoStorage(n int) (string, *trie.Trie, entrySlice) { // makeBoundaryAccountTrie constructs an account trie. Instead of filling // accounts normally, this function will fill a few accounts which have // boundary hash. -func makeBoundaryAccountTrie(n int) (string, *trie.Trie, entrySlice) { +func makeBoundaryAccountTrie(n int) (string, *trie.Trie, []*kv) { var ( - entries entrySlice + entries []*kv boundaries []common.Hash db = trie.NewDatabase(rawdb.NewMemoryDatabase()) @@ -1447,7 +1444,7 @@ func makeBoundaryAccountTrie(n int) (string, *trie.Trie, entrySlice) { accTrie.MustUpdate(elem.k, elem.v) entries = append(entries, elem) } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) // Commit the state changes into db and re-create the trie // for accessing later. @@ -1460,14 +1457,14 @@ func makeBoundaryAccountTrie(n int) (string, *trie.Trie, entrySlice) { // makeAccountTrieWithStorageWithUniqueStorage creates an account trie where each accounts // has a unique storage set. -func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool) (string, *trie.Trie, entrySlice, map[common.Hash]*trie.Trie, map[common.Hash]entrySlice) { +func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool) (string, *trie.Trie, []*kv, map[common.Hash]*trie.Trie, map[common.Hash][]*kv) { var ( db = trie.NewDatabase(rawdb.NewMemoryDatabase()) accTrie = trie.NewEmpty(db) - entries entrySlice + entries []*kv storageRoots = make(map[common.Hash]common.Hash) storageTries = make(map[common.Hash]*trie.Trie) - storageEntries = make(map[common.Hash]entrySlice) + storageEntries = make(map[common.Hash][]*kv) nodes = trienode.NewMergedNodeSet() ) // Create n accounts in the trie @@ -1494,7 +1491,7 @@ func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool) storageRoots[common.BytesToHash(key)] = stRoot storageEntries[common.BytesToHash(key)] = stEntries } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) // Commit account trie root, set := accTrie.Commit(true) @@ -1515,14 +1512,14 @@ func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool) } // makeAccountTrieWithStorage spits out a trie, along with the leafs -func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (string, *trie.Trie, entrySlice, map[common.Hash]*trie.Trie, map[common.Hash]entrySlice) { +func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (string, *trie.Trie, []*kv, map[common.Hash]*trie.Trie, map[common.Hash][]*kv) { var ( db = trie.NewDatabase(rawdb.NewMemoryDatabase()) accTrie = trie.NewEmpty(db) - entries entrySlice + entries []*kv storageRoots = make(map[common.Hash]common.Hash) storageTries = make(map[common.Hash]*trie.Trie) - storageEntries = make(map[common.Hash]entrySlice) + storageEntries = make(map[common.Hash][]*kv) nodes = trienode.NewMergedNodeSet() ) // Create n accounts in the trie @@ -1536,7 +1533,7 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (strin var ( stRoot common.Hash stNodes *trienode.NodeSet - stEntries entrySlice + stEntries []*kv ) if boundary { stRoot, stNodes, stEntries = makeBoundaryStorageTrie(common.BytesToHash(key), slots, db) @@ -1559,7 +1556,7 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (strin storageRoots[common.BytesToHash(key)] = stRoot storageEntries[common.BytesToHash(key)] = stEntries } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) // Commit account trie root, set := accTrie.Commit(true) @@ -1588,9 +1585,9 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (strin // makeStorageTrieWithSeed fills a storage trie with n items, returning the // not-yet-committed trie and the sorted entries. The seeds can be used to ensure // that tries are unique. -func makeStorageTrieWithSeed(owner common.Hash, n, seed uint64, db *trie.Database) (common.Hash, *trienode.NodeSet, entrySlice) { +func makeStorageTrieWithSeed(owner common.Hash, n, seed uint64, db *trie.Database) (common.Hash, *trienode.NodeSet, []*kv) { trie, _ := trie.New(trie.StorageTrieID(types.EmptyRootHash, owner, types.EmptyRootHash), db) - var entries entrySlice + var entries []*kv for i := uint64(1); i <= n; i++ { // store 'x' at slot 'x' slotValue := key32(i + seed) @@ -1603,7 +1600,7 @@ func makeStorageTrieWithSeed(owner common.Hash, n, seed uint64, db *trie.Databas trie.MustUpdate(elem.k, elem.v) entries = append(entries, elem) } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) root, nodes := trie.Commit(false) return root, nodes, entries } @@ -1611,9 +1608,9 @@ func makeStorageTrieWithSeed(owner common.Hash, n, seed uint64, db *trie.Databas // makeBoundaryStorageTrie constructs a storage trie. Instead of filling // storage slots normally, this function will fill a few slots which have // boundary hash. -func makeBoundaryStorageTrie(owner common.Hash, n int, db *trie.Database) (common.Hash, *trienode.NodeSet, entrySlice) { +func makeBoundaryStorageTrie(owner common.Hash, n int, db *trie.Database) (common.Hash, *trienode.NodeSet, []*kv) { var ( - entries entrySlice + entries []*kv boundaries []common.Hash trie, _ = trie.New(trie.StorageTrieID(types.EmptyRootHash, owner, types.EmptyRootHash), db) ) @@ -1654,7 +1651,7 @@ func makeBoundaryStorageTrie(owner common.Hash, n int, db *trie.Database) (commo trie.MustUpdate(elem.k, elem.v) entries = append(entries, elem) } - sort.Sort(entries) + slices.SortFunc(entries, (*kv).less) root, nodes := trie.Commit(false) return root, nodes, entries } diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index fdb02b1895..11d7ff4b69 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -17,7 +17,6 @@ package tracers import ( - "bytes" "context" "crypto/ecdsa" "encoding/json" @@ -25,7 +24,6 @@ import ( "fmt" "math/big" "reflect" - "sort" "sync/atomic" "testing" "time" @@ -45,6 +43,7 @@ import ( "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" + "golang.org/x/exp/slices" ) var ( @@ -785,19 +784,13 @@ type Account struct { addr common.Address } -type Accounts []Account - -func (a Accounts) Len() int { return len(a) } -func (a Accounts) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a Accounts) Less(i, j int) bool { return bytes.Compare(a[i].addr.Bytes(), a[j].addr.Bytes()) < 0 } - -func newAccounts(n int) (accounts Accounts) { +func newAccounts(n int) (accounts []Account) { for i := 0; i < n; i++ { key, _ := crypto.GenerateKey() addr := crypto.PubkeyToAddress(key.PublicKey) accounts = append(accounts, Account{key: key, addr: addr}) } - sort.Sort(accounts) + slices.SortFunc(accounts, func(a, b Account) bool { return a.addr.Less(b.addr) }) return accounts }