From 584db0f668b2d62d566331b0d6268866560f36e9 Mon Sep 17 00:00:00 2001 From: minh-bq <97180373+minh-bq@users.noreply.github.com> Date: Tue, 19 Nov 2024 14:01:21 +0700 Subject: [PATCH] consortium-v2: move extraData check with contract into a function (#632) This commit refactors the logic that checks extraData's validator information with contract data into a separate function. It also adds more information in the error cases. --- consensus/consortium/v2/consortium.go | 71 +++--- consensus/consortium/v2/consortium_test.go | 220 ++++++++++++++++++ .../v2/finality/consortium_header.go | 11 + 3 files changed, 270 insertions(+), 32 deletions(-) diff --git a/consensus/consortium/v2/consortium.go b/consensus/consortium/v2/consortium.go index c2171e7403..83bc8162a7 100644 --- a/consensus/consortium/v2/consortium.go +++ b/consensus/consortium/v2/consortium.go @@ -1201,6 +1201,37 @@ func (c *Consortium) upgradeTransparentProxyCode(blockNumber *big.Int, statedb * } } +func verifyValidatorExtraDataWithContract( + validatorInContract []finality.ValidatorWithBlsPub, + extraData *finality.HeaderExtraData, + checkBlsKey bool, + checkVoteWeight bool, +) error { + if len(validatorInContract) != len(extraData.CheckpointValidators) { + return fmt.Errorf("%w: contract: %v, extraData: %v", + errMismatchingValidators, validatorInContract, extraData.CheckpointValidators) + } + for i := range validatorInContract { + if validatorInContract[i].Address != extraData.CheckpointValidators[i].Address { + return fmt.Errorf("%w: contract: %v, extraData: %v", + errMismatchingValidators, validatorInContract, extraData.CheckpointValidators) + } + if checkBlsKey { + if !validatorInContract[i].BlsPublicKey.Equals(extraData.CheckpointValidators[i].BlsPublicKey) { + return fmt.Errorf("%w: contract: %v, extraData: %v", + errMismatchingValidators, validatorInContract, extraData.CheckpointValidators) + } + } + if checkVoteWeight { + if validatorInContract[i].Weight != extraData.CheckpointValidators[i].Weight { + return fmt.Errorf("%w: contract: %v, extraData: %v", + errMismatchingValidators, validatorInContract, extraData.CheckpointValidators) + } + } + } + return nil +} + // Finalize implements consensus.Engine that calls three methods from smart contracts: // - WrapUpEpoch at epoch to distribute rewards and sort the validators set // - Slash the validator who does not sign if it is in-turn @@ -1257,51 +1288,27 @@ func (c *Consortium) Finalize(chain consensus.ChainHeaderReader, header *types.H } bitSet := encodeValidatorBitSet(checkpointValidators, blockProducers) if bitSet != extraData.BlockProducersBitSet { - return errMismatchingValidators + return fmt.Errorf("%w: contract's bitset: %x, extraData's bitset: %x", + errMismatchingValidators, bitSet, extraData.BlockProducersBitSet) } } else { if len(blockProducers) != len(extraData.BlockProducers) { - return errMismatchingValidators + return fmt.Errorf("%w: contract's block producer: %v, extraData's block producer: %v", + errMismatchingValidators, blockProducers, extraData.BlockProducers) } for i := range blockProducers { if blockProducers[i] != extraData.BlockProducers[i] { - return errMismatchingValidators + return fmt.Errorf("%w: contract's block producer: %v, extraData's block producer: %v", + errMismatchingValidators, blockProducers, extraData.BlockProducers) } } } if c.IsPeriodBlock(chain, header, nil) { - if len(checkpointValidators) != len(extraData.CheckpointValidators) { - return errMismatchingValidators - } - for i := range checkpointValidators { - if checkpointValidators[i].Address != extraData.CheckpointValidators[i].Address { - return errMismatchingValidators - } - if !checkpointValidators[i].BlsPublicKey.Equals(extraData.CheckpointValidators[i].BlsPublicKey) { - return errMismatchingValidators - } - if checkpointValidators[i].Weight != extraData.CheckpointValidators[i].Weight { - return errMismatchingValidators - } - } + return verifyValidatorExtraDataWithContract(checkpointValidators, extraData, true, true) } } else { - if len(checkpointValidators) != len(extraData.CheckpointValidators) { - return errMismatchingValidators - } - isShillin := c.chainConfig.IsShillin(header.Number) - for i := range checkpointValidators { - if checkpointValidators[i].Address != extraData.CheckpointValidators[i].Address { - return errMismatchingValidators - } - - if isShillin { - if !checkpointValidators[i].BlsPublicKey.Equals(extraData.CheckpointValidators[i].BlsPublicKey) { - return errMismatchingValidators - } - } - } + return verifyValidatorExtraDataWithContract(checkpointValidators, extraData, isShillin, false) } } diff --git a/consensus/consortium/v2/consortium_test.go b/consensus/consortium/v2/consortium_test.go index b660c50b84..197b6f94a4 100644 --- a/consensus/consortium/v2/consortium_test.go +++ b/consensus/consortium/v2/consortium_test.go @@ -2737,3 +2737,223 @@ func TestVerifyBlobHeader(t *testing.T) { t.Fatal("Expected sidecars to be kept") } } + +func TestVerifyValidatorExtraDataWithContract(t *testing.T) { + type testcase struct { + validatorInContract []finality.ValidatorWithBlsPub + extraData *finality.HeaderExtraData + checkBlsKey bool + checkVoteWeight bool + output error + } + + blsKey1, _ := blst.RandKey() + blsKey2, _ := blst.RandKey() + + var tests []testcase = []testcase{ + // Length mismatches + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + }, + }, + extraData: &finality.HeaderExtraData{}, + checkBlsKey: false, + checkVoteWeight: false, + output: errMismatchingValidators, + }, + // Address mismatches + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey2.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(12)), + BlsPublicKey: blsKey1.PublicKey(), + }, + }, + }, + checkBlsKey: false, + checkVoteWeight: false, + output: errMismatchingValidators, + }, + // Address matches, we only check address here + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey2.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey1.PublicKey(), + }, + }, + }, + checkBlsKey: false, + checkVoteWeight: false, + output: nil, + }, + // Address matches, but BLS public key mismatches + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey2.PublicKey(), + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey1.PublicKey(), + }, + }, + }, + checkBlsKey: true, + checkVoteWeight: false, + output: errMismatchingValidators, + }, + // Address matches, BLS key matches, ignore weight + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 10, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 11, + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 5, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 6, + }, + }, + }, + checkBlsKey: true, + checkVoteWeight: false, + output: nil, + }, + // Address matches, BLS key matches, weight mismatches + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 10, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 11, + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 5, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 6, + }, + }, + }, + checkBlsKey: true, + checkVoteWeight: true, + output: errMismatchingValidators, + }, + // Everything matches + { + validatorInContract: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 5, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 6, + }, + }, + extraData: &finality.HeaderExtraData{ + CheckpointValidators: []finality.ValidatorWithBlsPub{ + { + Address: common.BigToAddress(big.NewInt(10)), + BlsPublicKey: blsKey1.PublicKey(), + Weight: 5, + }, + { + Address: common.BigToAddress(big.NewInt(11)), + BlsPublicKey: blsKey2.PublicKey(), + Weight: 6, + }, + }, + }, + checkBlsKey: true, + checkVoteWeight: true, + output: nil, + }, + } + + for _, test := range tests { + output := verifyValidatorExtraDataWithContract( + test.validatorInContract, + test.extraData, + test.checkBlsKey, + test.checkVoteWeight, + ) + if !errors.Is(output, test.output) { + t.Fatalf("Expect err: %v, got: %v", test.output, output) + } + } +} diff --git a/consensus/consortium/v2/finality/consortium_header.go b/consensus/consortium/v2/finality/consortium_header.go index 2706a504d1..f5bf4b2b03 100644 --- a/consensus/consortium/v2/finality/consortium_header.go +++ b/consensus/consortium/v2/finality/consortium_header.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "fmt" "math/big" "github.com/ethereum/go-ethereum/common" @@ -124,6 +125,16 @@ func (validator ValidatorWithBlsPub) MarshalJSON() ([]byte, error) { return json.Marshal(&savedValidator) } +func (validator ValidatorWithBlsPub) String() string { + var blsPublicKey string + if validator.BlsPublicKey != nil { + blsPublicKey = hex.EncodeToString(validator.BlsPublicKey.Marshal()) + } + + return fmt.Sprintf("Address: %v, BlsPublicKey: %s, Weight: %d", + validator.Address, blsPublicKey, validator.Weight) +} + // CheckpointValidatorAscending implements the sort interface to allow sorting a list // of checkpoint validator type CheckpointValidatorAscending []ValidatorWithBlsPub