fix(blockvalidation,subtreevalidation): reject forks below a passed checkpoint and keep older-era side blocks out of block assembly

CHANGELOG.md

@@ -106,6 +106,21 @@ Upgrading from v0.16.0 changes nothing for them; upgrading from an earlier relea
 
 ### Breaking — behaviour changes on upgrade
 
+- **A block below a checkpoint the tip has passed is rejected.** Block validation now
+  applies bitcoin-sv's `bad-fork-prior-to-checkpoint` rule. A block whose height is
+  below a checkpoint the current tip has reached fails validation with that reason
+  before its transactions are checked, and again just before it is stored, in case
+  the tip passes a checkpoint meanwhile. Nothing is persisted, and the invalid-block
+  notification strikes the peer that sent it. Initial sync is unaffected: checkpoints
+  above the tip do not count. Separately, in RUNNING a block's transactions now go to
+  block assembly only when the block is not above the next height to mine (tip+1)
+  and both heights fall under the same transaction rules: the same consensus script
+  flags, as the script engine computes them, and no BIP68 activation between them.
+  Transactions from a side block in an older era no longer reach the mining
+  template. On mainnet and testnet the checkpoint rule already rejects such blocks;
+  regtest, STN and networks without checkpoints rely on this check. Nothing to
+  configure. (#2069)
+
 - **A policy-mode script error is classified by a consensus re-run.** When policy
   validation fails on `SCRIPTNUM_OVERFLOW`, `OP_COUNT`, `SCRIPT_SIZE`, `PUBKEY_COUNT`,
   `SCRIPTNUM_MINENCODE` or `STACK_SIZE`, the validator runs the scripts again under

model/checkpoint.go

@@ -43,6 +43,25 @@ func BelowCheckpoint(checkpoints []chaincfg.Checkpoint, height uint32) bool {
 	return highest > 0 && height > 0 && height <= highest
 }
 
+// ForkBelowCheckpoint reports whether a block at blockHeight forks the chain below a
+// checkpoint the tip (tipHeight) has already reached: bitcoin-sv's
+// bad-fork-prior-to-checkpoint. Checkpoints above the tip do not count, so initial sync
+// is unaffected. A block AT a checkpoint height is left to the exact hash check
+// (catchup.ValidateHeaderAgainstCheckpoints).
+func ForkBelowCheckpoint(checkpoints []chaincfg.Checkpoint, blockHeight, tipHeight uint32) bool {
+	for _, cp := range checkpoints {
+		if cp.Height < 0 {
+			continue
+		}
+
+		if h := uint32(cp.Height); h <= tipHeight && blockHeight < h {
+			return true
+		}
+	}
+
+	return false
+}
+
 // OutpointOnlyEligible is the single eligibility gate for the below-checkpoint
 // outpoint-only fast path (PR 1178): the operator opted in, the UTXO store supports
 // outpoint-only spends (Stage A is SQL-only; Aerospike deferred to Stage B), chain

model/checkpoint_eligibility_test.go

@@ -48,6 +48,35 @@ func TestBelowCheckpoint(t *testing.T) {
 	}
 }
 
