fix(core): queue all future blocks of a batch instead of failing

consensus/XDPoS/engines/engine_v1/verify_header_test.go

@@ -0,0 +1,117 @@
+package engine_v1
+
+import (
+	"math/big"
+	"testing"
+	"time"
+
+	"github.com/XinFinOrg/XDPoSChain/common"
+	"github.com/XinFinOrg/XDPoSChain/consensus"
+	"github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils"
+	"github.com/XinFinOrg/XDPoSChain/core/types"
+	"github.com/XinFinOrg/XDPoSChain/params"
+	"github.com/stretchr/testify/assert"
+)
+
+// futureTestChainReader mimics a chain that holds no verifiable ancestors: it
+// hands out no headers, so any parent lookup has to fall back to the batch of
+// headers passed to the engine.
+type futureTestChainReader struct {
+	consensus.ChainReader // nil; methods other than the overrides below are only reachable under a regressed engine ordering
+	config                *params.ChainConfig
+}
+
+func (r *futureTestChainReader) Config() *params.ChainConfig {
+	return r.config
+}
+
+func (r *futureTestChainReader) GetHeader(common.Hash, uint64) *types.Header {
+	return nil
+}
+
+func (r *futureTestChainReader) GetHeaderByNumber(uint64) *types.Header {
+	return nil
+}
+
+func (r *futureTestChainReader) GetHeaderByHash(common.Hash) *types.Header {
+	return nil
+}
+
+func (r *futureTestChainReader) GetBlock(common.Hash, uint64) *types.Block {
+	return nil
+}
+
+func (r *futureTestChainReader) GetBlockByNumber(uint64) *types.Block {
+	return nil
+}
+
+func (r *futureTestChainReader) CurrentHeader() *types.Header {
+	return nil
+}
+
+// futureTestHeader builds a header that passes all standalone v1 checks so that
+// only the timestamp/parent ordering decides its fate.
+func futureTestHeader(config *params.ChainConfig, number int64, parentHash common.Hash, timestamp uint64) *types.Header {
+	header := &types.Header{
+		Number:     big.NewInt(number),
+		ParentHash: parentHash,
+		Difficulty: big.NewInt(1),
+		GasLimit:   1200000000,
+		Time:       timestamp,
+		Extra:      make([]byte, utils.ExtraVanity+utils.ExtraSeal),
+		UncleHash:  utils.UncleHash,
+	}
+	if config.IsEIP1559(header.Number) {
+		header.BaseFee = params.BaseFeeForBlock(config, header.Number)
+	}
+	return header
+}
+
+// TestFutureTimestampCheckPrecedesParentLookup pins the engine premise that the
+// insertChain future-batch handling relies on: the timestamp check runs before
+// the parent lookup, so a header whose parent is in the same batch and whose
+// timestamp is in the future surfaces as ErrFutureBlock, never as
+// ErrUnknownAncestor. If the checks are ever reordered, children of a future
+// block stop being classified as future blocks and insertChain treats a valid
+// delivery as an invalid chain, which makes the downloader drop the peer.
+func TestFutureTimestampCheckPrecedesParentLookup(t *testing.T) {
+	config := *params.TestXDPoSMockChainConfig
+	xdpos := *config.XDPoS
+	xdpos.SkipV1Validation = false // the timestamp check is part of full v1 validation
+	config.XDPoS = &xdpos
+
+	engine := New(&config, nil)
+	reader := &futureTestChainReader{config: &config}
+
+	now := uint64(time.Now().Unix())
+	block1 := futureTestHeader(&config, 1, common.Hash{}, now)
+	// Timestamp in the future and parent not resolvable from the reader: with
+	// the checks in the wrong order this header would answer ErrUnknownAncestor.
+	block2 := futureTestHeader(&config, 2, block1.Hash(), now+10000)
+
+	// Single-header path: the parent (block1) is neither in a batch nor in the
+	// database, so the parent lookup fails unless the timestamp check wins.
+	err := engine.VerifyHeader(reader, block2, true)
+	assert.Equal(t, consensus.ErrFutureBlock, err)
+
+	// Batch path: the parent (block1) is in the same batch, so the parent
+	// lookup can always succeed and must not mask the future classification.
+	headers := []*types.Header{block1, block2}
+	abort := make(chan struct{})
+	results := make(chan error, len(headers))
+	engine.VerifyHeaders(reader, headers, []bool{true, true}, abort, results)
+	for i := 0; i < len(headers); i++ {
+		select {
+		case result := <-results:
+			if i == 0 {
+				// block1 has no verifiable ancestor on the stub reader and no
+				// seal to recover; only the future classification of block2 is
+				// under test.
+				continue
+			}
+			assert.Equal(t, consensus.ErrFutureBlock, result)
+		case <-time.After(5 * time.Second):
+			t.Fatalf("timed out waiting for verify result %d", i)
+		}
+	}
+}

