diff --git a/consensus/consortium/common/constants.go b/consensus/consortium/common/constants.go index 6363311d20..c3c7450a68 100644 --- a/consensus/consortium/common/constants.go +++ b/consensus/consortium/common/constants.go @@ -53,4 +53,18 @@ var ( // ErrWrongDifficulty is returned if the difficulty of a block doesn't match the // turn of the signer. ErrWrongDifficulty = errors.New("wrong difficulty") + + ErrNonEpochExtraData = errors.New("non epoch block contains invalid validator fields") + + ErrAaronEpochExtraData = errors.New("aaron epoch block contains invalid validator fields") + + ErrTrippEpochExtraData = errors.New("tripp epoch block contains invalid validator fields") + + ErrPeriodBlockExtraData = errors.New("period block contains empty checkpoint validator fields") + + ErrNonPeriodBlockExtraData = errors.New("non-period block contains non-empty checkpoint validator fields") + + ErrPreTrippEpochExtraData = errors.New("pre-tripp epoch block contains empty checkpoint validator fields") + + ErrPreTrippEpochProducerExtraData = errors.New("pre-tripp epoch block contains invalid block producer fields") ) diff --git a/consensus/consortium/v2/consortium.go b/consensus/consortium/v2/consortium.go index 77255e73e0..34bb153817 100644 --- a/consensus/consortium/v2/consortium.go +++ b/consensus/consortium/v2/consortium.go @@ -361,33 +361,68 @@ func (c *Consortium) verifyValidatorFieldsInExtraData( header *types.Header, ) error { isEpoch := header.Number.Uint64()%c.config.EpochV2 == 0 || c.chainConfig.IsOnConsortiumV2(header.Number) - if !isEpoch && (len(extraData.CheckpointValidators) != 0 || len(extraData.BlockProducers) != 0) || extraData.BlockProducersBitSet != 0 { - return consortiumCommon.ErrExtraValidators + if !isEpoch { + if len(extraData.CheckpointValidators) != 0 || len(extraData.BlockProducers) != 0 || extraData.BlockProducersBitSet != 0 { + return fmt.Errorf( + "%w: checkpoint validator: %v, block producer: %v, block producer bitset: %v", + consortiumCommon.ErrNonEpochExtraData, + extraData.CheckpointValidators, + extraData.BlockProducers, + extraData.BlockProducersBitSet, + ) + } } if c.IsTrippEffective(chain, header) { if c.chainConfig.IsAaron(header.Number) { if isEpoch && (extraData.BlockProducersBitSet == 0 || len(extraData.BlockProducers) != 0) { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: block producer: %v, block producer bitset: %v", + consortiumCommon.ErrAaronEpochExtraData, + extraData.BlockProducers, + extraData.BlockProducersBitSet, + ) } } else if isEpoch && (extraData.BlockProducersBitSet != 0 || len(extraData.BlockProducers) == 0) { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: block producer: %v, block producer bitset: %v", + consortiumCommon.ErrTrippEpochExtraData, + extraData.BlockProducers, + extraData.BlockProducersBitSet, + ) } if c.IsPeriodBlock(chain, header) { if len(extraData.CheckpointValidators) == 0 { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: checkpoint validator: %v", + consortiumCommon.ErrPeriodBlockExtraData, + extraData.CheckpointValidators, + ) } } else { if len(extraData.CheckpointValidators) != 0 { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: checkpoint validator: %v", + consortiumCommon.ErrNonPeriodBlockExtraData, + extraData.CheckpointValidators, + ) } } } else { if isEpoch && len(extraData.CheckpointValidators) == 0 { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: checkpoint validator: %v", + consortiumCommon.ErrPreTrippEpochExtraData, + extraData.CheckpointValidators, + ) } if len(extraData.BlockProducers) != 0 || extraData.BlockProducersBitSet != 0 { - return consortiumCommon.ErrExtraValidators + return fmt.Errorf( + "%w: block producer: %v, block producer bitset: %v", + consortiumCommon.ErrPreTrippEpochProducerExtraData, + extraData.BlockProducers, + extraData.BlockProducersBitSet, + ) } } return nil @@ -1769,10 +1804,6 @@ func (c *Consortium) IsTrippEffective(chain consensus.ChainHeaderReader, header return c.testTrippEffective } if c.chainConfig.IsTripp(header.Number) { - if c.testTrippEffective { - return true - } - // When Tripp has been effective for long enough, we return true without any additional checks. if header.Number.Uint64() > c.chainConfig.TrippBlock.Uint64()+28800 { return true diff --git a/consensus/consortium/v2/consortium_test.go b/consensus/consortium/v2/consortium_test.go index 66981b01e4..6e2bb57718 100644 --- a/consensus/consortium/v2/consortium_test.go +++ b/consensus/consortium/v2/consortium_test.go @@ -855,6 +855,7 @@ func TestVerifyFinalitySignatureTripp(t *testing.T) { EpochV2: 300, }, recents: recents, + isTest: true, testTrippEffective: true, } snap.Hash = blockHash @@ -1311,6 +1312,7 @@ func TestAssembleFinalityVoteTripp(t *testing.T) { c := Consortium{ chainConfig: &chainConfig, votePool: &mock, + isTest: true, testTrippEffective: true, } @@ -1979,6 +1981,7 @@ func TestUpgradeAxieProxyCode(t *testing.T) { config: ¶ms.ConsortiumConfig{ EpochV2: 200, }, + isTest: true, testTrippEffective: true, } @@ -2386,7 +2389,7 @@ func TestIsTrippEffective(t *testing.T) { } } -func TestHeaderExtraDataCheckAfterTripp(t *testing.T) { +func TestHeaderExtraDataCheck(t *testing.T) { c := Consortium{ chainConfig: ¶ms.ChainConfig{ TrippBlock: common.Big0, @@ -2398,7 +2401,8 @@ func TestHeaderExtraDataCheckAfterTripp(t *testing.T) { testTrippEffective: true, } - // Not an epoch block, every validator field must be empty + // Case 1: not an epoch block, every validator field must be empty + // non-empty checkpoint validators header := types.Header{Number: big.NewInt(100)} extraData := finality.HeaderExtraData{ CheckpointValidators: []finality.ValidatorWithBlsPub{ @@ -2406,21 +2410,47 @@ func TestHeaderExtraDataCheckAfterTripp(t *testing.T) { }, } err := c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) - if err == nil { - t.Fatalf("Expect an error") + if !errors.Is(err, consortiumCommon.ErrNonEpochExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrNonEpochExtraData, err) + } + + // non-empty block producers + extraData = finality.HeaderExtraData{ + BlockProducers: []common.Address{ + {}, + }, + } + err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) + if !errors.Is(err, consortiumCommon.ErrNonEpochExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrNonEpochExtraData, err) } + // non-empty block producer bitset extraData = finality.HeaderExtraData{ + BlockProducersBitSet: 10, + } + err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) + if !errors.Is(err, consortiumCommon.ErrNonEpochExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrNonEpochExtraData, err) + } + + // Case 2: Not a period block, checkpoint validators must be empty + header = types.Header{Number: big.NewInt(200)} + extraData = finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + {}, + }, BlockProducers: []common.Address{ {}, }, } err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) - if err == nil { - t.Fatalf("Expect an error") + if !errors.Is(err, consortiumCommon.ErrNonPeriodBlockExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrNonPeriodBlockExtraData, err) } - // Not a period block, checkpoint validators must be empty + // Case 3: Before Tripp effective, block producer, block producer bitset must be empty + c.testTrippEffective = false header = types.Header{Number: big.NewInt(200)} extraData = finality.HeaderExtraData{ CheckpointValidators: []finality.ValidatorWithBlsPub{ @@ -2431,8 +2461,32 @@ func TestHeaderExtraDataCheckAfterTripp(t *testing.T) { }, } err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) - if err == nil { - t.Fatalf("Expect an error") + if !errors.Is(err, consortiumCommon.ErrPreTrippEpochProducerExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrPreTrippEpochProducerExtraData, err) + } + + header = types.Header{Number: big.NewInt(200)} + extraData = finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + {}, + }, + BlockProducersBitSet: 5, + } + err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) + if !errors.Is(err, consortiumCommon.ErrPreTrippEpochProducerExtraData) { + t.Fatalf("Expect err: %v got: %v", consortiumCommon.ErrPreTrippEpochProducerExtraData, err) + } + + // Case 4: A valid Aaron epoch block + c.chainConfig.AaronBlock = common.Big0 + c.testTrippEffective = true + header = types.Header{Number: big.NewInt(200)} + extraData = finality.HeaderExtraData{ + BlockProducersBitSet: 5, + } + err = c.verifyValidatorFieldsInExtraData(nil, &extraData, &header) + if err != nil { + t.Fatalf("Expect no error, got: %v", err) } }