core: reduce calls to Application.VerifyBlock (#247)

* remove sanity check when adding blocks.
* call VerifyBlock after lattice's sanity check.
* remove checkRelation flag.

4 files changed, 24 insertions, 28 deletions

diff --git a/core/consensus.go b/core/consensus.go
index 9e6768c..46727c4 100644
--- a/core/consensus.go
+++ b/core/consensus.go
@@ -818,7 +818,7 @@ func (con *Consensus) ProcessBlockRandomnessResult(
 
 // preProcessBlock performs Byzantine Agreement on the block.
 func (con *Consensus) preProcessBlock(b *types.Block) (err error) {
-	if err = con.lattice.SanityCheck(b, true); err != nil {
+	if err = con.lattice.SanityCheck(b); err != nil {
 		return
 	}
 	if err = con.baModules[b.Position.ChainID].processBlock(b); err != nil {
@@ -863,7 +863,7 @@ func (con *Consensus) processBlock(block *types.Block) (err error) {
 
 // processFinalizedBlock is the entry point for syncing blocks.
 func (con *Consensus) processFinalizedBlock(block *types.Block) (err error) {
-	if err = con.lattice.SanityCheck(block, false); err != nil {
+	if err = con.lattice.SanityCheck(block); err != nil {
 		return
 	}
 	con.ccModule.processFinalizedBlock(block)
diff --git a/core/lattice-data.go b/core/lattice-data.go
index 31604a6..b0fe9cf 100644
--- a/core/lattice-data.go
+++ b/core/lattice-data.go
@@ -268,11 +268,6 @@ func (data *latticeData) addBlock(
 		bAck    *types.Block
 		updated bool
 	)
-	// TODO(mission): sanity check twice, might hurt performance.
-	// If a block does not pass sanity check, report error.
-	if err = data.sanityCheck(block); err != nil {
-		return
-	}
 	if err = data.chains[block.Position.ChainID].addBlock(block); err != nil {
 		return
 	}
diff --git a/core/lattice.go b/core/lattice.go
index 0357a8d..3259f35 100644
--- a/core/lattice.go
+++ b/core/lattice.go
@@ -104,11 +104,10 @@ func (s *Lattice) PrepareEmptyBlock(b *types.Block) (err error) {
 }
 
 // SanityCheck check if a block is valid.
-// If checkRelation is true, it also checks with current lattice status.
 //
 // If some acking blocks don't exists, Lattice would help to cache this block
 // and retry when lattice updated in Lattice.ProcessBlock.
-func (s *Lattice) SanityCheck(b *types.Block, checkRelation bool) (err error) {
+func (s *Lattice) SanityCheck(b *types.Block) (err error) {
 	if b.IsEmpty() {
 		// Only need to verify block's hash.
 		var hash common.Hash
@@ -134,6 +133,22 @@ func (s *Lattice) SanityCheck(b *types.Block, checkRelation bool) (err error) {
 			return
 		}
 	}
+	if err = func() (err error) {
+		s.lock.RLock()
+		defer s.lock.RUnlock()
+		if err = s.data.sanityCheck(b); err != nil {
+			// Add to block pool, once the lattice updated,
+			// would be checked again.
+			if err == ErrAckingBlockNotExists {
+				s.pool.addBlock(b)
+			}
+			s.logger.Error("Sanity Check failed", "error", err)
+			return
+		}
+		return
+	}(); err != nil {
+		return
+	}
 	// Verify data in application layer.
 	s.logger.Debug("Calling Application.VerifyBlock", "block", b)
 	// TODO(jimmy-dexon): handle types.VerifyRetryLater.
@@ -141,20 +156,6 @@ func (s *Lattice) SanityCheck(b *types.Block, checkRelation bool) (err error) {
 		err = ErrInvalidBlock
 		return err
 	}
-	if !checkRelation {
-		return
-	}
-	s.lock.RLock()
-	defer s.lock.RUnlock()
-	if err = s.data.sanityCheck(b); err != nil {
-		// Add to block pool, once the lattice updated,
-		// would be checked again.
-		if err == ErrAckingBlockNotExists {
-			s.pool.addBlock(b)
-		}
-		s.logger.Error("Sanity Check failed", "error", err)
-		return
-	}
 	return
 }
 
diff --git a/core/lattice_test.go b/core/lattice_test.go
index a508f2b..8fac592 100644
--- a/core/lattice_test.go
+++ b/core/lattice_test.go
@@ -56,7 +56,7 @@ func (mgr *testLatticeMgr) processBlock(b *types.Block) (err error) {
 		verified  []*types.Block
 		pendings  = []*types.Block{b}
 	)
-	if err = mgr.lattice.SanityCheck(b, true); err != nil {
+	if err = mgr.lattice.SanityCheck(b); err != nil {
 		if err == ErrAckingBlockNotExists {
 			err = nil
 		}
@@ -225,11 +225,11 @@ func (s *LatticeTestSuite) TestSanityCheck() {
 		Timestamp: time.Now().UTC(),
 	}
 	req.NoError(auth.SignBlock(b))
-	req.NoError(lattice.SanityCheck(b, true))
+	req.NoError(lattice.SanityCheck(b))
 	// A block with incorrect signature should not pass sanity check.
 	b.Signature, err = auth.prvKey.Sign(common.NewRandomHash())
 	req.NoError(err)
-	req.Equal(lattice.SanityCheck(b, false), ErrIncorrectSignature)
+	req.Equal(lattice.SanityCheck(b), ErrIncorrectSignature)
 	// A block with un-sorted acks should not pass sanity check.
 	b.Acks = common.NewSortedHashes(common.Hashes{
 		common.NewRandomHash(),
@@ -240,10 +240,10 @@ func (s *LatticeTestSuite) TestSanityCheck() {
 	})
 	b.Acks[0], b.Acks[1] = b.Acks[1], b.Acks[0]
 	req.NoError(auth.SignBlock(b))
-	req.Equal(lattice.SanityCheck(b, false), ErrAcksNotSorted)
+	req.Equal(lattice.SanityCheck(b), ErrAcksNotSorted)
 	// A block with incorrect hash should not pass sanity check.
 	b.Hash = common.NewRandomHash()
-	req.Equal(lattice.SanityCheck(b, false), ErrIncorrectHash)
+	req.Equal(lattice.SanityCheck(b), ErrIncorrectHash)
 }
 
 func TestLattice(t *testing.T) {