From 7b571dddaa0c222c36707eb9c958fc3ad6198f16 Mon Sep 17 00:00:00 2001 From: wanwiset25 Date: Wed, 19 Aug 2026 01:33:30 +0700 Subject: [PATCH] fix(XDPoS/v2): do not reject syncInfo when the sender has no TC yet --- consensus/XDPoS/engines/engine_v2/engine.go | 12 ++- consensus/XDPoS/engines/engine_v2/timeout.go | 9 +++ .../tests/engine_v2_tests/sync_info_test.go | 81 +++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index c64f493a801d..1acb0b3da86d 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/consensus/XDPoS/engines/engine_v2/engine.go @@ -684,11 +684,15 @@ func (x *XDPoS_v2) VerifySyncInfoMessage(chain consensus.ChainReader, syncInfo * log.Warn("[VerifySyncInfoMessage] SyncInfo message verification failed due to QC", "blockNum", syncInfo.HighestQuorumCert.ProposedBlockInfo.Number, "round", syncInfo.HighestQuorumCert.ProposedBlockInfo.Round, "error", err) return false, err } - err = x.verifyTC(chain, syncInfo.HighestTimeoutCert) - if err != nil { - log.Warn("[VerifySyncInfoMessage] SyncInfo message verification failed due to TC", "gapNum", syncInfo.HighestTimeoutCert.GapNumber, "round", syncInfo.HighestTimeoutCert.Round, "error", err) - return false, err + + if !isBlankTC(syncInfo.HighestTimeoutCert) { + err = x.verifyTC(chain, syncInfo.HighestTimeoutCert) + if err != nil { + log.Warn("[VerifySyncInfoMessage] SyncInfo message verification failed due to TC", "gapNum", syncInfo.HighestTimeoutCert.GapNumber, "round", syncInfo.HighestTimeoutCert.Round, "error", err) + return false, err + } } + return true, nil } diff --git a/consensus/XDPoS/engines/engine_v2/timeout.go b/consensus/XDPoS/engines/engine_v2/timeout.go index 2e4a6debf91f..e4ec6102b17b 100644 --- a/consensus/XDPoS/engines/engine_v2/timeout.go +++ b/consensus/XDPoS/engines/engine_v2/timeout.go @@ -146,6 +146,15 @@ func (x *XDPoS_v2) getTCEpochInfo(chain consensus.ChainReader, timeoutRound type } return epochInfo, nil } + +// Round 0 with no signatures is the bootstrap TC installed by New(), not a real +// certificate: a TC only comes into existence once a round has timed out. +// Since TC lives in memory, when a node restarts, it's initialized as a blank. +// Rejecting the blank would discard the whole syncInfo which could contain useful QC. +func isBlankTC(timeoutCert *types.TimeoutCert) bool { + return timeoutCert != nil && timeoutCert.Round == types.Round(0) && len(timeoutCert.Signatures) == 0 +} + func (x *XDPoS_v2) verifyTC(chain consensus.ChainReader, timeoutCert *types.TimeoutCert) error { /* 1. Get epoch master node list by gapNumber diff --git a/consensus/tests/engine_v2_tests/sync_info_test.go b/consensus/tests/engine_v2_tests/sync_info_test.go index 5c17168d8acf..ccaf3d2c48ae 100644 --- a/consensus/tests/engine_v2_tests/sync_info_test.go +++ b/consensus/tests/engine_v2_tests/sync_info_test.go @@ -103,6 +103,87 @@ func TestSkipVerifySyncInfoIfBothQcTcNotQualified(t *testing.T) { assert.Nil(t, err) } +// A node that has never formed a TC holds the bootstrap TC (round 0, no signatures) and +// puts it into every syncInfo it sends. The QC in such a message is the only way for a +// node that missed a QC to catch up, so the placeholder TC must not invalidate it. +func TestVerifySyncInfoWithNewerQCAndBootstrapTC(t *testing.T) { + blockchain, _, currentBlock, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 905, params.TestXDPoSMockChainConfig, nil) + engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2 + + // The incoming syncInfo carries the newer QC, taken from the chain head. + var incoming types.ExtraFields_v2 + if err := utils.DecodeBytesExtraFields(currentBlock.Extra(), &incoming); err != nil { + t.Fatal("Fail to decode extra data", err) + } + + // Our node sits on an older QC and has never seen a TC, exactly like a node whose + // votes fell short of the threshold at network start. + var older types.ExtraFields_v2 + if err := utils.DecodeBytesExtraFields(blockchain.GetBlockByNumber(903).Extra(), &older); err != nil { + t.Fatal("Fail to decode extra data", err) + } + bootstrapTC := &types.TimeoutCert{ + Round: types.Round(0), + Signatures: []types.Signature{}, + } + engineV2.SetPropertiesFaker(older.QuorumCert, bootstrapTC) + + syncInfoMsg := &types.SyncInfo{ + HighestQuorumCert: incoming.QuorumCert, + HighestTimeoutCert: bootstrapTC, + } + + verified, err := engineV2.VerifySyncInfoMessage(blockchain, syncInfoMsg) + assert.Nil(t, err, "the bootstrap TC must not invalidate a syncInfo whose QC is newer and valid") + assert.True(t, verified) +} + +// The exemption is deliberately narrow: it covers only the empty placeholder, and any TC +// that actually carries signatures is still put through verifyTC in full. This pins that +// boundary, so the exemption cannot be widened by sending a TC with a bogus signature set. +// +// The trade-off it also documents: a TC that would never be processed anyway - here one +// staler than the one we hold - still invalidates the whole message, discarding a QC we do +// need. Verifying only the certificate that is ahead of ours would avoid that, at the cost +// of a broader change to the syncInfo path. +func TestVerifySyncInfoStillVerifiesNonEmptyTC(t *testing.T) { + blockchain, _, currentBlock, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 905, params.TestXDPoSMockChainConfig, nil) + engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2 + + var incoming types.ExtraFields_v2 + if err := utils.DecodeBytesExtraFields(currentBlock.Extra(), &incoming); err != nil { + t.Fatal("Fail to decode extra data", err) + } + var older types.ExtraFields_v2 + if err := utils.DecodeBytesExtraFields(blockchain.GetBlockByNumber(903).Extra(), &older); err != nil { + t.Fatal("Fail to decode extra data", err) + } + + // We already hold a newer TC than the one being sent to us. + ourTC := &types.TimeoutCert{ + Round: types.Round(5), + Signatures: []types.Signature{}, + } + engineV2.SetPropertiesFaker(older.QuorumCert, ourTC) + + // Their TC carries a signature, so it is not the placeholder and gets verified: round 1 + // with gap number 0, which has no snapshot here, so verification fails. + staleTC := &types.TimeoutCert{ + Round: types.Round(1), + Signatures: []types.Signature{SignHashByPK(acc1Key, types.TimeoutSigHash(&types.TimeoutForSign{Round: types.Round(1), GapNumber: 0}).Bytes())}, + GapNumber: 0, + } + + syncInfoMsg := &types.SyncInfo{ + HighestQuorumCert: incoming.QuorumCert, + HighestTimeoutCert: staleTC, + } + + verified, err := engineV2.VerifySyncInfoMessage(blockchain, syncInfoMsg) + assert.NotNil(t, err, "a TC carrying signatures must still be verified, not exempted") + assert.False(t, verified) +} + func TestVerifySyncInfoIfTCRoundIsAtNextEpoch(t *testing.T) { blockchain, _, _, _, _, _ := PrepareXDCTestBlockChainForV2Engine(t, 905, params.TestXDPoSMockChainConfig, nil) engineV2 := blockchain.Engine().(*XDPoS.XDPoS).EngineV2