move double signing detaction to post block process hook (#1281)

Author: libotony

chain/repository.go

@@ -255,17 +255,17 @@ func (r *Repository) ScanConflicts(blockNum uint32) (uint32, error) {
 }
 
 // GetConflicts returns an array of found conflicts
-func (r *Repository) GetConflicts(blockNum uint32) ([][]byte, error) {
+func (r *Repository) GetConflicts(blockNum uint32) ([]thor.Bytes32, error) {
 	prefix := binary.BigEndian.AppendUint32(nil, blockNum)
 
 	iter := r.hdrStore.Iterate(kv.Range(*util.BytesPrefix(prefix)))
 	defer iter.Release()
 
-	conflicts := make([][]byte, 0, 1)
+	conflicts := make([]thor.Bytes32, 0, 1)
 	for iter.Next() {
 		key := make([]byte, 32)
 		copy(key, iter.Key())
-		conflicts = append(conflicts, key)
+		conflicts = append(conflicts, thor.BytesToBytes32(key))
 	}
 	return conflicts, iter.Error()
 }

chain/repository_test.go

@@ -110,17 +110,17 @@ func TestConflicts(t *testing.T) {
 
 	assert.Equal(t, []any{uint32(1), nil}, M(repo.GetMaxBlockNum()))
 	assert.Equal(t, []any{uint32(1), nil}, M(repo.ScanConflicts(1)))
-	assert.Equal(t, []any{[][]byte{b1.Header().ID().Bytes()}, nil}, M(repo.GetConflicts(1)))
+	assert.Equal(t, []any{[]thor.Bytes32{b1.Header().ID()}, nil}, M(repo.GetConflicts(1)))
 
 	b1x := newBlock(b0, 20)
 	repo.AddBlock(b1x, nil, 1, false)
 	assert.Equal(t, []any{uint32(1), nil}, M(repo.GetMaxBlockNum()))
 	assert.Equal(t, []any{uint32(2), nil}, M(repo.ScanConflicts(1)))
 	switch bytes.Compare(b1.Header().ID().Bytes(), b1x.Header().ID().Bytes()) {
 	case -1:
-		assert.Equal(t, []any{[][]byte{b1.Header().ID().Bytes(), b1x.Header().ID().Bytes()}, nil}, M(repo.GetConflicts(1)))
+		assert.Equal(t, []any{[]thor.Bytes32{b1.Header().ID(), b1x.Header().ID()}, nil}, M(repo.GetConflicts(1)))
 	case 1:
-		assert.Equal(t, []any{[][]byte{b1x.Header().ID().Bytes(), b1.Header().ID().Bytes()}, nil}, M(repo.GetConflicts(1)))
+		assert.Equal(t, []any{[]thor.Bytes32{b1x.Header().ID(), b1.Header().ID()}, nil}, M(repo.GetConflicts(1)))
 	}
 }
 

cmd/thor/node/node.go

@@ -282,13 +282,14 @@ func (n *Node) txStashLoop(ctx context.Context) {
 }
 
 // guardBlockProcessing adds lock on block processing and maintains block conflicts.
-func (n *Node) guardBlockProcessing(blockNum uint32, process func(conflicts [][]byte) (thor.Bytes32, error)) error {
+func (n *Node) guardBlockProcessing(blockNum uint32, process func(conflicts uint32) (thor.Bytes32, error)) error {
 	n.processLock.Lock()
 	defer n.processLock.Unlock()
 
 	var (
-		err     error
-		blockID thor.Bytes32
+		err       error
+		blockID   thor.Bytes32
+		conflicts []thor.Bytes32
 	)
 
 	if blockNum > n.maxBlockNum {
@@ -298,19 +299,19 @@ func (n *Node) guardBlockProcessing(blockNum uint32, process func(conflicts [][]
 		}
 
 		// don't increase maxBlockNum if the block is unprocessable
-		if blockID, err = process(make([][]byte, 0)); err != nil {
+		if blockID, err = process(0); err != nil {
 			return err
 		}
 
 		n.maxBlockNum = blockNum
 		return nil
 	}
 
-	conflicts, err := n.repo.GetConflicts(blockNum)
+	conflicts, err = n.repo.GetConflicts(blockNum)
 	if err != nil {
 		return err
 	}
-	blockID, err = process(conflicts)
+	blockID, err = process(uint32(len(conflicts)))
 	if err != nil {
 		return err
 	}
@@ -336,45 +337,52 @@ func (n *Node) guardBlockProcessing(blockNum uint32, process func(conflicts [][]
 				}
 			}
 		}
-		return nil
-	}(); err != nil {
-		logger.Warn("failed to run post process hook", "err", err)
-	}
 
-	return nil
-}
-
-func (n *Node) processBlock(newBlock *block.Block, stats *blockStats) (bool, error) {
-	var isTrunk *bool
-
-	if err := n.guardBlockProcessing(newBlock.Header().Number(), func(conflicts [][]byte) (thor.Bytes32, error) {
-		// Check whether the block was already there.
-		// It can be skipped if no conflicts.
 		if len(conflicts) > 0 {
+			newBlock, err := n.repo.GetBlock(blockID)
+			if err != nil {
+				return err
+			}
+
 			newSigner, err := newBlock.Header().Signer()
 			if err != nil {
-				return thor.Bytes32{}, err
+				return err
 			}
 			// iter over conflicting blocks
 			for _, conflict := range conflicts {
-				conflictBlock, err := n.repo.GetBlock(thor.BytesToBytes32(conflict))
+				conflictBlock, err := n.repo.GetBlock(conflict)
 				if err != nil {
-					return thor.Bytes32{}, err
+					return err
 				}
 				// logic to verify that the blocks are different and from the same signer
-				signer, err := conflictBlock.Header().Signer()
+				existingSigner, err := conflictBlock.Header().Signer()
 				if err != nil {
-					return thor.Bytes32{}, err
+					return err
 				}
-				if signer == newSigner &&
-					conflictBlock.Header().ID() != newBlock.Header().ID() &&
-					conflictBlock.Header().StateRoot() != newBlock.Header().StateRoot() {
-					log.Warn("Double signing", "block", shortID(newBlock.Header().ID()), "previous", shortID(thor.BytesToBytes32(conflict)))
-					metricDoubleSignedBlocks().AddWithLabel(1, map[string]string{"signer": signer.String()})
+				if existingSigner == newSigner &&
+					conflictBlock.Header().ID() != newBlock.Header().ID() {
+					log.Warn("Double signing", "block", shortID(newBlock.Header().ID()), "previous", shortID(conflict), "signer", existingSigner)
+					metricDoubleSignedBlocks().AddWithLabel(1, map[string]string{"signer": existingSigner.String()})
 				}
 			}
+		}
+
+		return nil
+	}(); err != nil {
+		logger.Warn("failed to run post process hook", "err", err)
+	}
 
-			_, err = n.repo.GetBlockSummary(newBlock.Header().ID())
+	return nil
+}
+
+func (n *Node) processBlock(newBlock *block.Block, stats *blockStats) (bool, error) {
+	var isTrunk *bool
+
+	if err := n.guardBlockProcessing(newBlock.Header().Number(), func(conflicts uint32) (thor.Bytes32, error) {
+		// Check whether the block was already there.
+		// It can be skipped if no conflicts.
+		if conflicts > 0 {
+			_, err := n.repo.GetBlockSummary(newBlock.Header().ID())
 			if err != nil {
 				if !n.repo.IsNotFound(err) {
 					return thor.Bytes32{}, err
@@ -403,7 +411,7 @@ func (n *Node) processBlock(newBlock *block.Block, stats *blockStats) (bool, err
 		}
 
 		// process the new block
-		stage, receipts, err := n.cons.Process(parentSummary, newBlock, uint64(time.Now().Unix()), uint32(len(conflicts)))
+		stage, receipts, err := n.cons.Process(parentSummary, newBlock, uint64(time.Now().Unix()), conflicts)
 		if err != nil {
 			return thor.Bytes32{}, err
 		}
@@ -444,7 +452,7 @@ func (n *Node) processBlock(newBlock *block.Block, stats *blockStats) (bool, err
 		}
 
 		// add the new block into repository
-		if err := n.repo.AddBlock(newBlock, receipts, uint32(len(conflicts)), becomeNewBest); err != nil {
+		if err := n.repo.AddBlock(newBlock, receipts, conflicts, becomeNewBest); err != nil {
 			return thor.Bytes32{}, errors.Wrap(err, "add block")
 		}
 

cmd/thor/node/packer_loop.go

@@ -120,7 +120,7 @@ func (n *Node) pack(flow *packer.Flow) (err error) {
 		}
 	}()
 
-	return n.guardBlockProcessing(flow.Number(), func(conflicts [][]byte) (thor.Bytes32, error) {
+	return n.guardBlockProcessing(flow.Number(), func(conflicts uint32) (thor.Bytes32, error) {
 		var (
 			startTime  = mclock.Now()
 			logEnabled = !n.options.SkipLogs && !n.logDBFailed
@@ -150,7 +150,7 @@ func (n *Node) pack(flow *packer.Flow) (err error) {
 		}
 
 		// pack the new block
-		newBlock, stage, receipts, err := flow.Pack(n.master.PrivateKey, uint32(len(conflicts)), shouldVote)
+		newBlock, stage, receipts, err := flow.Pack(n.master.PrivateKey, conflicts, shouldVote)
 		if err != nil {
 			return thor.Bytes32{}, errors.Wrap(err, "failed to pack block")
 		}
@@ -177,7 +177,7 @@ func (n *Node) pack(flow *packer.Flow) (err error) {
 		}
 
 		// add the new block into repository
-		if err := n.repo.AddBlock(newBlock, receipts, uint32(len(conflicts)), true); err != nil {
+		if err := n.repo.AddBlock(newBlock, receipts, conflicts, true); err != nil {
 			return thor.Bytes32{}, errors.Wrap(err, "add block")
 		}