core: Block randomness will be processed again in finalized block (#338)

* Process randomness result in finalized block if missed

* Add test for missing block randomness

3 files changed, 165 insertions, 78 deletions

diff --git a/core/compaction-chain.go b/core/compaction-chain.go
index 4a5ba36..f6bc014 100644
--- a/core/compaction-chain.go
+++ b/core/compaction-chain.go
@@ -33,6 +33,10 @@ var (
 		"block not registered")
 	ErrNotInitiazlied = fmt.Errorf(
 		"not initialized")
+	ErrTSigNotReady = fmt.Errorf(
+		"tsig not ready")
+	ErrIncorrectBlockRandomnessResult = fmt.Errorf(
+		"incorrect block randomness result")
 )
 
 type finalizedBlockHeap = types.ByFinalizationHeight
@@ -154,6 +158,24 @@ func (cc *compactionChain) extractBlocks() []*types.Block {
 	return deliveringBlocks
 }
 
+func (cc *compactionChain) verifyRandomness(
+	blockHash common.Hash, round uint64, randomness []byte) (bool, error) {
+	if round == 0 {
+		return len(randomness) == 0, nil
+	}
+	// Randomness is not available at round 0.
+	v, ok, err := cc.tsigVerifier.UpdateAndGet(round)
+	if err != nil {
+		return false, err
+	}
+	if !ok {
+		return false, ErrTSigNotReady
+	}
+	return v.VerifySignature(blockHash, crypto.Signature{
+		Type:      "bls",
+		Signature: randomness}), nil
+}
+
 func (cc *compactionChain) processFinalizedBlock(block *types.Block) {
 	if block.Finalization.Height <= cc.lastBlock().Finalization.Height {
 		return
@@ -166,6 +188,19 @@ func (cc *compactionChain) processFinalizedBlock(block *types.Block) {
 
 	cc.lock.Lock()
 	defer cc.lock.Unlock()
+	// The randomness result is missed previously.
+	if cc.blockRegisteredNoLock(block.Hash) {
+		ok, err := cc.verifyRandomness(
+			block.Hash, block.Position.Round, block.Finalization.Randomness)
+		if err != nil {
+			panic(err)
+		}
+		if ok {
+			cc.blockRandomness[block.Hash] = block.Finalization.Randomness
+		}
+		return
+	}
+
 	heap.Push(cc.pendingFinalizedBlocks, block)
 
 	return
@@ -206,22 +241,14 @@ func (cc *compactionChain) extractFinalizedBlocks() []*types.Block {
 			b.Finalization.Height == prevBlock.Finalization.Height {
 			continue
 		}
-		round := b.Position.Round
-		if round != 0 {
-			// Randomness is not available at round 0.
-			v, ok, err := cc.tsigVerifier.UpdateAndGet(round)
-			if err != nil {
-				continue
-			}
-			if !ok {
-				toPending = append(toPending, b)
-				continue
-			}
-			if ok := v.VerifySignature(b.Hash, crypto.Signature{
-				Type:      "bls",
-				Signature: b.Finalization.Randomness}); !ok {
-				continue
-			}
+		ok, err := cc.verifyRandomness(
+			b.Hash, b.Position.Round, b.Finalization.Randomness)
+		if err != nil {
+			toPending = append(toPending, b)
+			continue
+		}
+		if !ok {
+			continue
 		}
 		// Fork resolution: choose block with smaller hash.
 		if prevBlock.Finalization.Height == b.Finalization.Height {
@@ -261,11 +288,19 @@ func (cc *compactionChain) processBlockRandomnessResult(
 	rand *types.BlockRandomnessResult) error {
 	cc.lock.Lock()
 	defer cc.lock.Unlock()
-	// TODO(jimmy-dexon): the result should not be discarded here. Blocks may
-	// be registered later.
 	if !cc.blockRegisteredNoLock(rand.BlockHash) {
+		// If the randomness result is discarded here, it'll later be processed by
+		//finalized block
 		return ErrBlockNotRegistered
 	}
+	ok, err := cc.verifyRandomness(
+		rand.BlockHash, rand.Position.Round, rand.Randomness)
+	if err != nil {
+		return err
+	}
+	if !ok {
+		return ErrIncorrectBlockRandomnessResult
+	}
 	cc.blockRandomness[rand.BlockHash] = rand.Randomness
 	return nil
 }
diff --git a/core/compaction-chain_test.go b/core/compaction-chain_test.go
index 2767d2d..586c89c 100644
--- a/core/compaction-chain_test.go
+++ b/core/compaction-chain_test.go
@@ -35,7 +35,28 @@ type CompactionChainTestSuite struct {
 func (s *CompactionChainTestSuite) SetupTest() {
 }
 
-func (s *CompactionChainTestSuite) newCompactionChain() *compactionChain {
+type mockTSigVerifier struct {
+	defaultRet bool
+	ret        map[common.Hash]bool
+}
+
+func newMockTSigVerifier(defaultRet bool) *mockTSigVerifier {
+	return &mockTSigVerifier{
+		defaultRet: defaultRet,
+		ret:        make(map[common.Hash]bool),
+	}
+}
+
+func (m *mockTSigVerifier) VerifySignature(
+	hash common.Hash, _ crypto.Signature) bool {
+	if ret, exist := m.ret[hash]; exist {
+		return ret
+	}
+	return m.defaultRet
+}
+
+func (s *CompactionChainTestSuite) newCompactionChain() (
+	*compactionChain, *mockTSigVerifier) {
 	_, pubKeys, err := test.NewKeys(4)
 	s.Require().NoError(err)
 	gov, err := test.NewGovernance(
@@ -43,7 +64,13 @@ func (s *CompactionChainTestSuite) newCompactionChain() *compactionChain {
 	s.Require().NoError(err)
 	cc := newCompactionChain(gov)
 	cc.init(&types.Block{})
-	return cc
+
+	mock := newMockTSigVerifier(true)
+	for i := 0; i < cc.tsigVerifier.cacheSize; i++ {
+		cc.tsigVerifier.verifier[uint64(i)] = mock
+	}
+
+	return cc, mock
 }
 
 func (s *CompactionChainTestSuite) generateBlocks(
@@ -67,7 +94,7 @@ func (s *CompactionChainTestSuite) generateBlocks(
 }
 
 func (s *CompactionChainTestSuite) TestProcessBlock() {
-	cc := s.newCompactionChain()
+	cc, _ := s.newCompactionChain()
 	now := time.Now().UTC()
 	blocks := make([]*types.Block, 10)
 	for idx := range blocks {
@@ -86,7 +113,7 @@ func (s *CompactionChainTestSuite) TestProcessBlock() {
 }
 
 func (s *CompactionChainTestSuite) TestExtractBlocks() {
-	cc := s.newCompactionChain()
+	cc, _ := s.newCompactionChain()
 	s.Require().Equal(uint32(4), cc.gov.Configuration(uint64(0)).NumChains)
 	blocks := make([]*types.Block, 10)
 	for idx := range blocks {
@@ -108,6 +135,7 @@ func (s *CompactionChainTestSuite) TestExtractBlocks() {
 		s.Require().NoError(cc.processBlockRandomnessResult(
 			&types.BlockRandomnessResult{
 				BlockHash:  blocks[i].Hash,
+				Position:   blocks[i].Position,
 				Randomness: h[:],
 			}))
 	}
@@ -128,6 +156,7 @@ func (s *CompactionChainTestSuite) TestExtractBlocks() {
 		s.Require().NoError(cc.processBlockRandomnessResult(
 			&types.BlockRandomnessResult{
 				BlockHash:  blocks[i].Hash,
+				Position:   blocks[i].Position,
 				Randomness: h[:],
 			}))
 	}
@@ -144,6 +173,7 @@ func (s *CompactionChainTestSuite) TestExtractBlocks() {
 		s.Require().NoError(cc.processBlockRandomnessResult(
 			&types.BlockRandomnessResult{
 				BlockHash:  blocks[i].Hash,
+				Position:   blocks[i].Position,
 				Randomness: h[:],
 			}))
 	}
@@ -156,6 +186,7 @@ func (s *CompactionChainTestSuite) TestExtractBlocks() {
 		s.Require().NoError(cc.processBlockRandomnessResult(
 			&types.BlockRandomnessResult{
 				BlockHash:  blocks[i].Hash,
+				Position:   blocks[i].Position,
 				Randomness: h[:],
 			}))
 	}
@@ -174,8 +205,77 @@ func (s *CompactionChainTestSuite) TestExtractBlocks() {
 	}
 }
 
+func (s *CompactionChainTestSuite) TestMissedRandomness() {
+	cc, _ := s.newCompactionChain()
+	s.Require().Equal(uint32(4), cc.gov.Configuration(uint64(0)).NumChains)
+	blocks := make([]*types.Block, 10)
+	for idx := range blocks {
+		blocks[idx] = &types.Block{
+			Hash: common.NewRandomHash(),
+			Position: types.Position{
+				Round:   1,
+				Height:  uint64(idx / 4),
+				ChainID: uint32(idx % 4),
+			},
+		}
+		s.Require().False(cc.blockRegistered(blocks[idx].Hash))
+		cc.registerBlock(blocks[idx])
+		s.Require().True(cc.blockRegistered(blocks[idx].Hash))
+	}
+	for i := range blocks {
+		s.Require().NoError(cc.processBlock(blocks[i]))
+		if i >= 4 && i < 6 {
+			h := common.NewRandomHash()
+			s.Require().NoError(cc.processBlockRandomnessResult(
+				&types.BlockRandomnessResult{
+					BlockHash:  blocks[i].Hash,
+					Position:   blocks[i].Position,
+					Randomness: h[:],
+				}))
+		}
+	}
+	s.Require().Len(cc.extractBlocks(), 0)
+
+	for i := range blocks {
+		if i >= 4 {
+			break
+		}
+		block := blocks[i].Clone()
+		h := common.NewRandomHash()
+		block.Finalization.Randomness = h[:]
+		block.Finalization.Height = uint64(i + 1)
+		cc.processFinalizedBlock(block)
+	}
+	delivered := cc.extractBlocks()
+	s.Require().Len(delivered, 6)
+
+	for i := 6; i < 10; i++ {
+		h := common.NewRandomHash()
+		s.Require().NoError(cc.processBlockRandomnessResult(
+			&types.BlockRandomnessResult{
+				BlockHash:  blocks[i].Hash,
+				Position:   blocks[i].Position,
+				Randomness: h[:],
+			}))
+	}
+
+	delivered = append(delivered, cc.extractBlocks()...)
+	s.Require().Len(delivered, 10)
+
+	// The delivered order should be the same as processing order.
+	for i, block := range delivered {
+		if i > 1 {
+			s.Equal(delivered[i-1].Finalization.Height+1,
+				delivered[i].Finalization.Height)
+			s.Equal(delivered[i-1].Hash,
+				delivered[i].Finalization.ParentHash)
+		}
+		s.Equal(block.Hash, blocks[i].Hash)
+	}
+}
+
 func (s *CompactionChainTestSuite) TestExtractBlocksRound0() {
-	cc := s.newCompactionChain()
+	cc, _ := s.newCompactionChain()
 	s.Require().Equal(uint32(4), cc.gov.Configuration(uint64(0)).NumChains)
 	blocks := make([]*types.Block, 10)
 	for idx := range blocks {
@@ -209,32 +309,8 @@ func (s *CompactionChainTestSuite) TestExtractBlocksRound0() {
 	}
 }
 
-type mockTSigVerifier struct {
-	defaultRet bool
-	ret        map[common.Hash]bool
-}
-
-func newMockTSigVerifier(defaultRet bool) *mockTSigVerifier {
-	return &mockTSigVerifier{
-		defaultRet: defaultRet,
-		ret:        make(map[common.Hash]bool),
-	}
-}
-
-func (m *mockTSigVerifier) VerifySignature(
-	hash common.Hash, _ crypto.Signature) bool {
-	if ret, exist := m.ret[hash]; exist {
-		return ret
-	}
-	return m.defaultRet
-}
-
 func (s *CompactionChainTestSuite) TestSyncFinalizedBlock() {
-	cc := s.newCompactionChain()
-	mock := newMockTSigVerifier(true)
-	for i := 0; i < cc.tsigVerifier.cacheSize; i++ {
-		cc.tsigVerifier.verifier[uint64(i)] = mock
-	}
+	cc, mock := s.newCompactionChain()
 	now := time.Now().UTC()
 	blocks := make([]*types.Block, 10)
 	for idx := range blocks {
@@ -326,11 +402,7 @@ func (s *CompactionChainTestSuite) TestSyncFinalizedBlock() {
 }
 
 func (s *CompactionChainTestSuite) TestSync() {
-	cc := s.newCompactionChain()
-	mock := newMockTSigVerifier(true)
-	for i := 0; i < cc.tsigVerifier.cacheSize; i++ {
-		cc.tsigVerifier.verifier[uint64(i)] = mock
-	}
+	cc, _ := s.newCompactionChain()
 	now := time.Now().UTC()
 	blocks := make([]*types.Block, 20)
 	for idx := range blocks {
@@ -384,13 +456,9 @@ func (s *CompactionChainTestSuite) TestSync() {
 }
 
 func (s *CompactionChainTestSuite) TestBootstrapSync() {
-	cc := s.newCompactionChain()
+	cc, _ := s.newCompactionChain()
 	numChains := cc.gov.Configuration(uint64(0)).NumChains
 	s.Require().Equal(uint32(4), numChains)
-	mock := newMockTSigVerifier(true)
-	for i := 0; i < cc.tsigVerifier.cacheSize; i++ {
-		cc.tsigVerifier.verifier[uint64(i)] = mock
-	}
 	now := time.Now().UTC()
 	blocks := make([]*types.Block, 20)
 	for idx := range blocks {
diff --git a/core/consensus.go b/core/consensus.go
index 215b9f7..e09ee25 100644
--- a/core/consensus.go
+++ b/core/consensus.go
@@ -56,8 +56,6 @@ var (
 		"incorrect vote position")
 	ErrIncorrectVoteProposer = fmt.Errorf(
 		"incorrect vote proposer")
-	ErrIncorrectBlockRandomnessResult = fmt.Errorf(
-		"incorrect block randomness result")
 )
 
 // consensusBAReceiver implements agreementReceiver.
@@ -936,31 +934,17 @@ func (con *Consensus) ProcessBlockRandomnessResult(
 	if rand.Position.Round == 0 {
 		return nil
 	}
-	if !con.ccModule.blockRegistered(rand.BlockHash) {
-		return nil
-	}
-	round := rand.Position.Round
-	v, ok, err := con.ccModule.tsigVerifier.UpdateAndGet(round)
-	if err != nil {
+	if err := con.ccModule.processBlockRandomnessResult(rand); err != nil {
+		if err == ErrBlockNotRegistered {
+			err = nil
+		}
 		return err
 	}
-	if !ok {
-		return nil
-	}
-	if !v.VerifySignature(
-		rand.BlockHash, crypto.Signature{Signature: rand.Randomness}) {
-		return ErrIncorrectBlockRandomnessResult
-	}
 	con.logger.Debug("Calling Network.BroadcastRandomnessResult",
 		"hash", rand.BlockHash,
 		"position", &rand.Position,
 		"randomness", hex.EncodeToString(rand.Randomness))
 	con.network.BroadcastRandomnessResult(rand)
-	if err := con.ccModule.processBlockRandomnessResult(rand); err != nil {
-		if err != ErrBlockNotRegistered {
-			return err
-		}
-	}
 	return nil
 }