block: remove Verify()

It's used in two places now:
 * Blockchain.AddBlock()
   This one does transaction duplication check of its own, doing it in
   Verify() is just a waste of time. Merkle tree root hash value check is
   still relevant though
 * Block.DecodeBinary()
   We're decoding blocks for the following purposes:
     - on restore from dump
       The block will be added to the chain via AddBlock() and that will do a
       full check of it (if configured to do so)
     - on retrieving the block from the DB (DAO)
       We trust the DB, if it's gone wild, this check won't really help
     - on receiving the block via P2P
       It's gonna be put into block queue and then end up in AddBlock() which
       will check it
     - on receiving the block via RPC (submitblock)
       It is to be passed into AddBlock()
     - on receiving the block via RPC in a client
       That's the only problematic case probably, but RPC client has to trust
       the server and it can check for the signature if it really
       cares. Or a separate in-client check might be added.

As we can see nothing really requires this verification to be done the way it
is now, AddBlock can just have a Merkle check and DecodeBinary can do fine
without it at all.

pkg/core/block/block.go

@@ -46,8 +46,8 @@ func (b *Block) Header() *Header {
 	}
 }
 
-// computeMerkleTree computes Merkle tree based on actual block's data.
-func (b *Block) computeMerkleTree() util.Uint256 {
+// ComputeMerkleRoot computes Merkle tree root hash based on actual block's data.
+func (b *Block) ComputeMerkleRoot() util.Uint256 {
 	hashes := make([]util.Uint256, len(b.Transactions)+1)
 	hashes[0] = b.ConsensusData.Hash()
 	for i, tx := range b.Transactions {
@@ -59,27 +59,7 @@ func (b *Block) computeMerkleTree() util.Uint256 {
 
 // RebuildMerkleRoot rebuilds the merkleroot of the block.
 func (b *Block) RebuildMerkleRoot() {
-	b.MerkleRoot = b.computeMerkleTree()
-}
-
-// Verify verifies the integrity of the block.
-func (b *Block) Verify() error {
-	if b.Transactions != nil {
-		hashes := map[util.Uint256]bool{}
-		for _, tx := range b.Transactions {
-			if !hashes[tx.Hash()] {
-				hashes[tx.Hash()] = true
-			} else {
-				return errors.New("transaction duplication is not allowed")
-			}
-		}
-	}
-
-	merkle := b.computeMerkleTree()
-	if !b.MerkleRoot.Equals(merkle) {
-		return errors.New("MerkleRoot mismatch")
-	}
-	return nil
+	b.MerkleRoot = b.ComputeMerkleRoot()
 }
 
 // NewBlockFromTrimmedBytes returns a new block from trimmed data.
@@ -173,7 +153,6 @@ func (b *Block) DecodeBinary(br *io.BinReader) {
 	if br.Err != nil {
 		return
 	}
-	br.Err = b.Verify()
 }
 
 // EncodeBinary encodes the block to the given BinWriter, implementing

pkg/core/block/block_test.go

@@ -108,17 +108,6 @@ func TestHashBlockEqualsHashHeader(t *testing.T) {
 	assert.Equal(t, block.Hash(), block.Header().Hash())
 }
 
-func TestBlockVerify(t *testing.T) {
-	block := newDumbBlock()
-	assert.NotNil(t, block.Verify())
-	block.RebuildMerkleRoot()
-	assert.Nil(t, block.Verify())
-
-	block.Transactions = []*transaction.Transaction{}
-	block.RebuildMerkleRoot()
-	assert.Nil(t, block.Verify())
-}
-
 func TestBinBlockDecodeEncode(t *testing.T) {
 	// transaction taken from mainnet: 2000000
 	rawtx := "0000000005440c786a66aaebf472aacb1d1db19d5b494c6a9226ea91bf5cf0e63a6605138cde5064efb81bc6539620b9e6d6d7c74f97d415b922c4fb4bb1833ce6a97a9d61f962fb7301000065f000005d12ac6c589d59f92e82d8bf60659cb716ffc1f101fd4a010c4011ff5d2138cf546d112ef712ee8a15277f7b6f1d5d2564b97497ac155782e6089cd3005dc9de81a8b22bb2f1c3a2edbac55e01581cb27980fdedf3a8bc57fa470c40657253c374a48da773fc653591f282a63a60695f29ab6c86300020ed505a019e5563e1be493efa71bdde37b16b4ec3f5f6dc2d2a2550151b020176b4dbe7afe40c403efdc559cb6bff135fd79138267db897c6fded01e3a0f15c0fb1c337359935d65e7ac49239f020951a74a96e11e73d225c9789953ffec40d5f7c9a84707b1d9a0c402804f24ab8034fa41223977ba48883eb94951184e31e5739872daf4f65461de3196ebf333f6d7dc4aff0b7b2143793179415f50a715484aba4e33b97dc636e150c40ed6b2ffeaef97eef746815ad16f5b8aed743892e93f7216bb744eb5c2f4cad91ae291919b61cd9a8d50fe85630d5e010c49a01ed687727c3ae5a7e17d4da213afdfd00150c2103009b7540e10f2562e5fd8fac9eaec25166a58b26e412348ff5a86927bfac22a20c21030205e9cefaea5a1dfc580af20c8d5aa2468bb0148f1a5e4605fc622c80e604ba0c210214baf0ceea3a66f17e7e1e839ea25fd8bed6cd82e6bb6e68250189065f44ff010c2103408dcd416396f64783ac587ea1e1593c57d9fea880c8a6a1920e92a2594778060c2102a7834be9b32e2981d157cb5bbd3acb42cfd11ea5c3b10224d7a44e98c5910f1b0c2102ba2c70f5996f357a43198705859fae2cfea13e1172962800772b3d588a9d4abd0c2102f889ecd43c5126ff1932d75fa87dea34fc95325fb724db93c8f79fe32cc3f180170b41138defaf0202c1353ed4e94d0cbc00be80024f7673890000000000261c130000000000e404210001f813c2cc8e18bbe4b3b87f8ef9105b50bb93918e01005d0300743ba40b0000000c14aa07cc3f2193a973904a09a6e60b87f1f96273970c14f813c2cc8e18bbe4b3b87f8ef9105b50bb93918e13c00c087472616e736665720c14bcaf41d684c7d4ad6ee0d99da9707b9d1f0c8e6641627d5b523801420c402360bbf64b9644c25f066dbd406454b07ab9f56e8e25d92d90c96c598f6c29d97eabdcf226f3575481662cfcdd064ee410978e5fae3f09a2f83129ba9cd82641290c2103caf763f91d3691cba5b5df3eb13e668fdace0295b37e2e259fd0fb152d354f900b4195440d78"
@@ -157,6 +146,9 @@ func TestBinBlockDecodeEncode(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, rawtx, hex.EncodeToString(data))
 
+	// update hidden hash value.
+	_ = b.ConsensusData.Hash()
+
 	testserdes.MarshalUnmarshalJSON(t, b, New(netmode.TestNet))
 }
 

pkg/core/blockchain.go

@@ -423,9 +423,9 @@ func (bc *Blockchain) AddBlock(block *block.Block) error {
 		}
 	}
 	if bc.config.VerifyBlocks {
-		err := block.Verify()
-		if err != nil {
-			return fmt.Errorf("block %s is invalid: %w", block.Hash().StringLE(), err)
+		merkle := block.ComputeMerkleRoot()
+		if !block.MerkleRoot.Equals(merkle) {
+			return errors.New("invalid block: MerkleRoot mismatch")
 		}
 		mp = mempool.New(len(block.Transactions))
 		for _, tx := range block.Transactions {

pkg/rpc/client/rpc_test.go

@@ -161,7 +161,10 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{
 			},
 			serverResponse: b1Verbose,
 			result: func(c *Client) interface{} {
-				return getResultBlock1()
+				res := getResultBlock1()
+				// update hidden hash value.
+				_ = res.Block.ConsensusData.Hash()
+				return res
 			},
 		},
 		{
@@ -190,7 +193,10 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{
 			},
 			serverResponse: b1Verbose,
 			result: func(c *Client) interface{} {
-				return getResultBlock1()
+				res := getResultBlock1()
+				// update hidden hash value.
+				_ = res.Block.ConsensusData.Hash()
+				return res
 			},
 		},
 	},