+// TestForkBelowCheckpoint: bitcoin-sv's bad-fork-prior-to-checkpoint. Only checkpoints
+// the tip has reached count; a block below the highest of those is a fork below it.
+func TestForkBelowCheckpoint(t *testing.T) {
+	cps := []chaincfg.Checkpoint{{Height: 100}, {Height: 2000}, {Height: 500}}
+
+	tests := []struct {
+		name        string
+		checkpoints []chaincfg.Checkpoint
+		blockHeight uint32
+		tipHeight   uint32
+		want        bool
+	}{
+		{name: "no checkpoints", checkpoints: nil, blockHeight: 1, tipHeight: 5000, want: false},
+		{name: "tip below every checkpoint (initial sync)", checkpoints: cps, blockHeight: 50, tipHeight: 49, want: false},
+		{name: "below highest checkpoint the tip reached", checkpoints: cps, blockHeight: 499, tipHeight: 1999, want: true},
+		{name: "at highest checkpoint the tip reached", checkpoints: cps, blockHeight: 500, tipHeight: 1999, want: false},
+		{name: "above reached checkpoint, below unreached one", checkpoints: cps, blockHeight: 1500, tipHeight: 1999, want: false},
+		{name: "tip exactly at highest checkpoint", checkpoints: cps, blockHeight: 1999, tipHeight: 2000, want: true},
+		{name: "tip past all checkpoints, block above them", checkpoints: cps, blockHeight: 2001, tipHeight: 3000, want: false},
+		{name: "negative-height checkpoint ignored", checkpoints: []chaincfg.Checkpoint{{Height: -5}}, blockHeight: 1, tipHeight: 10, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, ForkBelowCheckpoint(tt.checkpoints, tt.blockHeight, tt.tipHeight))
+		})
+	}
+}
+
 // TestOutpointOnlyEligible: full truth table for the shared outpoint-only gate.
 // Every conjunct must hold; any one missing keeps the gate closed (fail-safe).
 func TestOutpointOnlyEligible(t *testing.T) {

services/blockvalidation/BlockValidation.go

@@ -2097,8 +2097,8 @@ func (u *BlockValidation) ValidateBlockWithOptions(ctx context.Context, block *m
 		// and the operator revalidation endpoint carry authoritative heights), so this reads a
 		// trusted height. Run before the (expensive) header/subtree work so a checkpoint-
 		// conflicting block costs nothing. On the catchup path the header was already checked,
-		// so this is a cheap, harmless re-assertion. (Rejecting sub-checkpoint forks at
-		// non-checkpoint heights — the fork-depth rule — is a separate, broader change.)
+		// so this is a cheap, harmless re-assertion. Forks below a checkpoint at non-checkpoint
+		// heights are rejected by rejectForkBelowCheckpoint, right below.
 		if err = catchup.ValidateHeaderAgainstCheckpoints(block.Header, block.Height, u.settings.ChainCfgParams.Checkpoints); err != nil {
 			if !opts.IsRevalidation {
 				u.rejectFinalHeaderVerdict(block, opts.PeerID, baseURL, err.Error())
@@ -2107,6 +2107,13 @@ func (u *BlockValidation) ValidateBlockWithOptions(ctx context.Context, block *m
 			return errors.NewBlockInvalidError("[ValidateBlock][%s] block conflicts with hardcoded checkpoint", block.Hash().String(), err)
 		}
 
+		// Fork-below-checkpoint rule, before any subtree work. The non-optimistic path runs it
+		// again just before the block is stored, in case the tip passes a checkpoint while the
+		// subtrees validate.
+		if err = u.rejectForkBelowCheckpoint(ctx, block, opts, baseURL); err != nil {
+			return err
+		}
+
 		// Use cached headers if available (during catchup), otherwise fetch from blockchain.
 		//
 		// PRECONDITION: a caller passing CachedHeaders MUST also set DisableOptimisticMining.
@@ -2810,6 +2817,18 @@ func (u *BlockValidation) ValidateBlockWithOptions(ctx context.Context, block *m
 
 			u.logger.Infof("[ValidateBlock][%s] validating block DONE", block.Hash().String())
 
+			// Re-check the checkpoint fork rule: the tip can pass a checkpoint while the
+			// subtrees validate. Like the store below, it must not fail on a cancelled
+			// parent context after the subtree work is done.
+			recheckCtx, recheckCancel := context.WithTimeout(context.Background(), 30*time.Second)
+			err = u.rejectForkBelowCheckpoint(recheckCtx, block, opts, baseURL)
+			recheckCancel()
+
+			if err != nil {
+				releaseBlockNodes(block)
+				return err
+			}
+
 			// if valid, store the block (or update it if revalidating)
 			u.logger.Infof("[ValidateBlock][%s] adding block to blockchain", block.Hash().String())
 
@@ -2995,6 +3014,38 @@ func (u *BlockValidation) storeInvalidBlock(ctx context.Context, block *model.Bl
 	u.kafkaNotifyBlockInvalid(block, reason, peerID, peerURL)
 }
 
+// rejectForkBelowCheckpoint applies bitcoin-sv's bad-fork-prior-to-checkpoint rule
+// (gap-analysis 4697): a block below a checkpoint the tip has already passed is invalid.
+// Such a block is validated at its own height, so below Genesis or Chronicle its txs get
+// the older script rules (issue 4940). Only blocks at or below the highest checkpoint can
+// trip it, so the tip lookup is skipped for every block above it. ValidateBlock runs it
+// before subtree work and, on the non-optimistic path, again just before storing the
+// block, since the tip can pass a checkpoint in between during initial sync. The
+// optimistic path stores the block before any subtree work, right after the first check.
+func (u *BlockValidation) rejectForkBelowCheckpoint(ctx context.Context, block *model.Block, opts *ValidateBlockOptions, baseURL string) error {
+	checkpoints := u.settings.ChainCfgParams.Checkpoints
+	if !model.BelowCheckpoint(checkpoints, block.Height) {
+		return nil
+	}
+
+	_, tipMeta, err := u.blockchainClient.GetBestBlockHeader(ctx)
+	if err != nil {
+		return errors.NewServiceError("[ValidateBlock][%s] failed to get best block header for the checkpoint fork rule", block.Hash().String(), err)
+	}
+
+	if !model.ForkBelowCheckpoint(checkpoints, block.Height, tipMeta.Height) {
+		return nil
+	}
+
+	reason := fmt.Sprintf("bad-fork-prior-to-checkpoint: block height %d is below a checkpoint the tip (height %d) has passed", block.Height, tipMeta.Height)
+
+	if !opts.IsRevalidation {
+		u.rejectFinalHeaderVerdict(block, opts.PeerID, baseURL, reason)
+	}
+
+	return errors.NewBlockInvalidError("[ValidateBlock][%s] %s", block.Hash().String(), reason)
+}
+
 // rejectFinalHeaderVerdict handles a FINAL verdict reached before the body is bound to the
 // header: the header declares a target easier than the network limit, fails to meet the target
 // it declares, conflicts with a hardcoded checkpoint, carries the wrong difficulty bits for its

services/blockvalidation/checkpoint_callsite_test.go

@@ -52,8 +52,10 @@ func TestCheckpointValidationCallSites(t *testing.T) {
 					client, err := blockchain.NewLocalClient(ulogger.TestLogger{}, settings, store, nil, nil)
 					require.NoError(t, err)
 
-					// Reach a synthetic checkpoint through real SQL chain state. A new fork
-					// at height one must no longer inherit the historical difficulty skip.
+					// Build real SQL chain state up to height 2, below a synthetic checkpoint at
+					// 3 that the tip has not reached yet (a checkpoint the tip has passed rejects
+					// the fork outright, see TestValidateBlock_RejectsForkBelowCheckpoint). A new
+					// fork at height one must no longer inherit the historical difficulty skip.
 					prev := settings.ChainCfgParams.GenesisHash
 					for height := uint32(1); height <= 2; height++ {
 						header := settings.ChainCfgParams.GenesisBlock.Header
@@ -68,7 +70,7 @@ func TestCheckpointValidationCallSites(t *testing.T) {
 						require.NoError(t, client.AddBlock(ctx, block, "test"))
 						prev = block.Hash()
 					}
-					settings.ChainCfgParams.Checkpoints = []chaincfg.Checkpoint{{Height: 2, Hash: prev}}
+					settings.ChainCfgParams.Checkpoints = []chaincfg.Checkpoint{{Height: 3, Hash: &chainhash.Hash{3}}}
 					_, best, err := client.GetBestBlockHeader(ctx)
 					require.NoError(t, err)
 					require.Equal(t, uint32(2), best.Height)

services/blockvalidation/fork_below_checkpoint_test.go

@@ -0,0 +1,142 @@
+package blockvalidation
+
+import (
+	"context"
+	"sync/atomic"
+	"testing"
+
+	subtreepkg "github.com/bsv-blockchain/go-subtree"
+	"github.com/bsv-blockchain/teranode/errors"
+	"github.com/bsv-blockchain/teranode/model"
+	"github.com/bsv-blockchain/teranode/pkg/fileformat"
+	"github.com/bsv-blockchain/teranode/services/subtreevalidation"
+	"github.com/bsv-blockchain/teranode/ulogger"
+	"github.com/bsv-blockchain/teranode/util/test"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateBlock_RejectsForkBelowCheckpoint pins bitcoin-sv's bad-fork-prior-to-checkpoint
+// on the direct (non-catchup) path. A side block below a checkpoint the tip has passed is
+// validated at its own height, so below Genesis or Chronicle it gets the older script rules and
+// its txs can reach block assembly (issue 4940).
+func TestValidateBlock_RejectsForkBelowCheckpoint(t *testing.T) {
+	initPrometheusMetrics()
+
+	ctx := context.Background()
+
+	utxoStore, subtreeValidationClient, _, txStore, subtreeStore, cleanup := setup(t)
+	defer cleanup()
+
+	countingClient := &countingSubtreeValidationClient{Interface: subtreeValidationClient}
+
+	tSettings := test.CreateBaseTestSettings(t)
+	tSettings.BlockValidation.OptimisticMining = false
+
+	// Checkpoint at 1000, block at height 1.
+	block, mockBlockchain, _ := belowCheckpointPoWFixture(t, tSettings)
+
+	// The fixture's header deliberately fails its own target; make it pass so the block
+	// reaches the checkpoint rules.
+	nBits, err := model.NewNBitFromString("207fffff")
+	require.NoError(t, err)
+	block.Header.Bits = *nBits
+	for ok, _, _ := block.Header.HasMetTargetDifficulty(); !ok; ok, _, _ = block.Header.HasMetTargetDifficulty() {
+		block.Header.Nonce++
+	}
+
+	// The tip has passed the checkpoint.
+	for _, call := range mockBlockchain.ExpectedCalls {
+		if call.Method == "GetBestBlockHeader" {
+			call.ReturnArguments[1] = &model.BlockHeaderMeta{Height: 1500}
+		}
+	}
+	mockBlockchain.On("GetBlockExists", mock.Anything, block.Header.Hash()).Return(false, nil).Maybe()
+	mockBlockchain.On("AddBlock", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
+
+	bv := NewBlockValidation(ctx, ulogger.TestLogger{}, tSettings, mockBlockchain, subtreeStore, txStore, utxoStore, nil, countingClient)
+
+	err = bv.ValidateBlock(ctx, block, "test")
+	require.Error(t, err)
+	require.True(t, errors.Is(err, errors.ErrBlockInvalid), "a fork below the checkpoint must surface as BlockInvalid, got: %v", err)
+	require.Contains(t, err.Error(), "bad-fork-prior-to-checkpoint")
+
+	require.Equal(t, int32(0), countingClient.checkBlockSubtreesCalls.Load(),
+		"a fork below the checkpoint must be rejected before its transactions are validated")
+	mockBlockchain.AssertNotCalled(t, "AddBlock", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+// TestValidateBlock_RejectsForkBelowCheckpointPassedDuringValidation pins the re-check
+// before the block is stored: the tip is below the checkpoint when validation starts and
+// passes it while the block's subtrees are checked, so the block must not be stored.
+func TestValidateBlock_RejectsForkBelowCheckpointPassedDuringValidation(t *testing.T) {
+	initPrometheusMetrics()
+
+	ctx := context.Background()
+
+	utxoStore, subtreeValidationClient, _, txStore, subtreeStore, cleanup := setup(t)
+	defer cleanup()
+
+	passingClient := &passingSubtreeValidationClient{Interface: subtreeValidationClient}
+
+	tSettings := test.CreateBaseTestSettings(t)
+	tSettings.BlockValidation.OptimisticMining = false
+
+	// Checkpoint at 1000, block at height 1.
+	block, mockBlockchain, subtree := belowCheckpointPoWFixture(t, tSettings)
+
+	subtreeBytes, err := subtree.Serialize()
+	require.NoError(t, err)
+	require.NoError(t, subtreeStore.Set(ctx, subtree.RootHash()[:], fileformat.FileTypeSubtree, subtreeBytes))
+
+	subtreeMetaBytes, err := subtreepkg.NewSubtreeMeta(subtree).Serialize()
+	require.NoError(t, err)
+	require.NoError(t, subtreeStore.Set(ctx, subtree.RootHash()[:], fileformat.FileTypeSubtreeMeta, subtreeMetaBytes))
+
+	replicated := subtree.Duplicate()
+	replicated.ReplaceRootNode(block.CoinbaseTx.TxIDChainHash(), 0, uint64(block.CoinbaseTx.Size())) //nolint:gosec
+	block.Header.HashMerkleRoot = replicated.RootHash()
+
+	nBits, err := model.NewNBitFromString("207fffff")
+	require.NoError(t, err)
+	block.Header.Bits = *nBits
+	for ok, _, _ := block.Header.HasMetTargetDifficulty(); !ok; ok, _, _ = block.Header.HasMetTargetDifficulty() {
+		block.Header.Nonce++
+	}
+
+	// The first tip lookup is below the checkpoint, every later one has passed it.
+	for _, call := range mockBlockchain.ExpectedCalls {
+		if call.Method == "GetBestBlockHeader" {
+			call.ReturnArguments[1] = &model.BlockHeaderMeta{Height: 900}
+			call.Once()
+		}
+	}
+	mockBlockchain.On("GetBestBlockHeader", mock.Anything).Return(block.Header, &model.BlockHeaderMeta{Height: 1500}, nil)
+	mockBlockchain.On("GetNextWorkRequired", mock.Anything, mock.Anything, mock.Anything).Return(nBits, nil).Maybe()
+	mockBlockchain.On("GetBlockHeaderIDs", mock.Anything, mock.Anything, mock.Anything).Return([]uint32{0}, nil).Maybe()
+	mockBlockchain.On("SetBlockSubtreesSet", mock.Anything, mock.Anything).Return(nil).Maybe()
+	mockBlockchain.On("GetBlockHeadersFromHeight", mock.Anything, mock.Anything, mock.Anything).
+		Return(nil, nil, errors.NewNotFoundError("no main-chain header")).Maybe()
+	mockBlockchain.On("GetBlockExists", mock.Anything, block.Header.Hash()).Return(false, nil).Maybe()
+	mockBlockchain.On("AddBlock", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
+
+	bv := NewBlockValidation(ctx, ulogger.TestLogger{}, tSettings, mockBlockchain, subtreeStore, txStore, utxoStore, nil, passingClient)
+
+	err = bv.ValidateBlock(ctx, block, "test")
+	require.Error(t, err)
+	require.True(t, errors.Is(err, errors.ErrBlockInvalid), "a fork below the checkpoint must surface as BlockInvalid, got: %v", err)
+	require.Contains(t, err.Error(), "bad-fork-prior-to-checkpoint")
+	require.Equal(t, int32(1), passingClient.checkBlockSubtreesCalls.Load(), "the block must reach subtree validation before the tip passes the checkpoint")
+	mockBlockchain.AssertNotCalled(t, "AddBlock", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+// passingSubtreeValidationClient reports every block's subtrees valid and counts the calls.
+type passingSubtreeValidationClient struct {
+	subtreevalidation.Interface
+	checkBlockSubtreesCalls atomic.Int32
+}
+
+func (c *passingSubtreeValidationClient) CheckBlockSubtrees(context.Context, *model.Block, string, string) error {
+	c.checkBlockSubtreesCalls.Add(1)
+	return nil
+}

services/subtreevalidation/Server.go

@@ -968,8 +968,9 @@ func (u *Server) checkSubtreeFromBlock(ctx context.Context, request *subtreevali
 		// enabled in RUNNING): a floater child blessed at the candidate
 		// height and added to assembly is the same transaction policy-mode
 		// admission would have accepted into assembly anyway (policy
-		// substitutes tip+1 for unconfirmed parents — equal to the candidate
-		// height at the tip, and era flags cannot differ post-Genesis).
+		// substitutes tip+1 for unconfirmed parents). Assembly is fed only
+		// when the candidate height and tip+1 share the same transaction
+		// rules; see feedsBlockAssembly.
 		// Accepted-block txs are mined-removed from assembly as always.
 		validatorOptions := []validator.Option{
 			validator.WithSkipPolicyChecks(true),
@@ -987,10 +988,16 @@ func (u *Server) checkSubtreeFromBlock(ctx context.Context, request *subtreevali
 
 		// While catching up blocks, disable adding transactions to block
 		// assembly: bulk-history txs do not belong in our template. In
-		// RUNNING, assembly stays enabled so txs from a legacy-bridge tip
-		// block survive in the mempool if the block loses a reorg.
-		// IDLE is included: an operator STOP can land while catchup is running.
-		if *currentState != blockchain.FSMStateRUNNING {
+		// RUNNING, assembly stays enabled so txs from a legacy-bridge block
+		// survive in the mempool if the block loses a reorg, unless the block
+		// is above tip+1 or under different transaction rules from tip+1. See
+		// feedsBlockAssembly.
+		feeds, err := u.feedsBlockAssembly(ctx, *currentState, request.BlockHeight)
+		if err != nil {
+			return false, errors.NewProcessingError("[CheckSubtree] Failed to decide block assembly feed", err)
+		}
+
+		if !feeds {
 			validatorOptions = append(validatorOptions, validator.WithAddTXToBlockAssembly(false))
 		}