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] 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}