consensus/tests/engine_v2_tests/future_block_test.go

@@ -0,0 +1,85 @@
+package engine_v2_tests
+
+import (
+	"testing"
+	"time"
+
+	"github.com/XinFinOrg/XDPoSChain/core/rawdb"
+	"github.com/XinFinOrg/XDPoSChain/core/types"
+	"github.com/XinFinOrg/XDPoSChain/params"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestInsertChainQueuesFutureBatchEndToEnd combines the two halves the
+// future-batch fix relies on: the real XDPoS engine classifying children of a
+// future block as ErrFutureBlock (timestamp check precedes the parent lookup),
+// and insertChain consuming those results by queueing the whole tail instead
+// of failing the import. The core stub-based tests pin the loop mechanics and
+// the engine tests pin the classification; neither covers their composition,
+// which is the exact online scenario the fix targets.
+//
+// Deliberately not skipped in -short mode: the whole run (910-block harness
+// build included) stays well under a second, and this is the only test that
+// covers the composition.
+func TestInsertChainQueuesFutureBatchEndToEnd(t *testing.T) {
+	config := params.TestXDPoSMockChainConfig
+	blockchain, _, tip910, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 910, config, nil)
+	t.Cleanup(blockchain.Stop)
+	db := blockchain.ChainDb()
+
+	// Build blocks 911..915 in memory only; none is written into the DB.
+	switchBlock := config.XDPoS.V2.SwitchBlock.Int64()
+	blocks := make([]*types.Block, 0, 5)
+	current := tip910
+	for n := 911; n <= 915; n++ {
+		block := CreateBlock(
+			blockchain,
+			blockchain.Config(),
+			current,
+			n,
+			int64(n)-switchBlock,
+			signer.Hex(),
+			signer,
+			signFn,
+			nil,
+			nil,
+			"",
+		)
+		blocks = append(blocks, block)
+		current = block
+	}
+
+	// Re-timestamp the tail (912..915) into the near future. The hash changes
+	// so the QC no longer matches, which is fine: the timestamp check also
+	// precedes QC verification. Each re-stamped header also breaks the link to
+	// its built child, so the parents are re-chained along the clone line. Park
+	// the tail 15s inside addFutureBlock's 30s acceptance window: addFutureBlock
+	// re-reads time.Now() at enqueue time, so the 15s of slack also absorbs a
+	// wall-clock step back between stamping here and the enqueue check (a 25s
+	// offset would leave only 5s and fail on a ~6s NTP step back), and it stays
+	// far beyond the 100ms futureBlocksLoop period, so the queue cannot drain
+	// before the assertions run.
+	now := uint64(time.Now().Unix())
+	var batch types.Blocks
+	batch = append(batch, blocks[0]) // 911 keeps a past timestamp and imports
+	parent := blocks[0]
+	for _, block := range blocks[1:] {
+		header := block.Header()
+		header.ParentHash = parent.Hash()
+		header.Time = now + 15
+		parent = types.NewBlockWithHeader(header).WithBody(*block.Body())
+		batch = append(batch, parent)
+	}
+
+	n, err := blockchain.InsertChain(batch)
+	assert.Nil(t, err, "a future tail must not fail the import, the downloader would drop the peer")
+	assert.Equal(t, len(batch), n)
+
+	// The prefix imported, the tail did not reach the database.
+	assert.Equal(t, blocks[0].NumberU64(), blockchain.CurrentBlock().Number.Uint64())
+	assert.Equal(t, blocks[0].Hash(), blockchain.CurrentBlock().Hash())
+	for _, block := range batch[1:] {
+		assert.Nil(t, blockchain.GetBlockByNumber(block.NumberU64()), "future block %d must stay queued", block.NumberU64())
+		assert.Nil(t, rawdb.ReadBadBlock(db, block.Hash()), "future block %d must not be reported as bad block", block.NumberU64())
+	}
+}

