From a928ad9cfa968c291dae93a0485702340fc2a3ce Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 5 Feb 2020 14:24:36 +0300 Subject: [PATCH] mempool: make item an internal thing of mempool package Nobody outside should care about these details, mempool operates on transactions and that's it. --- pkg/consensus/consensus_test.go | 13 ++--- pkg/core/blockchain.go | 2 +- pkg/core/mempool/mem_pool.go | 80 +++++++++++++++---------------- pkg/core/mempool/mem_pool_test.go | 11 ++--- 4 files changed, 47 insertions(+), 59 deletions(-) diff --git a/pkg/consensus/consensus_test.go b/pkg/consensus/consensus_test.go index 654f86ade..9db01b198 100644 --- a/pkg/consensus/consensus_test.go +++ b/pkg/consensus/consensus_test.go @@ -5,7 +5,6 @@ import ( "github.com/CityOfZion/neo-go/config" "github.com/CityOfZion/neo-go/pkg/core" - "github.com/CityOfZion/neo-go/pkg/core/mempool" "github.com/CityOfZion/neo-go/pkg/core/storage" "github.com/CityOfZion/neo-go/pkg/core/transaction" "github.com/CityOfZion/neo-go/pkg/crypto/keys" @@ -22,8 +21,7 @@ func TestNewService(t *testing.T) { Type: transaction.MinerType, Data: &transaction.MinerTX{Nonce: 12345}, } - item := mempool.NewPoolItem(tx, new(feer)) - srv.Chain.GetMemPool().TryAdd(tx.Hash(), item) + srv.Chain.GetMemPool().Add(tx, new(feer)) var txx []block.Transaction require.NotPanics(t, func() { txx = srv.getVerifiedTx(1) }) @@ -41,9 +39,8 @@ func TestService_GetVerified(t *testing.T) { newMinerTx(4), } pool := srv.Chain.GetMemPool() - item := mempool.NewPoolItem(txs[3], new(feer)) - require.NoError(t, pool.TryAdd(txs[3].Hash(), item)) + require.NoError(t, pool.Add(txs[3], new(feer))) hashes := []util.Uint256{txs[0].Hash(), txs[1].Hash(), txs[2].Hash()} @@ -68,8 +65,7 @@ func TestService_GetVerified(t *testing.T) { t.Run("more than half of the last proposal will be reused", func(t *testing.T) { for _, tx := range txs[:2] { - item := mempool.NewPoolItem(tx, new(feer)) - require.NoError(t, pool.TryAdd(tx.Hash(), item)) + require.NoError(t, pool.Add(tx, new(feer))) } txx := srv.getVerifiedTx(10) @@ -119,8 +115,7 @@ func TestService_getTx(t *testing.T) { require.Equal(t, nil, srv.getTx(h)) - item := mempool.NewPoolItem(tx, new(feer)) - srv.Chain.GetMemPool().TryAdd(h, item) + srv.Chain.GetMemPool().Add(tx, new(feer)) got := srv.getTx(h) require.NotNil(t, got) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 48491edd6..b92d307f9 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1064,7 +1064,7 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { if err := bc.verifyTx(t, nil); err != nil { return err } - if err := bc.memPool.TryAdd(t.Hash(), mempool.NewPoolItem(t, bc)); err != nil { + if err := bc.memPool.Add(t, bc); err != nil { switch err { case mempool.ErrOOM: return ErrOOM diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go index d6a3b94d7..aa28b04a8 100644 --- a/pkg/core/mempool/mem_pool.go +++ b/pkg/core/mempool/mem_pool.go @@ -23,38 +23,38 @@ var ( ErrOOM = errors.New("out of memory") ) -// Item represents a transaction in the the Memory pool. -type Item struct { +// item represents a transaction in the the Memory pool. +type item struct { txn *transaction.Transaction timeStamp time.Time fee Feer } -// Items is a slice of Item. -type Items []*Item +// items is a slice of item. +type items []*item // Pool stores the unconfirms transactions. type Pool struct { lock *sync.RWMutex - unsortedTxn map[util.Uint256]*Item - unverifiedTxn map[util.Uint256]*Item - sortedHighPrioTxn Items - sortedLowPrioTxn Items - unverifiedSortedHighPrioTxn Items - unverifiedSortedLowPrioTxn Items + unsortedTxn map[util.Uint256]*item + unverifiedTxn map[util.Uint256]*item + sortedHighPrioTxn items + sortedLowPrioTxn items + unverifiedSortedHighPrioTxn items + unverifiedSortedLowPrioTxn items capacity int } -func (p Items) Len() int { return len(p) } -func (p Items) Swap(i, j int) { p[i], p[j] = p[j], p[i] } -func (p Items) Less(i, j int) bool { return p[i].CompareTo(p[j]) < 0 } +func (p items) Len() int { return len(p) } +func (p items) Swap(i, j int) { p[i], p[j] = p[j], p[i] } +func (p items) Less(i, j int) bool { return p[i].CompareTo(p[j]) < 0 } -// CompareTo returns the difference between two Items. +// CompareTo returns the difference between two items. // difference < 0 implies p < otherP. // difference = 0 implies p = otherP. // difference > 0 implies p > otherP. -func (p Item) CompareTo(otherP *Item) int { +func (p item) CompareTo(otherP *item) int { if otherP == nil { return 1 } @@ -114,9 +114,14 @@ func (mp *Pool) ContainsKey(hash util.Uint256) bool { return false } -// TryAdd try to add the Item to the Pool. -func (mp *Pool) TryAdd(hash util.Uint256, pItem *Item) error { - var pool *Items +// Add tries to add given transaction to the Pool. +func (mp *Pool) Add(t *transaction.Transaction, fee Feer) error { + var pool *items + var pItem = &item{ + txn: t, + timeStamp: time.Now().UTC(), + fee: fee, + } if pItem.fee.IsLowPriority(pItem.txn) { pool = &mp.sortedLowPrioTxn @@ -129,11 +134,11 @@ func (mp *Pool) TryAdd(hash util.Uint256, pItem *Item) error { mp.lock.Unlock() return ErrConflict } - if _, ok := mp.unsortedTxn[hash]; ok { + if _, ok := mp.unsortedTxn[t.Hash()]; ok { mp.lock.Unlock() return ErrDup } - mp.unsortedTxn[hash] = pItem + mp.unsortedTxn[t.Hash()] = pItem *pool = append(*pool, pItem) sort.Sort(pool) @@ -143,7 +148,7 @@ func (mp *Pool) TryAdd(hash util.Uint256, pItem *Item) error { mp.RemoveOverCapacity() } mp.lock.RLock() - _, ok := mp.unsortedTxn[hash] + _, ok := mp.unsortedTxn[t.Hash()] updateMempoolMetrics(len(mp.unsortedTxn), len(mp.unverifiedTxn)) mp.lock.RUnlock() if !ok { @@ -156,11 +161,11 @@ func (mp *Pool) TryAdd(hash util.Uint256, pItem *Item) error { // nothing if it doesn't). func (mp *Pool) Remove(hash util.Uint256) { var mapAndPools = []struct { - unsortedMap map[util.Uint256]*Item - sortedPools []*Items + unsortedMap map[util.Uint256]*item + sortedPools []*items }{ - {unsortedMap: mp.unsortedTxn, sortedPools: []*Items{&mp.sortedHighPrioTxn, &mp.sortedLowPrioTxn}}, - {unsortedMap: mp.unverifiedTxn, sortedPools: []*Items{&mp.unverifiedSortedHighPrioTxn, &mp.unverifiedSortedLowPrioTxn}}, + {unsortedMap: mp.unsortedTxn, sortedPools: []*items{&mp.sortedHighPrioTxn, &mp.sortedLowPrioTxn}}, + {unsortedMap: mp.unverifiedTxn, sortedPools: []*items{&mp.unverifiedSortedHighPrioTxn, &mp.unverifiedSortedLowPrioTxn}}, } mp.lock.Lock() for _, mapAndPool := range mapAndPools { @@ -168,7 +173,7 @@ func (mp *Pool) Remove(hash util.Uint256) { delete(mapAndPool.unsortedMap, hash) for _, pool := range mapAndPool.sortedPools { var num int - var item *Item + var item *item for num, item = range *pool { if hash.Equals(item.txn.Hash()) { break @@ -224,21 +229,12 @@ func (mp *Pool) RemoveOverCapacity() { } -// NewPoolItem returns a new Item. -func NewPoolItem(t *transaction.Transaction, fee Feer) *Item { - return &Item{ - txn: t, - timeStamp: time.Now().UTC(), - fee: fee, - } -} - // NewMemPool returns a new Pool struct. func NewMemPool(capacity int) Pool { return Pool{ lock: new(sync.RWMutex), - unsortedTxn: make(map[util.Uint256]*Item), - unverifiedTxn: make(map[util.Uint256]*Item), + unsortedTxn: make(map[util.Uint256]*item), + unverifiedTxn: make(map[util.Uint256]*item), capacity: capacity, } } @@ -258,13 +254,13 @@ func (mp *Pool) TryGetValue(hash util.Uint256) (*transaction.Transaction, bool) return nil, false } -// getLowestFeeTransaction returns the Item with the lowest fee amongst the "verifiedTxnSorted" -// and "unverifiedTxnSorted" Items along with a integer. The integer can assume two values, 1 and 2 which indicate -// that the Item with the lowest fee was found in "verifiedTxnSorted" respectively in "unverifiedTxnSorted". +// getLowestFeeTransaction returns the item with the lowest fee amongst the "verifiedTxnSorted" +// and "unverifiedTxnSorted" items along with a integer. The integer can assume two values, 1 and 2 which indicate +// that the item with the lowest fee was found in "verifiedTxnSorted" respectively in "unverifiedTxnSorted". // "verifiedTxnSorted" and "unverifiedTxnSorted" are sorted slice order by transaction fee ascending. This means that // the transaction with lowest fee start at index 0. // Reference: GetLowestFeeTransaction method in C# (https://github.com/neo-project/neo/blob/master/neo/Ledger/MemoryPool.cs) -func getLowestFeeTransaction(verifiedTxnSorted Items, unverifiedTxnSorted Items) (*Item, int) { +func getLowestFeeTransaction(verifiedTxnSorted items, unverifiedTxnSorted items) (*item, int) { minItem := min(unverifiedTxnSorted) verifiedMin := min(verifiedTxnSorted) if verifiedMin == nil || (minItem != nil && verifiedMin.CompareTo(minItem) >= 0) { @@ -278,7 +274,7 @@ func getLowestFeeTransaction(verifiedTxnSorted Items, unverifiedTxnSorted Items) // min returns the minimum item in a ascending sorted slice of pool items. // The function can't be applied to unsorted slice! -func min(sortedPool Items) *Item { +func min(sortedPool items) *item { if len(sortedPool) == 0 { return nil } diff --git a/pkg/core/mempool/mem_pool_test.go b/pkg/core/mempool/mem_pool_test.go index db6a5b25c..546febe75 100644 --- a/pkg/core/mempool/mem_pool_test.go +++ b/pkg/core/mempool/mem_pool_test.go @@ -36,12 +36,11 @@ func (fs *FeerStub) SystemFee(*transaction.Transaction) util.Fixed8 { func testMemPoolAddRemoveWithFeer(t *testing.T, fs Feer) { mp := NewMemPool(10) tx := newMinerTX() - item := NewPoolItem(tx, fs) _, ok := mp.TryGetValue(tx.Hash()) require.Equal(t, false, ok) - require.NoError(t, mp.TryAdd(tx.Hash(), item)) + require.NoError(t, mp.Add(tx, fs)) // Re-adding should fail. - require.Error(t, mp.TryAdd(tx.Hash(), item)) + require.Error(t, mp.Add(tx, fs)) tx2, ok := mp.TryGetValue(tx.Hash()) require.Equal(t, true, ok) require.Equal(t, tx, tx2) @@ -70,15 +69,13 @@ func TestMemPoolVerify(t *testing.T) { inhash1 := random.Uint256() tx.Inputs = append(tx.Inputs, transaction.Input{PrevHash: inhash1, PrevIndex: 0}) require.Equal(t, true, mp.Verify(tx)) - item := NewPoolItem(tx, &FeerStub{}) - require.NoError(t, mp.TryAdd(tx.Hash(), item)) + require.NoError(t, mp.Add(tx, &FeerStub{})) tx2 := newMinerTX() inhash2 := random.Uint256() tx2.Inputs = append(tx2.Inputs, transaction.Input{PrevHash: inhash2, PrevIndex: 0}) require.Equal(t, true, mp.Verify(tx2)) - item = NewPoolItem(tx2, &FeerStub{}) - require.NoError(t, mp.TryAdd(tx2.Hash(), item)) + require.NoError(t, mp.Add(tx2, &FeerStub{})) tx3 := newMinerTX() // Different index number, but the same PrevHash as in tx1.