From 3a379e4f3e1eab5cb4d1f0ab2271b310232adc64 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 14:38:58 +0300
Subject: [PATCH 01/10] core: don't reverify mempooled transactions in AddBlock

The end effect is almost as if `VerifyTransactions: false` was set in the
config, but without actually compromising the guarantees provided by it.

It almost doubles performance for single-mode benchmarks and makes block
processing smoother (more smaller blocks are being produced).
---
 pkg/core/blockchain.go      |  6 ++++++
 pkg/core/blockchain_test.go | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go
index ab8cae8b7..fd47b9f91 100644
--- a/pkg/core/blockchain.go
+++ b/pkg/core/blockchain.go
@@ -437,6 +437,12 @@ func (bc *Blockchain) AddBlock(block *block.Block) error {
 		}
 		if bc.config.VerifyTransactions {
 			for _, tx := range block.Transactions {
+				// Transactions are verified before adding them
+				// into the pool, so there is no point in doing
+				// it again even if we're verifying in-block transactions.
+				if bc.memPool.ContainsKey(tx.Hash()) {
+					continue
+				}
 				err := bc.VerifyTx(tx, block)
 				if err != nil {
 					return fmt.Errorf("transaction %s failed to verify: %w", tx.Hash().StringLE(), err)
diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go
index 1b9ba1af4..58aa342cc 100644
--- a/pkg/core/blockchain_test.go
+++ b/pkg/core/blockchain_test.go
@@ -117,6 +117,43 @@ func TestAddBlock(t *testing.T) {
 	assert.Equal(t, lastBlock.Hash(), bc.CurrentHeaderHash())
 }
 
+func TestAddBadBlock(t *testing.T) {
+	bc := newTestChain(t)
+	defer bc.Close()
+	// It has ValidUntilBlock == 0, which is wrong
+	tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
+	tx.Signers = []transaction.Signer{{
+		Account: testchain.MultisigScriptHash(),
+		Scopes:  transaction.FeeOnly,
+	}}
+	require.NoError(t, signTx(bc, tx))
+	b1 := bc.newBlock(tx)
+
+	require.Error(t, bc.AddBlock(b1))
+	bc.config.VerifyTransactions = false
+	require.NoError(t, bc.AddBlock(b1))
+
+	b2 := bc.newBlock()
+	b2.PrevHash = util.Uint256{}
+
+	require.Error(t, bc.AddBlock(b2))
+	bc.config.VerifyBlocks = false
+	require.NoError(t, bc.AddBlock(b2))
+
+	tx = transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
+	tx.ValidUntilBlock = 128
+	tx.Signers = []transaction.Signer{{
+		Account: testchain.MultisigScriptHash(),
+		Scopes:  transaction.FeeOnly,
+	}}
+	require.NoError(t, signTx(bc, tx))
+	require.NoError(t, bc.PoolTx(tx))
+	bc.config.VerifyTransactions = true
+	bc.config.VerifyBlocks = true
+	b3 := bc.newBlock(tx)
+	require.NoError(t, bc.AddBlock(b3))
+}
+
 func TestScriptFromWitness(t *testing.T) {
 	witness := &transaction.Witness{}
 	h := util.Uint160{1, 2, 3}

From c7032022f8c4848f6236ad64ef8788cbc44a2034 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 15:27:13 +0300
Subject: [PATCH 02/10] core: don't search through the whole DAO in
 isTxStillRelevant

New transactions are added to the chain with blocks. If there is no
transaction X at height N in DAO, it could only be added with block N+1, so
it has to be present there. Therefore we can replace `dao.HasTransaction()`
check with a search through in-block transactions. HasTransaction() is nasty
in that it may add useless load the DB and this code is being run with a big
Blockchain lock held, so we don't want to be delayed here at all.

Improves single-node TPS by ~2%.
---
 pkg/core/blockchain.go      | 28 ++++++++++++++++++----------
 pkg/core/blockchain_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go
index fd47b9f91..c69116048 100644
--- a/pkg/core/blockchain.go
+++ b/pkg/core/blockchain.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"sort"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -607,7 +608,8 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 		}
 	}
 
-	for _, tx := range block.Transactions {
+	var txHashes = make([]util.Uint256, len(block.Transactions))
+	for i, tx := range block.Transactions {
 		if err := cache.StoreAsTransaction(tx, block.Index); err != nil {
 			return err
 		}
@@ -624,8 +626,8 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 			if err != nil {
 				return fmt.Errorf("failed to persist invocation results: %w", err)
 			}
-			for i := range systemInterop.Notifications {
-				bc.handleNotification(&systemInterop.Notifications[i], cache, block, tx.Hash())
+			for j := range systemInterop.Notifications {
+				bc.handleNotification(&systemInterop.Notifications[j], cache, block, tx.Hash())
 			}
 		} else {
 			bc.log.Warn("contract invocation failed",
@@ -646,7 +648,11 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 		if err != nil {
 			return fmt.Errorf("failed to store tx exec result: %w", err)
 		}
+		txHashes[i] = tx.Hash()
 	}
+	sort.Slice(txHashes, func(i, j int) bool {
+		return txHashes[i].CompareTo(txHashes[j]) < 0
+	})
 
 	root := bc.dao.MPT.StateRoot()
 	var prevHash util.Uint256
@@ -688,7 +694,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 	}
 	bc.topBlock.Store(block)
 	atomic.StoreUint32(&bc.blockHeight, block.Index)
-	bc.memPool.RemoveStale(bc.isTxStillRelevant, bc)
+	bc.memPool.RemoveStale(func(tx *transaction.Transaction) bool { return bc.isTxStillRelevant(tx, txHashes) }, bc)
 	bc.lock.Unlock()
 
 	updateBlockHeightMetric(block.Index)
@@ -1251,16 +1257,18 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e
 }
 
 // isTxStillRelevant is a callback for mempool transaction filtering after the
-// new block addition. It returns false for transactions already present in the
-// chain (added by the new block), transactions using some inputs that are
-// already used (double spends) and does witness reverification for non-standard
+// new block addition. It returns false for transactions added by the new block
+// (passed via txHashes) and does witness reverification for non-standard
 // contracts. It operates under the assumption that full transaction verification
 // was already done so we don't need to check basic things like size, input/output
-// correctness, etc.
-func (bc *Blockchain) isTxStillRelevant(t *transaction.Transaction) bool {
+// correctness, presence in blocks before the new one, etc.
+func (bc *Blockchain) isTxStillRelevant(t *transaction.Transaction, txHashes []util.Uint256) bool {
 	var recheckWitness bool
 
-	if bc.dao.HasTransaction(t.Hash()) {
+	index := sort.Search(len(txHashes), func(i int) bool {
+		return txHashes[i].CompareTo(t.Hash()) >= 0
+	})
+	if index < len(txHashes) && txHashes[index].Equals(t.Hash()) {
 		return false
 	}
 	for i := range t.Scripts {
diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go
index 58aa342cc..dbb589b4a 100644
--- a/pkg/core/blockchain_test.go
+++ b/pkg/core/blockchain_test.go
@@ -402,6 +402,34 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 	})
 }
 
+func TestMemPoolRemoval(t *testing.T) {
+	const added = 16
+	const notAdded = 32
+	bc := newTestChain(t)
+	defer bc.Close()
+	addedTxes := make([]*transaction.Transaction, added)
+	notAddedTxes := make([]*transaction.Transaction, notAdded)
+	for i := range addedTxes {
+		addedTxes[i] = bc.newTestTx(testchain.MultisigScriptHash(), []byte{byte(opcode.PUSH1)})
+		require.NoError(t, signTx(bc, addedTxes[i]))
+		require.NoError(t, bc.PoolTx(addedTxes[i]))
+	}
+	for i := range notAddedTxes {
+		notAddedTxes[i] = bc.newTestTx(testchain.MultisigScriptHash(), []byte{byte(opcode.PUSH1)})
+		require.NoError(t, signTx(bc, notAddedTxes[i]))
+		require.NoError(t, bc.PoolTx(notAddedTxes[i]))
+	}
+	b := bc.newBlock(addedTxes...)
+	require.NoError(t, bc.AddBlock(b))
+	mempool := bc.GetMemPool()
+	for _, tx := range addedTxes {
+		require.False(t, mempool.ContainsKey(tx.Hash()))
+	}
+	for _, tx := range notAddedTxes {
+		require.True(t, mempool.ContainsKey(tx.Hash()))
+	}
+}
+
 func TestHasBlock(t *testing.T) {
 	bc := newTestChain(t)
 	blocks, err := bc.genBlocks(50)

From 0bf2fa915e55e11b5a8b3d379c260eb6b8164ec6 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 16:55:01 +0300
Subject: [PATCH 03/10] consensus: don't decrypt the key again and again, cache
 it

It's cached in dbft for a view anyway, so there is no big difference here
from security POV. Lets us squeeze yet another 4% TPS improvement.

Make the system fail if unable to decrypt the key along the way, which is a
part of #1312.
---
 pkg/consensus/consensus.go | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go
index 928109ded..e616043b0 100644
--- a/pkg/consensus/consensus.go
+++ b/pkg/consensus/consensus.go
@@ -16,6 +16,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
+	"github.com/nspcc-dev/neo-go/pkg/encoding/address"
 	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract"
 	"github.com/nspcc-dev/neo-go/pkg/util"
@@ -255,9 +256,14 @@ func (s *service) getKeyPair(pubs []crypto.PublicKey) (int, crypto.PrivateKey, c
 			continue
 		}
 
-		key, err := keys.NEP2Decrypt(acc.EncryptedWIF, s.Config.Wallet.Password)
-		if err != nil {
-			continue
+		key := acc.PrivateKey()
+		if acc.PrivateKey() == nil {
+			err := acc.Decrypt(s.Config.Wallet.Password)
+			if err != nil {
+				s.log.Fatal("can't unlock account", zap.String("address", address.Uint160ToString(sh)))
+				break
+			}
+			key = acc.PrivateKey()
 		}
 
 		return i, &privateKey{PrivateKey: key}, &publicKey{PublicKey: key.PublicKey()}

From e998c102cae987b3f9fa42b08ae64139c8c32c19 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 17:57:30 +0300
Subject: [PATCH 04/10] mempool: rename NewMemPool into New

"mempool" is a package name already.
---
 pkg/core/blockchain.go            |  2 +-
 pkg/core/mempool/mem_pool.go      |  4 ++--
 pkg/core/mempool/mem_pool_test.go | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go
index c69116048..bac3ea1ad 100644
--- a/pkg/core/blockchain.go
+++ b/pkg/core/blockchain.go
@@ -168,7 +168,7 @@ func NewBlockchain(s storage.Store, cfg config.ProtocolConfiguration, log *zap.L
 		headersOpDone: make(chan struct{}),
 		stopCh:        make(chan struct{}),
 		runToExitCh:   make(chan struct{}),
-		memPool:       mempool.NewMemPool(cfg.MemPoolSize),
+		memPool:       mempool.New(cfg.MemPoolSize),
 		keyCache:      make(map[util.Uint160]map[string]*keys.PublicKey),
 		sbCommittee:   committee,
 		log:           log,
diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go
index 4a862d7da..00fcb464c 100644
--- a/pkg/core/mempool/mem_pool.go
+++ b/pkg/core/mempool/mem_pool.go
@@ -247,8 +247,8 @@ func (mp *Pool) checkPolicy(tx *transaction.Transaction, policyChanged bool) boo
 	return false
 }
 
-// NewMemPool returns a new Pool struct.
-func NewMemPool(capacity int) Pool {
+// New returns a new Pool struct.
+func New(capacity int) Pool {
 	return Pool{
 		verifiedMap:  make(map[util.Uint256]*item),
 		verifiedTxes: make([]*item, 0, capacity),
diff --git a/pkg/core/mempool/mem_pool_test.go b/pkg/core/mempool/mem_pool_test.go
index ad404dcd7..a7420b255 100644
--- a/pkg/core/mempool/mem_pool_test.go
+++ b/pkg/core/mempool/mem_pool_test.go
@@ -28,7 +28,7 @@ func (fs *FeerStub) GetUtilityTokenBalance(uint160 util.Uint160) *big.Int {
 }
 
 func testMemPoolAddRemoveWithFeer(t *testing.T, fs Feer) {
-	mp := NewMemPool(10)
+	mp := New(10)
 	tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
 	tx.Nonce = 0
 	tx.Signers = []transaction.Signer{{Account: util.Uint160{1, 2, 3}}}
@@ -56,7 +56,7 @@ func TestMemPoolAddRemove(t *testing.T) {
 func TestOverCapacity(t *testing.T) {
 	var fs = &FeerStub{}
 	const mempoolSize = 10
-	mp := NewMemPool(mempoolSize)
+	mp := New(mempoolSize)
 
 	for i := 0; i < mempoolSize; i++ {
 		tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
@@ -129,7 +129,7 @@ func TestOverCapacity(t *testing.T) {
 func TestGetVerified(t *testing.T) {
 	var fs = &FeerStub{}
 	const mempoolSize = 10
-	mp := NewMemPool(mempoolSize)
+	mp := New(mempoolSize)
 
 	txes := make([]*transaction.Transaction, 0, mempoolSize)
 	for i := 0; i < mempoolSize; i++ {
@@ -153,7 +153,7 @@ func TestGetVerified(t *testing.T) {
 func TestRemoveStale(t *testing.T) {
 	var fs = &FeerStub{}
 	const mempoolSize = 10
-	mp := NewMemPool(mempoolSize)
+	mp := New(mempoolSize)
 
 	txes1 := make([]*transaction.Transaction, 0, mempoolSize/2)
 	txes2 := make([]*transaction.Transaction, 0, mempoolSize/2)
@@ -186,7 +186,7 @@ func TestRemoveStale(t *testing.T) {
 }
 
 func TestMemPoolFees(t *testing.T) {
-	mp := NewMemPool(10)
+	mp := New(10)
 	sender0 := util.Uint160{1, 2, 3}
 	tx0 := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0)
 	tx0.NetworkFee = balance.Int64() + 1

From 0d8cc437fe0bf96d356c87c87a1dd717f78bcc9a Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 18:36:57 +0300
Subject: [PATCH 05/10] mempool: swap checks in Add, fail fast

Checking for duplicates is easier than checking the balance, so it should be
done first.
---
 pkg/core/mempool/mem_pool.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go
index 00fcb464c..b4798d004 100644
--- a/pkg/core/mempool/mem_pool.go
+++ b/pkg/core/mempool/mem_pool.go
@@ -136,14 +136,14 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer) error {
 		timeStamp: time.Now().UTC(),
 	}
 	mp.lock.Lock()
-	if !mp.checkTxConflicts(t, fee) {
-		mp.lock.Unlock()
-		return ErrConflict
-	}
 	if mp.containsKey(t.Hash()) {
 		mp.lock.Unlock()
 		return ErrDup
 	}
+	if !mp.checkTxConflicts(t, fee) {
+		mp.lock.Unlock()
+		return ErrConflict
+	}
 
 	mp.verifiedMap[t.Hash()] = pItem
 	// Insert into sorted array (from max to min, that could also be done

From 55b2cbb74dcecacc22cbfffd727831ccca459148 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 19:27:15 +0300
Subject: [PATCH 06/10] core: refactor and improve verification and pooling

Now we have VerifyTx() and PoolTx() APIs that either verify transaction in
isolation or verify it against the mempool (either the primary one or the one
given) and then add it there. There is no possibility to check against the
mempool, but not add a transaction to it, but I doubt we really need it.

It allows to remove some duplication between old PoolTx and verifyTx where
they both tried to check transaction against mempool (verifying first and then
adding it). It also saves us utility token balance check because it's done by
the mempool anyway and we no longer need to do that explicitly in verifyTx.

It makes AddBlock() and verifyBlock() transaction's checks more correct,
because previously they could miss that even though sender S has enough
balance to pay for A, B or C, he can't pay for all of them.

Caveats:
 * consensus is running concurrently to other processes, so things could
   change while verifyBlock() is iterating over transactions, this will be
   mitigated in subsequent commits

Improves TPS value for single node by at least 11%.

Fixes #667, fixes #668.
---
 pkg/consensus/consensus.go            | 15 ++++-
 pkg/core/blockchain.go                | 88 +++++++++++++++------------
 pkg/core/blockchain_test.go           | 41 +++++++++++--
 pkg/core/blockchainer/blockchainer.go |  4 +-
 pkg/core/mempool/mem_pool.go          | 34 +++++++----
 pkg/network/helper_test.go            |  4 +-
 6 files changed, 127 insertions(+), 59 deletions(-)

diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go
index e616043b0..e685238fe 100644
--- a/pkg/consensus/consensus.go
+++ b/pkg/consensus/consensus.go
@@ -13,6 +13,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
 	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/hash"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
@@ -358,9 +359,21 @@ func (s *service) verifyBlock(b block.Block) bool {
 	}
 
 	var fee int64
+	var pool = mempool.New(len(coreb.Transactions))
+	var mainPool = s.Chain.GetMemPool()
 	for _, tx := range coreb.Transactions {
+		var err error
+
 		fee += tx.SystemFee
-		if err := s.Chain.VerifyTx(tx, coreb); err != nil {
+		if mainPool.ContainsKey(tx.Hash()) {
+			err = pool.Add(tx, s.Chain)
+			if err == nil {
+				continue
+			}
+		} else {
+			err = s.Chain.PoolTx(tx, pool)
+		}
+		if err != nil {
 			s.log.Warn("invalid transaction in proposed block",
 				zap.Stringer("hash", tx.Hash()),
 				zap.Error(err))
diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go
index bac3ea1ad..63685bd30 100644
--- a/pkg/core/blockchain.go
+++ b/pkg/core/blockchain.go
@@ -113,7 +113,7 @@ type Blockchain struct {
 	stopCh      chan struct{}
 	runToExitCh chan struct{}
 
-	memPool mempool.Pool
+	memPool *mempool.Pool
 
 	// This lock protects concurrent access to keyCache.
 	keyCacheLock sync.RWMutex
@@ -437,14 +437,20 @@ func (bc *Blockchain) AddBlock(block *block.Block) error {
 			return fmt.Errorf("block %s is invalid: %w", block.Hash().StringLE(), err)
 		}
 		if bc.config.VerifyTransactions {
+			var mp = mempool.New(len(block.Transactions))
 			for _, tx := range block.Transactions {
+				var err error
 				// Transactions are verified before adding them
 				// into the pool, so there is no point in doing
 				// it again even if we're verifying in-block transactions.
 				if bc.memPool.ContainsKey(tx.Hash()) {
-					continue
+					err = mp.Add(tx, bc)
+					if err == nil {
+						continue
+					}
+				} else {
+					err = bc.verifyAndPoolTx(tx, mp)
 				}
-				err := bc.VerifyTx(tx, block)
 				if err != nil {
 					return fmt.Errorf("transaction %s failed to verify: %w", tx.Hash().StringLE(), err)
 				}
@@ -1164,7 +1170,7 @@ func (bc *Blockchain) GetMaxBlockSystemFee() int64 {
 
 // GetMemPool returns the memory pool of the blockchain.
 func (bc *Blockchain) GetMemPool() *mempool.Pool {
-	return &bc.memPool
+	return bc.memPool
 }
 
 // ApplyPolicyToTxSet applies configured policies to given transaction set. It
@@ -1222,8 +1228,9 @@ var (
 	ErrTxInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
 )
 
-// verifyTx verifies whether a transaction is bonafide or not.
-func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) error {
+// verifyAndPoolTx verifies whether a transaction is bonafide or not and tries
+// to add it to the mempool given.
+func (bc *Blockchain) verifyAndPoolTx(t *transaction.Transaction, pool *mempool.Pool) error {
 	height := bc.BlockHeight()
 	if t.ValidUntilBlock <= height || t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement {
 		return fmt.Errorf("%w: ValidUntilBlock = %d, current height = %d", ErrTxExpired, t.ValidUntilBlock, height)
@@ -1233,11 +1240,6 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e
 		// Only one %w can be used.
 		return fmt.Errorf("%w: %v", ErrPolicy, err)
 	}
-	balance := bc.GetUtilityTokenBalance(t.Sender())
-	need := t.SystemFee + t.NetworkFee
-	if balance.Cmp(big.NewInt(need)) < 0 {
-		return fmt.Errorf("%w: balance is %v, need: %v", ErrInsufficientFunds, balance, need)
-	}
 	size := io.GetVarSize(t)
 	if size > transaction.MaxTransactionSize {
 		return fmt.Errorf("%w: (%d > MaxTransactionSize %d)", ErrTxTooBig, size, transaction.MaxTransactionSize)
@@ -1247,13 +1249,30 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e
 	if netFee < 0 {
 		return fmt.Errorf("%w: net fee is %v, need %v", ErrTxSmallNetworkFee, t.NetworkFee, needNetworkFee)
 	}
-	if block == nil {
-		if ok := bc.memPool.Verify(t, bc); !ok {
+	if bc.dao.HasTransaction(t.Hash()) {
+		return fmt.Errorf("blockchain: %w", ErrAlreadyExists)
+	}
+	err := bc.verifyTxWitnesses(t, nil)
+	if err != nil {
+		return err
+	}
+	err = pool.Add(t, bc)
+	if err != nil {
+		switch {
+		case errors.Is(err, mempool.ErrConflict):
 			return ErrMemPoolConflict
+		case errors.Is(err, mempool.ErrDup):
+			return fmt.Errorf("mempool: %w", ErrAlreadyExists)
+		case errors.Is(err, mempool.ErrInsufficientFunds):
+			return ErrInsufficientFunds
+		case errors.Is(err, mempool.ErrOOM):
+			return ErrOOM
+		default:
+			return err
 		}
 	}
 
-	return bc.verifyTxWitnesses(t, block)
+	return nil
 }
 
 // isTxStillRelevant is a callback for mempool transaction filtering after the
@@ -1360,38 +1379,31 @@ func (bc *Blockchain) verifyStateRootWitness(r *state.MPTRoot) error {
 		bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO))
 }
 
-// VerifyTx verifies whether a transaction is bonafide or not. Block parameter
-// is used for easy interop access and can be omitted for transactions that are
-// not yet added into any block.
-// Golang implementation of Verify method in C# (https://github.com/neo-project/neo/blob/master/neo/Network/P2P/Payloads/Transaction.cs#L270).
-func (bc *Blockchain) VerifyTx(t *transaction.Transaction, block *block.Block) error {
+// VerifyTx verifies whether transaction is bonafide or not relative to the
+// current blockchain state. Note that this verification is completely isolated
+// from the main node's mempool.
+func (bc *Blockchain) VerifyTx(t *transaction.Transaction) error {
+	var mp = mempool.New(1)
 	bc.lock.RLock()
 	defer bc.lock.RUnlock()
-	return bc.verifyTx(t, block)
+	return bc.verifyAndPoolTx(t, mp)
 }
 
-// PoolTx verifies and tries to add given transaction into the mempool.
-func (bc *Blockchain) PoolTx(t *transaction.Transaction) error {
+// PoolTx verifies and tries to add given transaction into the mempool. If not
+// given, the default mempool is used. Passing multiple pools is not supported.
+func (bc *Blockchain) PoolTx(t *transaction.Transaction, pools ...*mempool.Pool) error {
+	var pool = bc.memPool
+
 	bc.lock.RLock()
 	defer bc.lock.RUnlock()
-
-	if bc.HasTransaction(t.Hash()) {
-		return fmt.Errorf("blockchain: %w", ErrAlreadyExists)
+	// Programmer error.
+	if len(pools) > 1 {
+		panic("too many pools given")
 	}
-	if err := bc.verifyTx(t, nil); err != nil {
-		return err
+	if len(pools) == 1 {
+		pool = pools[0]
 	}
-	if err := bc.memPool.Add(t, bc); err != nil {
-		switch {
-		case errors.Is(err, mempool.ErrOOM):
-			return ErrOOM
-		case errors.Is(err, mempool.ErrDup):
-			return fmt.Errorf("mempool: %w", ErrAlreadyExists)
-		default:
-			return err
-		}
-	}
-	return nil
+	return bc.verifyAndPoolTx(t, pool)
 }
 
 //GetStandByValidators returns validators from the configuration.
diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go
index dbb589b4a..2bed8c7a7 100644
--- a/pkg/core/blockchain_test.go
+++ b/pkg/core/blockchain_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
 	"github.com/nspcc-dev/neo-go/pkg/core/block"
 	"github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames"
+	"github.com/nspcc-dev/neo-go/pkg/core/mempool"
 	"github.com/nspcc-dev/neo-go/pkg/core/state"
 	"github.com/nspcc-dev/neo-go/pkg/core/storage"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
@@ -271,7 +272,7 @@ func TestVerifyTx(t *testing.T) {
 	checkResult(t, res, stackitem.NewBool(true))
 
 	checkErr := func(t *testing.T, expectedErr error, tx *transaction.Transaction) {
-		err := bc.verifyTx(tx, nil)
+		err := bc.VerifyTx(tx)
 		fmt.Println(err)
 		require.True(t, errors.Is(err, expectedErr))
 	}
@@ -287,7 +288,7 @@ func TestVerifyTx(t *testing.T) {
 	t.Run("BlockedAccount", func(t *testing.T) {
 		tx := bc.newTestTx(accs[1].PrivateKey().GetScriptHash(), testScript)
 		require.NoError(t, accs[1].SignTx(tx))
-		err := bc.verifyTx(tx, nil)
+		err := bc.VerifyTx(tx)
 		require.True(t, errors.Is(err, ErrPolicy))
 	})
 	t.Run("InsufficientGas", func(t *testing.T) {
@@ -314,12 +315,13 @@ func TestVerifyTx(t *testing.T) {
 		tx := bc.newTestTx(h, testScript)
 		tx.NetworkFee = balance / 2
 		require.NoError(t, accs[0].SignTx(tx))
-		checkErr(t, nil, tx)
+		require.NoError(t, bc.PoolTx(tx))
 
 		tx2 := bc.newTestTx(h, testScript)
 		tx2.NetworkFee = balance / 2
-		require.NoError(t, bc.memPool.Add(tx2, bc))
-		checkErr(t, ErrMemPoolConflict, tx)
+		require.NoError(t, accs[0].SignTx(tx2))
+		err := bc.PoolTx(tx2)
+		require.True(t, errors.Is(err, ErrMemPoolConflict))
 	})
 	t.Run("NotEnoughWitnesses", func(t *testing.T) {
 		tx := bc.newTestTx(h, testScript)
@@ -337,6 +339,35 @@ func TestVerifyTx(t *testing.T) {
 		tx.Scripts[0].InvocationScript[10] = ^tx.Scripts[0].InvocationScript[10]
 		checkErr(t, ErrVerificationFailed, tx)
 	})
+	t.Run("OldTX", func(t *testing.T) {
+		tx := bc.newTestTx(h, testScript)
+		require.NoError(t, accs[0].SignTx(tx))
+		b := bc.newBlock(tx)
+		require.NoError(t, bc.AddBlock(b))
+
+		err := bc.VerifyTx(tx)
+		require.True(t, errors.Is(err, ErrAlreadyExists))
+	})
+	t.Run("MemPooledTX", func(t *testing.T) {
+		tx := bc.newTestTx(h, testScript)
+		require.NoError(t, accs[0].SignTx(tx))
+		require.NoError(t, bc.PoolTx(tx))
+
+		err := bc.PoolTx(tx)
+		require.True(t, errors.Is(err, ErrAlreadyExists))
+	})
+	t.Run("MemPoolOOM", func(t *testing.T) {
+		bc.memPool = mempool.New(1)
+		tx1 := bc.newTestTx(h, testScript)
+		tx1.NetworkFee += 10000 // Give it more priority.
+		require.NoError(t, accs[0].SignTx(tx1))
+		require.NoError(t, bc.PoolTx(tx1))
+
+		tx2 := bc.newTestTx(h, testScript)
+		require.NoError(t, accs[0].SignTx(tx2))
+		err := bc.PoolTx(tx2)
+		require.True(t, errors.Is(err, ErrOOM))
+	})
 }
 
 func TestVerifyHashAgainstScript(t *testing.T) {
diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go
index 34f4f74c0..dea4412b6 100644
--- a/pkg/core/blockchainer/blockchainer.go
+++ b/pkg/core/blockchainer/blockchainer.go
@@ -51,12 +51,12 @@ type Blockchainer interface {
 	mempool.Feer // fee interface
 	GetMaxBlockSize() uint32
 	GetMaxBlockSystemFee() int64
-	PoolTx(*transaction.Transaction) error
+	PoolTx(t *transaction.Transaction, pools ...*mempool.Pool) error
 	SubscribeForBlocks(ch chan<- *block.Block)
 	SubscribeForExecutions(ch chan<- *state.AppExecResult)
 	SubscribeForNotifications(ch chan<- *state.NotificationEvent)
 	SubscribeForTransactions(ch chan<- *transaction.Transaction)
-	VerifyTx(*transaction.Transaction, *block.Block) error
+	VerifyTx(*transaction.Transaction) error
 	GetMemPool() *mempool.Pool
 	UnsubscribeFromBlocks(ch chan<- *block.Block)
 	UnsubscribeFromExecutions(ch chan<- *state.AppExecResult)
diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go
index b4798d004..521074336 100644
--- a/pkg/core/mempool/mem_pool.go
+++ b/pkg/core/mempool/mem_pool.go
@@ -12,6 +12,10 @@ import (
 )
 
 var (
+	// ErrInsufficientFunds is returned when Sender is not able to pay for
+	// transaction being added irrespective of the other contents of the
+	// pool.
+	ErrInsufficientFunds = errors.New("insufficient funds")
 	// ErrConflict is returned when transaction being added is incompatible
 	// with the contents of the memory pool (Sender doesn't have enough GAS
 	// to pay for all transactions in the pool).
@@ -114,7 +118,7 @@ func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer, needChe
 		senderFee.balance = feer.GetUtilityTokenBalance(tx.Sender())
 		mp.fees[tx.Sender()] = senderFee
 	}
-	if needCheck && !checkBalance(tx, senderFee) {
+	if needCheck && checkBalance(tx, senderFee) != nil {
 		return false
 	}
 	senderFee.feeSum += tx.SystemFee + tx.NetworkFee
@@ -122,11 +126,18 @@ func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer, needChe
 	return true
 }
 
-// checkBalance returns true in case when sender has enough GAS to pay for the
+// checkBalance returns nil in case when sender has enough GAS to pay for the
 // transaction
-func checkBalance(tx *transaction.Transaction, balance utilityBalanceAndFees) bool {
-	needFee := balance.feeSum + tx.SystemFee + tx.NetworkFee
-	return balance.balance.Cmp(big.NewInt(needFee)) >= 0
+func checkBalance(tx *transaction.Transaction, balance utilityBalanceAndFees) error {
+	txFee := tx.SystemFee + tx.NetworkFee
+	if balance.balance.Cmp(big.NewInt(txFee)) < 0 {
+		return ErrInsufficientFunds
+	}
+	needFee := balance.feeSum + txFee
+	if balance.balance.Cmp(big.NewInt(needFee)) < 0 {
+		return ErrConflict
+	}
+	return nil
 }
 
 // Add tries to add given transaction to the Pool.
@@ -140,9 +151,10 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer) error {
 		mp.lock.Unlock()
 		return ErrDup
 	}
-	if !mp.checkTxConflicts(t, fee) {
+	err := mp.checkTxConflicts(t, fee)
+	if err != nil {
 		mp.lock.Unlock()
-		return ErrConflict
+		return err
 	}
 
 	mp.verifiedMap[t.Hash()] = pItem
@@ -248,8 +260,8 @@ func (mp *Pool) checkPolicy(tx *transaction.Transaction, policyChanged bool) boo
 }
 
 // New returns a new Pool struct.
-func New(capacity int) Pool {
-	return Pool{
+func New(capacity int) *Pool {
+	return &Pool{
 		verifiedMap:  make(map[util.Uint256]*item),
 		verifiedTxes: make([]*item, 0, capacity),
 		capacity:     capacity,
@@ -283,7 +295,7 @@ func (mp *Pool) GetVerifiedTransactions() []*transaction.Transaction {
 }
 
 // checkTxConflicts is an internal unprotected version of Verify.
-func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) bool {
+func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) error {
 	senderFee, ok := mp.fees[tx.Sender()]
 	if !ok {
 		senderFee.balance = fee.GetUtilityTokenBalance(tx.Sender())
@@ -298,5 +310,5 @@ func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) bool {
 func (mp *Pool) Verify(tx *transaction.Transaction, feer Feer) bool {
 	mp.lock.RLock()
 	defer mp.lock.RUnlock()
-	return mp.checkTxConflicts(tx, feer)
+	return mp.checkTxConflicts(tx, feer) == nil
 }
diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go
index eb6649b60..8c8d67d3c 100644
--- a/pkg/network/helper_test.go
+++ b/pkg/network/helper_test.go
@@ -149,7 +149,7 @@ func (chain testChain) GetUtilityTokenBalance(uint160 util.Uint160) *big.Int {
 	panic("TODO")
 }
 
-func (chain testChain) PoolTx(*transaction.Transaction) error {
+func (chain testChain) PoolTx(*transaction.Transaction, ...*mempool.Pool) error {
 	panic("TODO")
 }
 
@@ -166,7 +166,7 @@ func (chain testChain) SubscribeForTransactions(ch chan<- *transaction.Transacti
 	panic("TODO")
 }
 
-func (chain testChain) VerifyTx(*transaction.Transaction, *block.Block) error {
+func (chain testChain) VerifyTx(*transaction.Transaction) error {
 	panic("TODO")
 }
 

From d9b8704b48749b2026844644085ca59f16ac6978 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Wed, 19 Aug 2020 19:38:50 +0300
Subject: [PATCH 07/10] consensus: check for chain's height in verifyBlock

We may already be behind and this check could be irrelevant.
---
 pkg/consensus/consensus.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go
index e685238fe..4971fd52d 100644
--- a/pkg/consensus/consensus.go
+++ b/pkg/consensus/consensus.go
@@ -349,6 +349,10 @@ func (s *service) getTx(h util.Uint256) block.Transaction {
 func (s *service) verifyBlock(b block.Block) bool {
 	coreb := &b.(*neoBlock).Block
 
+	if s.Chain.BlockHeight() >= coreb.Index {
+		s.log.Warn("proposed block has already outdated")
+		return false
+	}
 	maxBlockSize := int(s.Chain.GetMaxBlockSize())
 	size := io.GetVarSize(coreb)
 	if size > maxBlockSize {
@@ -379,6 +383,10 @@ func (s *service) verifyBlock(b block.Block) bool {
 				zap.Error(err))
 			return false
 		}
+		if s.Chain.BlockHeight() >= coreb.Index {
+			s.log.Warn("proposed block has already outdated")
+			return false
+		}
 	}
 
 	maxBlockSysFee := s.Chain.GetMaxBlockSystemFee()

From 7fedb4f4ba5d06d61facbf152efc683fdcb276fe Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Thu, 20 Aug 2020 18:47:14 +0300
Subject: [PATCH 08/10] testchain: move newBlock there from rpc/server

Allow its reuse by other components.
---
 pkg/internal/testchain/address.go   | 36 +++++++++++++++++++++++++++++
 pkg/rpc/server/server_test.go       | 34 +++------------------------
 pkg/rpc/server/subscription_test.go |  4 ++--
 3 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/pkg/internal/testchain/address.go b/pkg/internal/testchain/address.go
index 27cc58d6a..e3cd1ca0f 100644
--- a/pkg/internal/testchain/address.go
+++ b/pkg/internal/testchain/address.go
@@ -1,6 +1,12 @@
 package testchain
 
 import (
+	"testing"
+	"time"
+
+	"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/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/address"
@@ -8,6 +14,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract"
 	"github.com/nspcc-dev/neo-go/pkg/util"
 	"github.com/nspcc-dev/neo-go/pkg/vm/emit"
+	"github.com/stretchr/testify/require"
 )
 
 // privNetKeys is a list of unencrypted WIFs sorted by public key.
@@ -94,3 +101,32 @@ func Sign(data []byte) []byte {
 	}
 	return buf.Bytes()
 }
+
+// NewBlock creates new block for the given blockchain with the given offset
+// (usually, 1), primary node index and transactions.
+func NewBlock(t *testing.T, bc blockchainer.Blockchainer, offset uint32, primary uint32, txs ...*transaction.Transaction) *block.Block {
+	witness := transaction.Witness{VerificationScript: MultisigVerificationScript()}
+	height := bc.BlockHeight()
+	h := bc.GetHeaderHash(int(height))
+	hdr, err := bc.GetHeader(h)
+	require.NoError(t, err)
+	b := &block.Block{
+		Base: block.Base{
+			PrevHash:      hdr.Hash(),
+			Timestamp:     (uint64(time.Now().UTC().Unix()) + uint64(hdr.Index)) * 1000,
+			Index:         hdr.Index + offset,
+			NextConsensus: witness.ScriptHash(),
+			Script:        witness,
+			Network:       bc.GetConfig().Magic,
+		},
+		ConsensusData: block.ConsensusData{
+			PrimaryIndex: primary,
+			Nonce:        1111,
+		},
+		Transactions: txs,
+	}
+	_ = b.RebuildMerkleRoot()
+
+	b.Script.InvocationScript = Sign(b.GetSignedPart())
+	return b
+}
diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go
index d0a759625..eb7cdd273 100644
--- a/pkg/rpc/server/server_test.go
+++ b/pkg/rpc/server/server_test.go
@@ -18,7 +18,6 @@ import (
 	"github.com/gorilla/websocket"
 	"github.com/nspcc-dev/neo-go/pkg/core"
 	"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/state"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/address"
@@ -729,7 +728,7 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
 	t.Run("submit", func(t *testing.T) {
 		rpc := `{"jsonrpc": "2.0", "id": 1, "method": "submitblock", "params": ["%s"]}`
 		t.Run("invalid signature", func(t *testing.T) {
-			s := newBlock(t, chain, 1, 0)
+			s := testchain.NewBlock(t, chain, 1, 0)
 			s.Script.VerificationScript[8] ^= 0xff
 			body := doRPCCall(fmt.Sprintf(rpc, encodeBlock(t, s)), httpSrv.URL, t)
 			checkErrGetResult(t, body, true)
@@ -759,13 +758,13 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) []
 		}
 
 		t.Run("invalid height", func(t *testing.T) {
-			b := newBlock(t, chain, 2, 0, newTx())
+			b := testchain.NewBlock(t, chain, 2, 0, newTx())
 			body := doRPCCall(fmt.Sprintf(rpc, encodeBlock(t, b)), httpSrv.URL, t)
 			checkErrGetResult(t, body, true)
 		})
 
 		t.Run("positive", func(t *testing.T) {
-			b := newBlock(t, chain, 1, 0, newTx())
+			b := testchain.NewBlock(t, chain, 1, 0, newTx())
 			body := doRPCCall(fmt.Sprintf(rpc, encodeBlock(t, b)), httpSrv.URL, t)
 			data := checkErrGetResult(t, body, false)
 			var res = new(result.RelayResult)
@@ -934,33 +933,6 @@ func encodeBlock(t *testing.T, b *block.Block) string {
 	return hex.EncodeToString(w.Bytes())
 }
 
-func newBlock(t *testing.T, bc blockchainer.Blockchainer, index uint32, primary uint32, txs ...*transaction.Transaction) *block.Block {
-	witness := transaction.Witness{VerificationScript: testchain.MultisigVerificationScript()}
-	height := bc.BlockHeight()
-	h := bc.GetHeaderHash(int(height))
-	hdr, err := bc.GetHeader(h)
-	require.NoError(t, err)
-	b := &block.Block{
-		Base: block.Base{
-			PrevHash:      hdr.Hash(),
-			Timestamp:     (uint64(time.Now().UTC().Unix()) + uint64(hdr.Index)) * 1000,
-			Index:         hdr.Index + index,
-			NextConsensus: witness.ScriptHash(),
-			Script:        witness,
-			Network:       bc.GetConfig().Magic,
-		},
-		ConsensusData: block.ConsensusData{
-			PrimaryIndex: primary,
-			Nonce:        1111,
-		},
-		Transactions: txs,
-	}
-	_ = b.RebuildMerkleRoot()
-
-	b.Script.InvocationScript = testchain.Sign(b.GetSignedPart())
-	return b
-}
-
 func (tc rpcTestCase) getResultPair(e *executor) (expected interface{}, res interface{}) {
 	expected = tc.result(e)
 	resVal := reflect.New(reflect.TypeOf(expected).Elem())
diff --git a/pkg/rpc/server/subscription_test.go b/pkg/rpc/server/subscription_test.go
index 69377b6ac..0cb1c8197 100644
--- a/pkg/rpc/server/subscription_test.go
+++ b/pkg/rpc/server/subscription_test.go
@@ -284,7 +284,7 @@ func TestFilteredBlockSubscriptions(t *testing.T) {
 		if primary == 3 {
 			expectedCnt++
 		}
-		b := newBlock(t, chain, 1, primary)
+		b := testchain.NewBlock(t, chain, 1, primary)
 		require.NoError(t, chain.AddBlock(b))
 	}
 
@@ -437,7 +437,7 @@ func testSubscriptionOverflow(t *testing.T) {
 
 	// Push a lot of new blocks, but don't read events for them.
 	for i := 0; i < blockCnt; i++ {
-		b := newBlock(t, chain, 1, 0)
+		b := testchain.NewBlock(t, chain, 1, 0)
 		require.NoError(t, chain.AddBlock(b))
 	}
 	for i := 0; i < blockCnt; i++ {

From 95a80c6922c57bb59541309b158efa131b9740b7 Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Thu, 20 Aug 2020 18:47:52 +0300
Subject: [PATCH 09/10] consensus: add verifyBlock tests

Especially the one for #668.
---
 pkg/consensus/consensus_test.go | 78 +++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/pkg/consensus/consensus_test.go b/pkg/consensus/consensus_test.go
index acf219fe1..6a34a1269 100644
--- a/pkg/consensus/consensus_test.go
+++ b/pkg/consensus/consensus_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/config"
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
 	"github.com/nspcc-dev/neo-go/pkg/core"
+	"github.com/nspcc-dev/neo-go/pkg/core/native"
 	"github.com/nspcc-dev/neo-go/pkg/core/storage"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
@@ -193,6 +194,83 @@ func TestService_OnPayload(t *testing.T) {
 	srv.Chain.Close()
 }
 
+func TestVerifyBlock(t *testing.T) {
+	srv := newTestService(t)
+	defer srv.Chain.Close()
+	t.Run("good empty", func(t *testing.T) {
+		b := testchain.NewBlock(t, srv.Chain, 1, 0)
+		require.True(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("good pooled tx", func(t *testing.T) {
+		tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 100000)
+		tx.ValidUntilBlock = 1
+		addSender(t, tx)
+		signTx(t, srv.Chain.FeePerByte(), tx)
+		require.NoError(t, srv.Chain.PoolTx(tx))
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, tx)
+		require.True(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("good non-pooled tx", func(t *testing.T) {
+		tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 100000)
+		tx.ValidUntilBlock = 1
+		addSender(t, tx)
+		signTx(t, srv.Chain.FeePerByte(), tx)
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, tx)
+		require.True(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("good conflicting tx", func(t *testing.T) {
+		tx1 := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 100000)
+		tx1.NetworkFee = 20_000_000 * native.GASFactor
+		tx1.ValidUntilBlock = 1
+		addSender(t, tx1)
+		signTx(t, srv.Chain.FeePerByte(), tx1)
+		tx2 := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 100000)
+		tx2.NetworkFee = 20_000_000 * native.GASFactor
+		tx2.ValidUntilBlock = 1
+		addSender(t, tx2)
+		signTx(t, srv.Chain.FeePerByte(), tx2)
+		require.NoError(t, srv.Chain.PoolTx(tx1))
+		require.Error(t, srv.Chain.PoolTx(tx2))
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, tx2)
+		require.True(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("bad old", func(t *testing.T) {
+		b := testchain.NewBlock(t, srv.Chain, 1, 0)
+		b.Index = srv.Chain.BlockHeight()
+		require.False(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("bad big size", func(t *testing.T) {
+		script := make([]byte, int(srv.Chain.GetMaxBlockSize()))
+		script[0] = byte(opcode.RET)
+		tx := transaction.New(netmode.UnitTestNet, script, 100000)
+		tx.ValidUntilBlock = 1
+		addSender(t, tx)
+		signTx(t, srv.Chain.FeePerByte(), tx)
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, tx)
+		require.False(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("bad tx", func(t *testing.T) {
+		tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 100000)
+		tx.ValidUntilBlock = 1
+		addSender(t, tx)
+		signTx(t, srv.Chain.FeePerByte(), tx)
+		tx.Scripts[0].InvocationScript[16] = ^tx.Scripts[0].InvocationScript[16]
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, tx)
+		require.False(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+	t.Run("bad big sys fee", func(t *testing.T) {
+		txes := make([]*transaction.Transaction, 2)
+		for i := range txes {
+			txes[i] = transaction.New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, srv.Chain.GetMaxBlockSystemFee()/2+1)
+			txes[i].ValidUntilBlock = 1
+			addSender(t, txes[i])
+			signTx(t, srv.Chain.FeePerByte(), txes[i])
+		}
+		b := testchain.NewBlock(t, srv.Chain, 1, 0, txes...)
+		require.False(t, srv.verifyBlock(&neoBlock{Block: *b}))
+	})
+}
+
 func shouldReceive(t *testing.T, ch chan Payload) {
 	select {
 	case <-ch:

From 0e086d61ac701952855543870a5231e59771490e Mon Sep 17 00:00:00 2001
From: Roman Khimov <roman@nspcc.ru>
Date: Thu, 20 Aug 2020 19:06:59 +0300
Subject: [PATCH 10/10] mempool: store feeSum as big.Int

Prevent (very) potential overflow.
---
 pkg/core/mempool/mem_pool.go      | 16 +++++++++-------
 pkg/core/mempool/mem_pool_test.go |  8 ++++----
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go
index 521074336..2ccf4001d 100644
--- a/pkg/core/mempool/mem_pool.go
+++ b/pkg/core/mempool/mem_pool.go
@@ -41,7 +41,7 @@ type items []*item
 // sender's transactions which are currently in mempool
 type utilityBalanceAndFees struct {
 	balance *big.Int
-	feeSum  int64
+	feeSum  *big.Int
 }
 
 // Pool stores the unconfirms transactions.
@@ -116,12 +116,13 @@ func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer, needChe
 	senderFee, ok := mp.fees[tx.Sender()]
 	if !ok {
 		senderFee.balance = feer.GetUtilityTokenBalance(tx.Sender())
+		senderFee.feeSum = big.NewInt(0)
 		mp.fees[tx.Sender()] = senderFee
 	}
 	if needCheck && checkBalance(tx, senderFee) != nil {
 		return false
 	}
-	senderFee.feeSum += tx.SystemFee + tx.NetworkFee
+	senderFee.feeSum.Add(senderFee.feeSum, big.NewInt(tx.SystemFee+tx.NetworkFee))
 	mp.fees[tx.Sender()] = senderFee
 	return true
 }
@@ -129,12 +130,12 @@ func (mp *Pool) tryAddSendersFee(tx *transaction.Transaction, feer Feer, needChe
 // checkBalance returns nil in case when sender has enough GAS to pay for the
 // transaction
 func checkBalance(tx *transaction.Transaction, balance utilityBalanceAndFees) error {
-	txFee := tx.SystemFee + tx.NetworkFee
-	if balance.balance.Cmp(big.NewInt(txFee)) < 0 {
+	txFee := big.NewInt(tx.SystemFee + tx.NetworkFee)
+	if balance.balance.Cmp(txFee) < 0 {
 		return ErrInsufficientFunds
 	}
-	needFee := balance.feeSum + txFee
-	if balance.balance.Cmp(big.NewInt(needFee)) < 0 {
+	needFee := txFee.Add(txFee, balance.feeSum)
+	if balance.balance.Cmp(needFee) < 0 {
 		return ErrConflict
 	}
 	return nil
@@ -212,7 +213,7 @@ func (mp *Pool) Remove(hash util.Uint256) {
 			mp.verifiedTxes = mp.verifiedTxes[:num]
 		}
 		senderFee := mp.fees[it.txn.Sender()]
-		senderFee.feeSum -= it.txn.SystemFee + it.txn.NetworkFee
+		senderFee.feeSum.Sub(senderFee.feeSum, big.NewInt(it.txn.SystemFee+it.txn.NetworkFee))
 		mp.fees[it.txn.Sender()] = senderFee
 	}
 	updateMempoolMetrics(len(mp.verifiedTxes))
@@ -299,6 +300,7 @@ func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) error {
 	senderFee, ok := mp.fees[tx.Sender()]
 	if !ok {
 		senderFee.balance = fee.GetUtilityTokenBalance(tx.Sender())
+		senderFee.feeSum = big.NewInt(0)
 	}
 	return checkBalance(tx, senderFee)
 }
diff --git a/pkg/core/mempool/mem_pool_test.go b/pkg/core/mempool/mem_pool_test.go
index a7420b255..0744f25ef 100644
--- a/pkg/core/mempool/mem_pool_test.go
+++ b/pkg/core/mempool/mem_pool_test.go
@@ -205,7 +205,7 @@ func TestMemPoolFees(t *testing.T) {
 	require.Equal(t, 1, len(mp.fees))
 	require.Equal(t, utilityBalanceAndFees{
 		balance: balance,
-		feeSum:  tx1.NetworkFee,
+		feeSum:  big.NewInt(tx1.NetworkFee),
 	}, mp.fees[sender0])
 
 	// balance shouldn't change after adding one more transaction
@@ -217,7 +217,7 @@ func TestMemPoolFees(t *testing.T) {
 	require.Equal(t, 1, len(mp.fees))
 	require.Equal(t, utilityBalanceAndFees{
 		balance: balance,
-		feeSum:  balance.Int64(),
+		feeSum:  balance,
 	}, mp.fees[sender0])
 
 	// can't add more transactions as we don't have enough GAS
@@ -229,7 +229,7 @@ func TestMemPoolFees(t *testing.T) {
 	require.Equal(t, 1, len(mp.fees))
 	require.Equal(t, utilityBalanceAndFees{
 		balance: balance,
-		feeSum:  balance.Int64(),
+		feeSum:  balance,
 	}, mp.fees[sender0])
 
 	// check whether sender's fee updates correctly
@@ -242,7 +242,7 @@ func TestMemPoolFees(t *testing.T) {
 	require.Equal(t, 1, len(mp.fees))
 	require.Equal(t, utilityBalanceAndFees{
 		balance: balance,
-		feeSum:  tx2.NetworkFee,
+		feeSum:  big.NewInt(tx2.NetworkFee),
 	}, mp.fees[sender0])
 
 	// there should be nothing left