consensus/tests/engine_v2_tests/verify_header_test.go

@@ -687,3 +687,81 @@ func TestVerifyHeadersDoesNotFabricateBatchBlocksForHookPenalty(t *testing.T) {
 		}
 	}
 }
+
+// TestFutureTimestampCheckPrecedesParentLookup pins the engine premise that the
+// insertChain future-batch handling relies on: the timestamp check runs before
+// the parent lookup, so a header whose parent is in the same batch and whose
+// timestamp is in the future surfaces as ErrFutureBlock, never as
+// ErrUnknownAncestor. If the checks are ever reordered, children of a future
+// block stop being classified as future blocks and insertChain treats a valid
+// delivery as an invalid chain, which makes the downloader drop the peer.
+func TestFutureTimestampCheckPrecedesParentLookup(t *testing.T) {
+	skipLongInShortMode(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)
+	// Block 901 is the first v2 block with round of 1
+	blockchain, _, block910, signer, signFn, _ := PrepareXDCTestBlockChainForV2Engine(t, 910, &config, nil)
+	adaptor := blockchain.Engine().(*XDPoS.XDPoS)
+
+	// Build blocks 911 and 912 in memory only; neither is written into the DB.
+	block911 := CreateBlock(
+		blockchain,
+		blockchain.Config(),
+		block910,
+		911,
+		int64(911)-config.XDPoS.V2.SwitchBlock.Int64(),
+		signer.Hex(),
+		signer,
+		signFn,
+		nil,
+		nil,
+		"",
+	)
+	block912 := CreateBlock(
+		blockchain,
+		blockchain.Config(),
+		block911,
+		912,
+		int64(912)-config.XDPoS.V2.SwitchBlock.Int64(),
+		signer.Hex(),
+		signer,
+		signFn,
+		nil,
+		nil,
+		"",
+	)
+
+	// Re-timestamp 912 into the future. The hash changes so the QC no longer
+	// matches, which is fine: the timestamp check also precedes QC verification.
+	futureHeader := block912.Header()
+	futureHeader.Time = uint64(time.Now().Unix() + 10000)
+
+	// Batch path: the parent (911) is in the same batch, so the parent lookup
+	// can always succeed and must not mask the future classification.
+	headers := []*types.Header{block911.Header(), futureHeader}
+	fullVerifies := []bool{true, true}
+	_, results := adaptor.VerifyHeaders(blockchain, headers, fullVerifies)
+	for i := 0; i < len(headers); i++ {
+		select {
+		case result := <-results:
+			if i == 0 {
+				assert.Nil(t, result)
+				continue
+			}
+			assert.Equal(t, consensus.ErrFutureBlock, result)
+		case <-time.After(5 * time.Second):
+			t.Fatalf("timed out waiting for verify result %d", i)
+		}
+	}
+
+	// Single-header path: the parent (911) is neither in a batch nor in the DB,
+	// so an engine that checks the parent before the timestamp would answer
+	// ErrUnknownAncestor here instead of ErrFutureBlock.
+	err = adaptor.VerifyHeader(blockchain, futureHeader, true)
+	assert.Equal(t, consensus.ErrFutureBlock, err)
+}