From 63f4f3465922bda6ce572d7e6d77ecd4dd586e2d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 5 Jun 2020 19:01:10 +0300 Subject: [PATCH] mempool: drop TxWithFee type All the fees are in transaction, this makes no sense. --- pkg/consensus/consensus.go | 11 +++++------ pkg/core/blockchain.go | 6 +++--- pkg/core/blockchainer/blockchainer.go | 2 +- pkg/core/mempool/mem_pool.go | 19 ++++++------------- pkg/core/mempool/mem_pool_test.go | 14 ++++++-------- pkg/network/helper_test.go | 2 +- pkg/rpc/server/server.go | 2 +- pkg/rpc/server/server_test.go | 2 +- 8 files changed, 24 insertions(+), 34 deletions(-) diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 190f3577c..e0098f70e 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -12,7 +12,6 @@ import ( "github.com/nspcc-dev/dbft/payload" coreb "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/blockchainer" - "github.com/nspcc-dev/neo-go/pkg/core/mempool" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/io" @@ -394,13 +393,13 @@ func (s *service) getBlock(h util.Uint256) block.Block { func (s *service) getVerifiedTx(count int) []block.Transaction { pool := s.Config.Chain.GetMemPool() - var txx []mempool.TxWithFee + var txx []*transaction.Transaction if s.dbft.ViewNumber > 0 { - txx = make([]mempool.TxWithFee, 0, len(s.lastProposal)) + txx = make([]*transaction.Transaction, 0, len(s.lastProposal)) for i := range s.lastProposal { - if tx, fee, ok := pool.TryGetValue(s.lastProposal[i]); ok { - txx = append(txx, mempool.TxWithFee{Tx: tx, Fee: fee}) + if tx, ok := pool.TryGetValue(s.lastProposal[i]); ok { + txx = append(txx, tx) } } @@ -417,7 +416,7 @@ func (s *service) getVerifiedTx(count int) []block.Transaction { res := make([]block.Transaction, len(txx)) for i := range txx { - res[i] = txx[i].Tx + res[i] = txx[i] } return res diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 8eb65985d..1979b39d6 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -825,7 +825,7 @@ func (bc *Blockchain) headerListLen() (n int) { // GetTransaction returns a TX and its height by the given hash. func (bc *Blockchain) GetTransaction(hash util.Uint256) (*transaction.Transaction, uint32, error) { - if tx, _, ok := bc.memPool.TryGetValue(hash); ok { + if tx, ok := bc.memPool.TryGetValue(hash); ok { return tx, 0, nil // the height is not actually defined for memPool transaction. Not sure if zero is a good number in this case. } return bc.dao.GetTransaction(hash) @@ -1075,14 +1075,14 @@ func (bc *Blockchain) GetMemPool() *mempool.Pool { // ApplyPolicyToTxSet applies configured policies to given transaction set. It // expects slice to be ordered by fee and returns a subslice of it. -func (bc *Blockchain) ApplyPolicyToTxSet(txes []mempool.TxWithFee) []mempool.TxWithFee { +func (bc *Blockchain) ApplyPolicyToTxSet(txes []*transaction.Transaction) []*transaction.Transaction { if bc.config.MaxTransactionsPerBlock != 0 && len(txes) > bc.config.MaxTransactionsPerBlock { txes = txes[:bc.config.MaxTransactionsPerBlock] } maxFree := bc.config.MaxFreeTransactionsPerBlock if maxFree != 0 { lowStart := sort.Search(len(txes), func(i int) bool { - return bc.IsLowPriority(txes[i].Fee) + return bc.IsLowPriority(txes[i].NetworkFee) }) if lowStart+maxFree < len(txes) { txes = txes[:lowStart+maxFree] diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index 7d93f307b..5b84826b4 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -14,7 +14,7 @@ import ( // Blockchainer is an interface that abstract the implementation // of the blockchain. type Blockchainer interface { - ApplyPolicyToTxSet([]mempool.TxWithFee) []mempool.TxWithFee + ApplyPolicyToTxSet([]*transaction.Transaction) []*transaction.Transaction GetConfig() config.ProtocolConfiguration AddHeaders(...*block.Header) error AddBlock(*block.Block) error diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go index f0ed818b4..748c8b811 100644 --- a/pkg/core/mempool/mem_pool.go +++ b/pkg/core/mempool/mem_pool.go @@ -33,12 +33,6 @@ type item struct { // items is a slice of item. type items []*item -// TxWithFee combines transaction and its precalculated network fee. -type TxWithFee struct { - Tx *transaction.Transaction - Fee util.Fixed8 -} - // utilityBalanceAndFees stores sender's balance and overall fees of // sender's transactions which are currently in mempool type utilityBalanceAndFees struct { @@ -260,26 +254,25 @@ func NewMemPool(capacity int) Pool { } // TryGetValue returns a transaction and its fee if it exists in the memory pool. -func (mp *Pool) TryGetValue(hash util.Uint256) (*transaction.Transaction, util.Fixed8, bool) { +func (mp *Pool) TryGetValue(hash util.Uint256) (*transaction.Transaction, bool) { mp.lock.RLock() defer mp.lock.RUnlock() if pItem, ok := mp.verifiedMap[hash]; ok { - return pItem.txn, pItem.txn.NetworkFee, ok + return pItem.txn, ok } - return nil, 0, false + return nil, false } // GetVerifiedTransactions returns a slice of transactions with their fees. -func (mp *Pool) GetVerifiedTransactions() []TxWithFee { +func (mp *Pool) GetVerifiedTransactions() []*transaction.Transaction { mp.lock.RLock() defer mp.lock.RUnlock() - var t = make([]TxWithFee, len(mp.verifiedTxes)) + var t = make([]*transaction.Transaction, len(mp.verifiedTxes)) for i := range mp.verifiedTxes { - t[i].Tx = mp.verifiedTxes[i].txn - t[i].Fee = mp.verifiedTxes[i].txn.NetworkFee + t[i] = mp.verifiedTxes[i].txn } return t diff --git a/pkg/core/mempool/mem_pool_test.go b/pkg/core/mempool/mem_pool_test.go index 33474540d..969652c11 100644 --- a/pkg/core/mempool/mem_pool_test.go +++ b/pkg/core/mempool/mem_pool_test.go @@ -32,16 +32,16 @@ func testMemPoolAddRemoveWithFeer(t *testing.T, fs Feer) { mp := NewMemPool(10) tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) tx.Nonce = 0 - _, _, ok := mp.TryGetValue(tx.Hash()) + _, ok := mp.TryGetValue(tx.Hash()) require.Equal(t, false, ok) require.NoError(t, mp.Add(tx, fs)) // Re-adding should fail. require.Error(t, mp.Add(tx, fs)) - tx2, _, ok := mp.TryGetValue(tx.Hash()) + tx2, ok := mp.TryGetValue(tx.Hash()) require.Equal(t, true, ok) require.Equal(t, tx, tx2) mp.Remove(tx.Hash()) - _, _, ok = mp.TryGetValue(tx.Hash()) + _, ok = mp.TryGetValue(tx.Hash()) require.Equal(t, false, ok) // Make sure nothing left in the mempool after removal. assert.Equal(t, 0, len(mp.verifiedMap)) @@ -142,9 +142,7 @@ func TestGetVerified(t *testing.T) { require.Equal(t, mempoolSize, mp.Count()) verTxes := mp.GetVerifiedTransactions() require.Equal(t, mempoolSize, len(verTxes)) - for _, txf := range verTxes { - require.Contains(t, txes, txf.Tx) - } + require.ElementsMatch(t, txes, verTxes) for _, tx := range txes { mp.Remove(tx.Hash()) } @@ -181,8 +179,8 @@ func TestRemoveStale(t *testing.T) { require.Equal(t, mempoolSize/2, mp.Count()) verTxes := mp.GetVerifiedTransactions() for _, txf := range verTxes { - require.NotContains(t, txes1, txf.Tx) - require.Contains(t, txes2, txf.Tx) + require.NotContains(t, txes1, txf) + require.Contains(t, txes2, txf) } } diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go index bdd85eecd..fe60dac6d 100644 --- a/pkg/network/helper_test.go +++ b/pkg/network/helper_test.go @@ -25,7 +25,7 @@ type testChain struct { blockheight uint32 } -func (chain testChain) ApplyPolicyToTxSet([]mempool.TxWithFee) []mempool.TxWithFee { +func (chain testChain) ApplyPolicyToTxSet([]*transaction.Transaction) []*transaction.Transaction { panic("TODO") } func (chain testChain) GetConfig() config.ProtocolConfiguration { diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 79fdeae1e..dbed9ab8b 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -461,7 +461,7 @@ func (s *Server) getRawMempool(_ request.Params) (interface{}, *response.Error) mp := s.chain.GetMemPool() hashList := make([]util.Uint256, 0) for _, item := range mp.GetVerifiedTransactions() { - hashList = append(hashList, item.Tx.Hash()) + hashList = append(hashList, item.Hash()) } return hashList, nil } diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index ca1d11690..69af43432 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -943,7 +943,7 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) [] // `expected` stores hashes of previously added txs expected := make([]util.Uint256, 0) for _, tx := range mp.GetVerifiedTransactions() { - expected = append(expected, tx.Tx.Hash()) + expected = append(expected, tx.Hash()) } for i := 0; i < 5; i++ { tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0)