diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index ab37fd820f..43c3cdaf95 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/consensus/XDPoS/engines/engine_v2/engine.go @@ -121,16 +121,6 @@ func New(config *params.XDPoSConfig, db ethdb.Database, waitPeriodCh chan int) * return engine } -/* V2 Block -SignerFn is a signer callback function to request a hash to be signed by a -backing account. -type SignerFn func(accounts.Account, []byte) ([]byte, error) - -sigHash returns the hash which is used as input for the delegated-proof-of-stake -signing. It is the hash of the entire header apart from the 65 byte signature -contained at the end of the extra data. -*/ - func (x *XDPoS_v2) UpdateParams(header *types.Header) { _, round, _, err := x.getExtraFields(header) if err != nil { @@ -148,6 +138,15 @@ func (x *XDPoS_v2) UpdateParams(header *types.Header) { }() } +/* V2 Block +SignerFn is a signer callback function to request a hash to be signed by a +backing account. +type SignerFn func(accounts.Account, []byte) ([]byte, error) + +sigHash returns the hash which is used as input for the delegated-proof-of-stake +signing. It is the hash of the entire header apart from the 65 byte signature +contained at the end of the extra data. +*/ func (x *XDPoS_v2) SignHash(header *types.Header) (hash common.Hash) { return sigHash(header) } @@ -255,7 +254,7 @@ func (x *XDPoS_v2) YourTurn(chain consensus.ChainReader, parent *types.Header, s _, parentRound, _, err := x.getExtraFields(parent) minePeriod := x.config.V2.Config(uint64(parentRound) + 1).MinePeriod // plus 1 means current block if waitedTime < int64(minePeriod) { - log.Trace("[YourTurn] wait after mine period", "minePeriod", x.config.V2.CurrentConfig.MinePeriod, "waitedTime", waitedTime) + log.Trace("[YourTurn] wait after mine period", "minePeriod", minePeriod, "waitedTime", waitedTime) return false, nil } diff --git a/consensus/XDPoS/engines/engine_v2/utils.go b/consensus/XDPoS/engines/engine_v2/utils.go index 59115729ab..0b1faf3cf2 100644 --- a/consensus/XDPoS/engines/engine_v2/utils.go +++ b/consensus/XDPoS/engines/engine_v2/utils.go @@ -141,3 +141,17 @@ func (x *XDPoS_v2) getExtraFields(header *types.Header) (*types.QuorumCert, type } return decodedExtraField.QuorumCert, decodedExtraField.Round, masternodes, nil } + +func (x *XDPoS_v2) GetRoundNumber(header *types.Header) (types.Round, error) { + // If not v2 yet, return 0 + if header.Number.Cmp(x.config.V2.SwitchBlock) <= 0 { + return types.Round(0), nil + } else { + var decodedExtraField types.ExtraFields_v2 + err := utils.DecodeBytesExtraFields(header.Extra, &decodedExtraField) + if err != nil { + return types.Round(0), err + } + return decodedExtraField.Round, nil + } +} diff --git a/consensus/XDPoS/engines/engine_v2/verifyHeader.go b/consensus/XDPoS/engines/engine_v2/verifyHeader.go index 6ea6cc1ae3..107a99cc91 100644 --- a/consensus/XDPoS/engines/engine_v2/verifyHeader.go +++ b/consensus/XDPoS/engines/engine_v2/verifyHeader.go @@ -58,9 +58,6 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade if parent == nil || parent.Number.Uint64() != number-1 || parent.Hash() != header.ParentHash { return consensus.ErrUnknownAncestor } - if parent.Number.Uint64() > x.config.V2.SwitchBlock.Uint64() && parent.Time.Uint64()+uint64(x.config.V2.CurrentConfig.MinePeriod) > header.Time.Uint64() { - return utils.ErrInvalidTimestamp - } // Verify this is truely a v2 block first quorumCert, round, _, err := x.getExtraFields(header) @@ -68,6 +65,13 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade log.Warn("[verifyHeader] decode extra field error", "err", err) return utils.ErrInvalidV2Extra } + + minePeriod := uint64(x.config.V2.Config(uint64(round)).MinePeriod) + if parent.Number.Uint64() > x.config.V2.SwitchBlock.Uint64() && parent.Time.Uint64()+minePeriod > header.Time.Uint64() { + log.Warn("[verifyHeader] Fail to verify header due to invalid timestamp", "ParentTime", parent.Time.Uint64(), "MinePeriod", minePeriod, "HeaderTime", header.Time.Uint64(), "Hash", header.Hash().Hex()) + return utils.ErrInvalidTimestamp + } + if round <= quorumCert.ProposedBlockInfo.Round { return utils.ErrRoundInvalid } diff --git a/consensus/tests/engine_v2_tests/verify_header_test.go b/consensus/tests/engine_v2_tests/verify_header_test.go index 503f56c54f..11bb9c1fd3 100644 --- a/consensus/tests/engine_v2_tests/verify_header_test.go +++ b/consensus/tests/engine_v2_tests/verify_header_test.go @@ -259,6 +259,59 @@ func TestConfigSwitchOnDifferentCertThreshold(t *testing.T) { assert.Equal(t, utils.ErrValidatorNotWithinMasternodes, err) } +func TestConfigSwitchOnDifferentMindPeriod(t *testing.T) { + b, err := json.Marshal(params.TestXDPoSMockChainConfig) + assert.Nil(t, err) + configString := string(b) + + var config params.ChainConfig + err = json.Unmarshal([]byte(configString), &config) + assert.Nil(t, err) + // Enable verify + config.XDPoS.V2.SkipV2Validation = false + // Block 901 is the first v2 block with round of 1 + blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 915, &config, nil) + + adaptor := blockchain.Engine().(*XDPoS.XDPoS) + + // Genrate 911 QC + proposedBlockInfo := &types.BlockInfo{ + Hash: blockchain.GetBlockByNumber(911).Hash(), + Round: types.Round(11), + Number: blockchain.GetBlockByNumber(911).Number(), + } + voteForSign := &types.VoteForSign{ + ProposedBlockInfo: proposedBlockInfo, + GapNumber: 450, + } + + // Sign from acc 1, 2, 3 + acc1SignedHash := SignHashByPK(acc1Key, types.VoteSigHash(voteForSign).Bytes()) + acc2SignedHash := SignHashByPK(acc2Key, types.VoteSigHash(voteForSign).Bytes()) + acc3SignedHash := SignHashByPK(acc3Key, types.VoteSigHash(voteForSign).Bytes()) + var signaturesFirst []types.Signature + signaturesFirst = append(signaturesFirst, acc1SignedHash, acc2SignedHash, acc3SignedHash) + quorumCert := &types.QuorumCert{ + ProposedBlockInfo: proposedBlockInfo, + Signatures: signaturesFirst, + GapNumber: 450, + } + + extra := types.ExtraFields_v2{ + Round: types.Round(12), + QuorumCert: quorumCert, + } + extraInBytes, _ := extra.EncodeToBytes() + + // after 910 require 5 signs, but we only give 3 signs + block911 := blockchain.GetBlockByNumber(911).Header() + block911.Extra = extraInBytes + block911.Time = big.NewInt(blockchain.GetBlockByNumber(910).Time().Int64() + 2) //2 is previous config, should get the right config from round + err = adaptor.VerifyHeader(blockchain, block911, true) + + assert.Equal(t, utils.ErrInvalidTimestamp, err) +} + func TestShouldFailIfNotEnoughQCSignatures(t *testing.T) { b, err := json.Marshal(params.TestXDPoSMockChainConfig) assert.Nil(t, err) diff --git a/core/blockchain.go b/core/blockchain.go index 9b23f71333..102b861384 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -18,6 +18,7 @@ package core import ( + "encoding/json" "errors" "fmt" "io" @@ -2313,6 +2314,15 @@ func (bc *BlockChain) addBadBlock(block *types.Block) { func (bc *BlockChain) reportBlock(block *types.Block, receipts types.Receipts, err error) { bc.addBadBlock(block) + // V2 specific logs + config, _ := json.Marshal(bc.chainConfig) + + var roundNumber = types.Round(0) + engine, ok := bc.Engine().(*XDPoS.XDPoS) + if ok { + roundNumber, err = engine.EngineV2.GetRoundNumber(block.Header()) + } + var receiptString string for _, receipt := range receipts { receiptString += fmt.Sprintf("\t%v\n", receipt) @@ -2325,9 +2335,10 @@ Number: %v Hash: 0x%x %v +Round: %v Error: %v ############################## -`, bc.chainConfig, block.Number(), block.Hash(), receiptString, err)) +`, string(config), block.Number(), block.Hash(), receiptString, roundNumber, err)) } // InsertHeaderChain attempts to insert the given header chain in to the local diff --git a/params/config.go b/params/config.go index 80f6bfd61c..25bd371186 100644 --- a/params/config.go +++ b/params/config.go @@ -401,7 +401,6 @@ func (v *V2) UpdateConfig(round uint64) { for i := range v.configIndex { if v.configIndex[i] <= round { index = v.configIndex[i] - } else { break } } @@ -418,7 +417,6 @@ func (v *V2) Config(round uint64) *V2Config { for i := range v.configIndex { if v.configIndex[i] <= configRound { index = v.configIndex[i] - } else { break } } @@ -433,9 +431,10 @@ func (v *V2) BuildConfigIndex() { } // sort, sort lib doesn't support type uint64, it's ok to have O(n^2) because only few items in the list + // Make it descending order for i := 0; i < len(list)-1; i++ { for j := i + 1; j < len(list); j++ { - if list[i] > list[j] { + if list[i] < list[j] { list[i], list[j] = list[j], list[i] } } diff --git a/params/config_test.go b/params/config_test.go index 9143762d23..8992e15b4f 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -114,6 +114,6 @@ func TestV2Config(t *testing.T) { func TestBuildConfigIndex(t *testing.T) { TestnetChainConfig.XDPoS.V2.BuildConfigIndex() index := TestnetChainConfig.XDPoS.V2.ConfigIndex() - expected := []uint64{0, 10, 899} + expected := []uint64{899, 10, 0} assert.Equal(t, expected, index) }