Merge pull request #1522 from chaosma/master

Improve cross shard transaction verification mechanism

api/service/syncing/syncing.go

@@ -538,8 +538,9 @@ func (ss *StateSync) updateBlockAndStatus(block *types.Block, bc *core.BlockChai
 	utils.Logger().Info().Str("blockHex", bc.CurrentBlock().Hash().Hex()).Msg("[SYNC] Current Block")
 
 	// Verify block signatures
+	// TODO chao: only when block is verified against last commit sigs, we can update the block and status
 	if block.NumberU64() > 1 {
-		err := core.VerifyBlockLastCommitSigs(bc, block)
+		err := core.VerifyBlockLastCommitSigs(bc, block.Header())
 		if err != nil {
 			utils.Logger().Error().Err(err).Msgf("[SYNC] failed verifying signatures for new block %d", block.NumberU64())
 			return false

core/block_validator.go

@@ -24,6 +24,7 @@ import (
 	bls2 "github.com/harmony-one/harmony/crypto/bls"
 	"github.com/harmony-one/harmony/internal/ctxerror"
 
+	"github.com/harmony-one/harmony/block"
 	consensus_engine "github.com/harmony-one/harmony/consensus/engine"
 	"github.com/harmony-one/harmony/core/state"
 	"github.com/harmony-one/harmony/core/types"
@@ -109,19 +110,13 @@ func (v *BlockValidator) ValidateState(block, parent *types.Block, statedb *stat
 	return nil
 }
 
-// VerifyBlockLastCommitSigs verifies the last commit sigs of the block
+// VerifyHeaderWithSignature verifies the header with corresponding commit sigs
-func VerifyBlockLastCommitSigs(bc *BlockChain, block *types.Block) error {
+func VerifyHeaderWithSignature(bc *BlockChain, header *block.Header, commitSig [96]byte, commitBitmap []byte) error {
-	header := block.Header()
+	shardState, err := bc.ReadShardState(header.Epoch())
-	parentBlock := bc.GetBlockByNumber(block.NumberU64() - 1)
+	committee := shardState.FindCommitteeByID(header.ShardID())
-	if parentBlock == nil {
-		return ctxerror.New("[VerifyNewBlock] Failed to get parent block", "shardID", header.ShardID(), "blockNum", header.Number())
-	}
-	parentHeader := parentBlock.Header()
-	shardState, err := bc.ReadShardState(parentHeader.Epoch())
-	committee := shardState.FindCommitteeByID(parentHeader.ShardID())
 
 	if err != nil || committee == nil {
-		return ctxerror.New("[VerifyNewBlock] Failed to read shard state for cross link header", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
+		return ctxerror.New("[VerifyHeaderWithSignature] Failed to read shard state", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
 	}
 	var committerKeys []*bls.PublicKey
 
@@ -136,34 +131,46 @@ func VerifyBlockLastCommitSigs(bc *BlockChain, block *types.Block) error {
 		committerKeys = append(committerKeys, committerKey)
 	}
 	if !parseKeysSuccess {
-		return ctxerror.New("[VerifyNewBlock] cannot convert BLS public key", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
+		return ctxerror.New("[VerifyBlockWithSignature] cannot convert BLS public key", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
 	}
 
 	mask, err := bls2.NewMask(committerKeys, nil)
 	if err != nil {
-		return ctxerror.New("[VerifyNewBlock] cannot create group sig mask", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
+		return ctxerror.New("[VerifyHeaderWithSignature] cannot create group sig mask", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
 	}
-	if err := mask.SetMask(header.LastCommitBitmap()); err != nil {
+	if err := mask.SetMask(commitBitmap); err != nil {
-		return ctxerror.New("[VerifyNewBlock] cannot set group sig mask bits", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
+		return ctxerror.New("[VerifyHeaderWithSignature] cannot set group sig mask bits", "shardID", header.ShardID(), "blockNum", header.Number()).WithCause(err)
 	}
 
 	aggSig := bls.Sign{}
-	lastCommitSig := header.LastCommitSignature()
+	err = aggSig.Deserialize(commitSig[:])
-	err = aggSig.Deserialize(lastCommitSig[:])
 	if err != nil {
 		return ctxerror.New("[VerifyNewBlock] unable to deserialize multi-signature from payload").WithCause(err)
 	}
 
 	blockNumBytes := make([]byte, 8)
-	binary.LittleEndian.PutUint64(blockNumBytes, header.Number().Uint64()-1)
+	binary.LittleEndian.PutUint64(blockNumBytes, header.Number().Uint64())
-	parentHash := header.ParentHash()
+	hash := header.Hash()
-	commitPayload := append(blockNumBytes, parentHash[:]...)
+	commitPayload := append(blockNumBytes, hash[:]...)
 	if !aggSig.VerifyHash(mask.AggregatePublic, commitPayload) {
-		return ctxerror.New("[VerifyNewBlock] Failed to verify the signature for last commit sig", "shardID", header.ShardID(), "blockNum", header.Number())
+		return ctxerror.New("[VerifyHeaderWithSignature] Failed to verify the signature for last commit sig", "shardID", header.ShardID(), "blockNum", header.Number())
 	}
 	return nil
 }
 
+// VerifyBlockLastCommitSigs verifies the last commit sigs of the block
+func VerifyBlockLastCommitSigs(bc *BlockChain, header *block.Header) error {
+	parentBlock := bc.GetBlockByNumber(header.Number().Uint64() - 1)
+	if parentBlock == nil {
+		return ctxerror.New("[VerifyBlockLastCommitSigs] Failed to get parent block", "shardID", header.ShardID(), "blockNum", header.Number())
+	}
+	parentHeader := parentBlock.Header()
+	lastCommitSig := header.LastCommitSignature()
+	lastCommitBitmap := header.LastCommitBitmap()
+
+	return VerifyHeaderWithSignature(bc, parentHeader, lastCommitSig, lastCommitBitmap)
+}
+
 // CalcGasLimit computes the gas limit of the next block after parent. It aims
 // to keep the baseline gas above the provided floor, and increase it towards the
 // ceil if the blocks are full. If the ceil is exceeded, it will always decrease

node/node_handler.go

@@ -338,7 +338,7 @@ func (node *Node) VerifyNewBlock(newBlock *types.Block) error {
 	//  e.g. "child.Number == child.IsGenesis() ? 0 : parent.Number+1"?
 
 	if newBlock.NumberU64() > 1 {
-		err := core.VerifyBlockLastCommitSigs(node.Blockchain(), newBlock)
+		err := core.VerifyBlockLastCommitSigs(node.Blockchain(), newBlock.Header())
 		if err != nil {
 			return err
 		}