diff --git a/common/countdown/countdown.go b/common/countdown/countdown.go index 955460787bea..860381e110fd 100644 --- a/common/countdown/countdown.go +++ b/common/countdown/countdown.go @@ -62,6 +62,8 @@ func (t *CountdownTimer) startTimer() { if err != nil { log.Error("OnTimeoutFn error", err) } + log.Debug("Reset timer after timeout reached and OnTimeoutFn processed") + timer.Reset(t.timeoutDuration) case <-t.resetc: log.Debug("Reset countdown timer") timer.Reset(t.timeoutDuration) diff --git a/common/countdown/countdown_test.go b/common/countdown/countdown_test.go index b962a9748136..758287e5e172 100644 --- a/common/countdown/countdown_test.go +++ b/common/countdown/countdown_test.go @@ -1,6 +1,7 @@ package countdown import ( + "fmt" "testing" "time" @@ -59,7 +60,53 @@ firstReset: // Now the countdown is paused after calling the callback function, let's reset it again assert.True(t, countdown.isInitilised()) expectedTimeAfterReset := time.Now().Add(5000 * time.Millisecond) + <-called + // Always initilised + assert.True(t, countdown.isInitilised()) + if time.Now().After(expectedTimeAfterReset) { + t.Log("Correctly reset the countdown second time") + } else { + t.Fatalf("Countdown did not reset correctly second time") + } +} + +func TestCountdownShouldResetEvenIfErrored(t *testing.T) { + called := make(chan int) + OnTimeoutFn := func(time time.Time) error { + called <- 1 + return fmt.Errorf("ERROR!") + } + + countdown := NewCountDown(5000 * time.Millisecond) + countdown.OnTimeoutFn = OnTimeoutFn + // Check countdown did not start + assert.False(t, countdown.isInitilised()) countdown.Reset() + // Now the countdown should already started + assert.True(t, countdown.isInitilised()) + expectedCalledTime := time.Now().Add(9000 * time.Millisecond) + resetTimer := time.NewTimer(4000 * time.Millisecond) + +firstReset: + for { + select { + case <-called: + if time.Now().After(expectedCalledTime) { + // Make sure the countdown runs forever + assert.True(t, countdown.isInitilised()) + t.Log("Correctly reset the countdown once") + } else { + t.Fatalf("Countdown did not reset correctly first time") + } + break firstReset + case <-resetTimer.C: + countdown.Reset() + } + } + + // Now the countdown is paused after calling the callback function, let's reset it again + assert.True(t, countdown.isInitilised()) + expectedTimeAfterReset := time.Now().Add(5000 * time.Millisecond) <-called // Always initilised assert.True(t, countdown.isInitilised()) diff --git a/consensus/XDPoS/XDPoS.go b/consensus/XDPoS/XDPoS.go index 95df621c79c9..e132b17e5fb3 100644 --- a/consensus/XDPoS/XDPoS.go +++ b/consensus/XDPoS/XDPoS.go @@ -445,6 +445,19 @@ func (x *XDPoS) GetAuthorisedSignersFromSnapshot(chain consensus.ChainReader, he } } +func (x *XDPoS) FindParentBlockToAssign(chain consensus.ChainReader, currentBlock *types.Block) *types.Block { + switch x.config.BlockConsensusVersion(currentBlock.Number()) { + case params.ConsensusEngineVersion2: + block := x.EngineV2.FindParentBlockToAssign(chain) + if block == nil { + return currentBlock + } + return block + default: // Default "v1" + return currentBlock + } +} + /** Caching */ @@ -502,7 +515,7 @@ func (x *XDPoS) initialV2FromLastV1(chain consensus.ChainReader, header *types.H checkpointBlockNumber := header.Number.Uint64() - header.Number.Uint64()%x.config.Epoch checkpointHeader := chain.GetHeaderByNumber(checkpointBlockNumber) masternodes := x.EngineV1.GetMasternodesFromCheckpointHeader(checkpointHeader) - err := x.EngineV2.Initial(chain, header, masternodes) + err := x.EngineV2.Initial(chain, masternodes) if err != nil { return err } diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index 7add26c1c259..4f170327d9ff 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/consensus/XDPoS/engines/engine_v2/engine.go @@ -121,11 +121,11 @@ func (x *XDPoS_v2) SignHash(header *types.Header) (hash common.Hash) { return sigHash(header) } -func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, masternodes []common.Address) error { +func (x *XDPoS_v2) Initial(chain consensus.ChainReader, masternodes []common.Address) error { log.Info("[Initial] initial v2 related parameters") if x.highestQuorumCert.ProposedBlockInfo.Hash != (common.Hash{}) { // already initialized - log.Error("[Initial] Already initialized", "blockNum", header.Number, "Hash", header.Hash()) + log.Error("[Initial] Already initialized", "x.highestQuorumCert.ProposedBlockInfo.Hash", x.highestQuorumCert.ProposedBlockInfo.Hash) return nil } @@ -133,12 +133,14 @@ func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, ma defer x.lock.Unlock() // Check header if it is the first consensus v2 block, if so, assign initial values to current round and highestQC - log.Info("[Initial] highest QC for consensus v2 first block", "BlockNum", header.Number.String(), "BlockHash", header.Hash()) + log.Info("[Initial] highest QC for consensus v2 first block") // Generate new parent blockInfo and put it into QC + // TODO: XIN-147 to initilise V2 engine in a more dynamic way + firstV2BlockHeader := chain.GetHeaderByNumber(x.config.V2.SwitchBlock.Uint64()) blockInfo := &utils.BlockInfo{ - Hash: header.Hash(), + Hash: firstV2BlockHeader.Hash(), Round: utils.Round(0), - Number: header.Number, + Number: firstV2BlockHeader.Number, } quorumCert := &utils.QuorumCert{ ProposedBlockInfo: blockInfo, @@ -148,7 +150,7 @@ func (x *XDPoS_v2) Initial(chain consensus.ChainReader, header *types.Header, ma x.highestQuorumCert = quorumCert // Initial snapshot - lastGapNum := header.Number.Uint64() - header.Number.Uint64()%x.config.Epoch - x.config.Gap + lastGapNum := firstV2BlockHeader.Number.Uint64() - firstV2BlockHeader.Number.Uint64()%x.config.Epoch - x.config.Gap lastGapHeader := chain.GetHeaderByNumber(lastGapNum) snap := newSnapshot(lastGapNum, lastGapHeader.Hash(), x.currentRound, x.highestQuorumCert, masternodes) @@ -183,6 +185,7 @@ func (x *XDPoS_v2) Prepare(chain consensus.ChainReader, header *types.Header) er x.lock.RUnlock() if header.ParentHash != highestQC.ProposedBlockInfo.Hash { + log.Warn("[Prepare] parent hash and QC hash does not match", "blockNum", header.Number, "parentHash", header.ParentHash, "QCHash", highestQC.ProposedBlockInfo.Hash, "QCNumber", highestQC.ProposedBlockInfo.Number) return consensus.ErrNotReadyToPropose } @@ -569,14 +572,11 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade return utils.ErrInvalidV2Extra } quorumCert := decodedExtraField.QuorumCert - if quorumCert == nil || quorumCert.Signatures == nil || len(quorumCert.Signatures) == 0 { - return utils.ErrInvalidQC - } - - if quorumCert.ProposedBlockInfo.Hash == (common.Hash{}) { - return utils.ErrEmptyBlockInfoHash + err = x.verifyQC(chain, quorumCert) + if err != nil { + log.Warn("[verifyHeader] fail to verify QC", "QCNumber", quorumCert.ProposedBlockInfo.Number, "QCsigLength", len(quorumCert.Signatures)) + return err } - // Nonces must be 0x00..0 or 0xff..f, zeroes enforced on checkpoints if !bytes.Equal(header.Nonce[:], utils.NonceAuthVote) && !bytes.Equal(header.Nonce[:], utils.NonceDropVote) { return utils.ErrInvalidVote @@ -590,21 +590,25 @@ func (x *XDPoS_v2) verifyHeader(chain consensus.ChainReader, header *types.Heade return utils.ErrInvalidUncleHash } - // Verify v2 block that is on the epoch switch - if header.Validators != nil { - // Skip if it's the first v2 block as it wil inherit from last v1 epoch block - if header.Number.Uint64() > x.config.V2.SwitchBlock.Uint64()+1 && header.Coinbase != (common.Address{}) { - return utils.ErrInvalidCheckpointBeneficiary - } + isEpochSwitch, _, err := x.IsEpochSwitch(header) // Verify v2 block that is on the epoch switch + if err != nil { + log.Error("[verifyHeader] error when checking if header is epoch switch header", "Hash", header.Hash(), "Number", header.Number, "Error", err) + return err + } + if isEpochSwitch { if !bytes.Equal(header.Nonce[:], utils.NonceDropVote) { return utils.ErrInvalidCheckpointVote } - if len(header.Validators) == 0 { + if header.Validators == nil || len(header.Validators) == 0 { return utils.ErrEmptyEpochSwitchValidators } if len(header.Validators)%common.AddressLength != 0 { return utils.ErrInvalidCheckpointSigners } + } else { + if header.Validators != nil { + return utils.ErrInvalidFieldInNonEpochSwitch + } } // If all checks passed, validate any special fields for hard forks @@ -1000,6 +1004,15 @@ func (x *XDPoS_v2) verifyQC(blockChainReader consensus.ChainReader, quorumCert * return fmt.Errorf("Fail to verify QC due to failure in getting epoch switch info") } + if quorumCert == nil { + log.Warn("[verifyQC] QC is Nil") + return utils.ErrInvalidQC + } else if (quorumCert.ProposedBlockInfo.Number.Uint64() > x.config.V2.SwitchBlock.Uint64()) && (quorumCert.Signatures == nil || (len(quorumCert.Signatures) < x.config.V2.CertThreshold)) { + //First V2 Block QC, QC Signatures is initial nil + log.Warn("[verifyHeader] Invalid QC Signature is nil or empty", "QC", quorumCert, "QCNumber", quorumCert.ProposedBlockInfo.Number, "Signatures len", len(quorumCert.Signatures)) + return utils.ErrInvalidQC + } + var wg sync.WaitGroup wg.Add(len(quorumCert.Signatures)) var haveError error @@ -1049,6 +1062,10 @@ func (x *XDPoS_v2) processQC(blockChainReader consensus.ChainReader, quorumCert } // 2. Get QC from header and update lockQuorumCert(lockQuorumCert is the parent of highestQC) proposedBlockHeader := blockChainReader.GetHeaderByHash(quorumCert.ProposedBlockInfo.Hash) + if proposedBlockHeader == nil { + log.Error("[processQC] Block not found using the QC", "quorumCert.ProposedBlockInfo.Hash", quorumCert.ProposedBlockInfo.Hash, "quorumCert.ProposedBlockInfo.Number", quorumCert.ProposedBlockInfo.Number) + return fmt.Errorf("Block not found, number: %v, hash: %v", quorumCert.ProposedBlockInfo.Number, quorumCert.ProposedBlockInfo.Hash) + } if proposedBlockHeader.Number.Cmp(x.config.V2.SwitchBlock) > 0 { // Extra field contain parent information var decodedExtraField utils.ExtraFields_v2 @@ -1568,3 +1585,11 @@ func (x *XDPoS_v2) GetPreviousPenaltyByHash(chain consensus.ChainReader, hash co header := chain.GetHeaderByHash(epochSwitchInfo.EpochSwitchBlockInfo.Hash) return common.ExtractAddressFromBytes(header.Penalties) } + +func (x *XDPoS_v2) FindParentBlockToAssign(chain consensus.ChainReader) *types.Block { + parent := chain.GetBlock(x.highestQuorumCert.ProposedBlockInfo.Hash, x.highestQuorumCert.ProposedBlockInfo.Number.Uint64()) + if parent == nil { + log.Error("[FindParentBlockToAssign] Can not find parent block from highestQC proposedBlockInfo", "x.highestQuorumCert.ProposedBlockInfo.Hash", x.highestQuorumCert.ProposedBlockInfo.Hash, "x.highestQuorumCert.ProposedBlockInfo.Number", x.highestQuorumCert.ProposedBlockInfo.Number.Uint64()) + } + return parent +} diff --git a/consensus/XDPoS/utils/errors.go b/consensus/XDPoS/utils/errors.go index d4632727f34a..a0571ea2f2b8 100644 --- a/consensus/XDPoS/utils/errors.go +++ b/consensus/XDPoS/utils/errors.go @@ -80,9 +80,10 @@ var ( ErrEmptyEpochSwitchValidators = errors.New("empty validators list on epoch switch block") - ErrInvalidV2Extra = errors.New("Invalid v2 extra in the block") - ErrInvalidQC = errors.New("Invalid QC content") - ErrEmptyBlockInfoHash = errors.New("BlockInfo hash is empty") + ErrInvalidV2Extra = errors.New("Invalid v2 extra in the block") + ErrInvalidQC = errors.New("Invalid QC content") + ErrEmptyBlockInfoHash = errors.New("BlockInfo hash is empty") + ErrInvalidFieldInNonEpochSwitch = errors.New("Invalid field exist in a non-epoch swtich block") ) type ErrIncomingMessageRoundNotEqualCurrentRound struct { diff --git a/consensus/tests/adaptor_test.go b/consensus/tests/adaptor_test.go index 89f3bb5e44c6..f0fd64a156d3 100644 --- a/consensus/tests/adaptor_test.go +++ b/consensus/tests/adaptor_test.go @@ -226,3 +226,31 @@ func TestGetCurrentEpochSwitchBlock(t *testing.T) { assert.Equal(t, uint64(1), epochNum) } } + +func TestGetParentBlock(t *testing.T) { + blockchain, _, block900, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 900, params.TestXDPoSMockChainConfig, 0) + adaptor := blockchain.Engine().(*XDPoS.XDPoS) + + // V1 + block := adaptor.FindParentBlockToAssign(blockchain, block900) + assert.Equal(t, block, block900) + + // Initialise + err := adaptor.EngineV2.Initial(blockchain, []common.Address{}) + assert.Nil(t, err) + + // V2 + blockNum := 901 + blockCoinBase := "0x111000000000000000000000000000000123" + block901 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block900, blockNum, 1, blockCoinBase, signer, signFn, nil) + blockchain.InsertBlock(block901) + + // let's inject another one, but the highestedQC has not been updated, so it shall still point to 900 + blockNum = 902 + block902 := CreateBlock(blockchain, params.TestXDPoSMockChainConfig, block901, blockNum, 1, blockCoinBase, signer, signFn, nil) + blockchain.InsertBlock(block902) + + block = adaptor.FindParentBlockToAssign(blockchain, block902) + + assert.Equal(t, block900.Hash(), block.Hash()) +} diff --git a/consensus/tests/authorised_masternode_test.go b/consensus/tests/authorised_masternode_test.go index 5c59e5a971c3..fe86f3bca105 100644 --- a/consensus/tests/authorised_masternode_test.go +++ b/consensus/tests/authorised_masternode_test.go @@ -103,18 +103,18 @@ func TestIsYourTurnConsensusV2(t *testing.T) { blockchain.InsertBlock(currentBlock) // Less then Mine Period - isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7")) + isYourTurn, err := adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e")) assert.Nil(t, err) assert.False(t, isYourTurn) time.Sleep(time.Duration(minePeriod) * time.Second) - // The first address is valid - isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7")) + // The second address is valid as the round starting from 1 + isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e")) assert.Nil(t, err) assert.True(t, isYourTurn) - // The second and third address are not valid - isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e")) + // The first and third address are not valid + isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7")) assert.Nil(t, err) assert.False(t, isYourTurn) isYourTurn, err = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7")) @@ -127,14 +127,14 @@ func TestIsYourTurnConsensusV2(t *testing.T) { blockchain.InsertBlock(currentBlock) time.Sleep(time.Duration(minePeriod) * time.Second) - adaptor.EngineV2.SetNewRoundFaker(1, false) - isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc703c4b2bD70c169f5717101CaeE543299Fc946C7")) + adaptor.EngineV2.SetNewRoundFaker(2, false) + isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e")) assert.False(t, isYourTurn) - isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc0D3ab14BBaD3D99F4203bd7a11aCB94882050E7e")) + isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7")) assert.True(t, isYourTurn) - isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc71562b71999873DB5b286dF957af199Ec94617F7")) + isYourTurn, _ = adaptor.YourTurn(blockchain, currentBlock.Header(), common.HexToAddress("xdc5F74529C0338546f82389402a01c31fB52c6f434")) assert.False(t, isYourTurn) } diff --git a/consensus/tests/test_helper.go b/consensus/tests/test_helper.go index 4dd8567a63ce..1f7173d6d682 100644 --- a/consensus/tests/test_helper.go +++ b/consensus/tests/test_helper.go @@ -419,7 +419,7 @@ func PrepareXDCTestBlockChainForV2Engine(t *testing.T, numOfBlocks int, chainCon checkpointBlockNumber := lastv1BlockNumber - lastv1BlockNumber%chainConfig.XDPoS.Epoch checkpointHeader := blockchain.GetHeaderByNumber(checkpointBlockNumber) masternodes := engine.EngineV1.GetMasternodesFromCheckpointHeader(checkpointHeader) - err := engine.EngineV2.Initial(blockchain, block.Header(), masternodes) + err := engine.EngineV2.Initial(blockchain, masternodes) if err != nil { panic(err) } @@ -513,8 +513,12 @@ func CreateBlock(blockchain *BlockChain, chainConfig *params.ChainConfig, starti if err != nil { panic(fmt.Errorf("Error generate QC by creating signedHash: %v", err)) } + // Sign from acc 1, 2, 3 + acc1SignedHash := SignHashByPK(acc1Key, utils.VoteSigHash(proposedBlockInfo).Bytes()) + acc2SignedHash := SignHashByPK(acc2Key, utils.VoteSigHash(proposedBlockInfo).Bytes()) + acc3SignedHash := SignHashByPK(acc3Key, utils.VoteSigHash(proposedBlockInfo).Bytes()) var signatures []utils.Signature - signatures = append(signatures, signedHash) + signatures = append(signatures, signedHash, acc1SignedHash, acc2SignedHash, acc3SignedHash) quorumCert := &utils.QuorumCert{ ProposedBlockInfo: proposedBlockInfo, Signatures: signatures, diff --git a/miner/worker.go b/miner/worker.go index 46053004ac45..5a0a72a2fa00 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -527,7 +527,15 @@ func (self *worker) commitNewWork() { defer self.currentMu.Unlock() tstart := time.Now() - parent := self.chain.CurrentBlock() + + c := self.engine.(*XDPoS.XDPoS) + var parent *types.Block + if c != nil { + parent = c.FindParentBlockToAssign(self.chain, self.chain.CurrentBlock()) + } else { + parent = self.chain.CurrentBlock() + } + var signers map[common.Address]struct{} if parent.Hash().Hex() == self.lastParentBlockCommit { return @@ -540,7 +548,6 @@ func (self *worker) commitNewWork() { if atomic.LoadInt32(&self.mining) == 1 { // check if we are right after parent's coinbase in the list if self.config.XDPoS != nil { - c := self.engine.(*XDPoS.XDPoS) ok, err := c.YourTurn(self.chain, parent.Header(), self.coinbase) if err != nil { log.Warn("Failed when trying to commit new work", "err", err)