From 6bb33326bba648ee4edde552c7fd8097bab624de Mon Sep 17 00:00:00 2001 From: Siggi Date: Thu, 29 Oct 2026 14:16:00 +0000 Subject: [PATCH] fix(blockvalidation): check difficulty of every catch-up header before fetching blocks Catch-up's header precheck (step 9.5) recomputed the DAA from the fetched headers alone, so it skipped every header whose 144-block window reached below the common ancestor - the first ~147 of any fork - as well as pre-DAA headers and chains with no difficulty adjustment. A short fork mined against a target the peer chose, including one header on top of our tip, passed into step 10, where the subtree prewarm downloads block data before any block is validated. The precheck now uses blockchain.Difficulty.CalcNextWorkRequiredFrom, the calculator full-block validation uses, over a header source in which the fetched headers answer for themselves and the blockchain client answers for the ancestor and everything below it. Every header is judged, and the check fails closed: a wrong nBits is a malicious peer, and a requirement that cannot be computed stops catch-up without blaming the peer. The header source is the legacy header cache's branchSource, moved to services/blockchain as NewBranchSource so both callers share it. ComputeTarget was exported only for the old in-memory precheck and is folded back into Difficulty.computeTarget. Stub-client catch-up fixtures now declare NoDifficultyAdjustment, since the precheck reads ancestry the stub does not have, and the Sybil fixture is anchored on a 207fffff chain so its honest chain repeats its anchor's bits. --- CHANGELOG.md | 14 + services/blockchain/Difficulty.go | 15 +- .../branch_source.go} | 65 ++-- services/blockvalidation/BlockValidation.go | 13 +- services/blockvalidation/catchup.go | 6 +- .../catchup_ancestor_link_test.go | 33 ++ .../blockvalidation/catchup_difficulty.go | 176 ++++----- .../catchup_difficulty_test.go | 361 ++++++++---------- .../catchup_malicious_peer_test.go | 19 +- services/blockvalidation/catchup_test.go | 6 + .../checkpoint_prefix_nbits_test.go | 39 +- services/legacy/netsync/header_cache.go | 35 +- 12 files changed, 363 insertions(+), 419 deletions(-) rename services/{legacy/netsync/header_source.go => blockchain/branch_source.go} (58%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ee315d202..0602ff256b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -877,6 +877,20 @@ Upgrading from v0.16.0 changes nothing for them; upgrading from an earlier relea ### Not breaking, but worth knowing +- **Catch-up checks the difficulty of every offered header before fetching any + block.** The header precheck used to skip the first ~147 headers after the + common ancestor, every pre-DAA header and chains that never adjust difficulty, + leaving them to block validation after the subtree prewarm had already fetched + their data. A short fork mined against a target the peer chose could therefore + make the node download block data. Every header is now checked against the + same calculator block validation uses, reading the window below the ancestor + from the blockchain store. A peer offering a wrong `nBits` is reported as + malicious (`invalid_difficulty`) before any body is requested, and catch-up + fails without blaming the peer when the expected difficulty cannot be read. + Expect a few hundred extra blockchain store reads per catch-up. Block + validation's own check is unchanged. No action is required. + (#2084) + - **Peer subtrees announced during a catchup are validated afterwards instead of lost.** While catching up, the subtree validator used to drop every peer-announced subtree, so the next block arrived with all of them unvalidated diff --git a/services/blockchain/Difficulty.go b/services/blockchain/Difficulty.go index 02aa4eb13a..d1552dc5cc 100644 --- a/services/blockchain/Difficulty.go +++ b/services/blockchain/Difficulty.go @@ -359,21 +359,8 @@ func (d *Difficulty) calcHistoricalWorkRequired(ctx context.Context, src HeaderS // - suitableFirstBlock: First block in the difficulty adjustment window // - suitableLastBlock: Last block in the difficulty adjustment window func (d *Difficulty) computeTarget(suitableFirstBlock *model.SuitableBlock, suitableLastBlock *model.SuitableBlock) (*model.NBit, error) { - return ComputeTarget(d.settings, suitableFirstBlock, suitableLastBlock) -} + tSettings := d.settings -// ComputeTarget calculates the DAA target difficulty from the first and last suitable -// blocks of the difficulty adjustment window. It is the pure arithmetic core shared by -// the store-backed Difficulty.CalcNextWorkRequired path and by callers that reconstruct -// the window from headers not yet persisted (e.g. block validation catchup). It reads -// only the ChainWork and Time of the two suitable blocks plus the chain parameters, so -// it makes no store access and holds no state. -// -// Parameters: -// - tSettings: chain settings (TargetTimePerBlock, PowLimit, NoDifficultyAdjustment) -// - suitableFirstBlock: first (older) suitable block in the window -// - suitableLastBlock: last (newer) suitable block in the window -func ComputeTarget(tSettings *settings.Settings, suitableFirstBlock *model.SuitableBlock, suitableLastBlock *model.SuitableBlock) (*model.NBit, error) { lastSuitableBits, _ := model.NewNBitFromSlice(suitableLastBlock.NBits) // If regtest we don't adjust the difficulty if tSettings.ChainCfgParams.NoDifficultyAdjustment { diff --git a/services/legacy/netsync/header_source.go b/services/blockchain/branch_source.go similarity index 58% rename from services/legacy/netsync/header_source.go rename to services/blockchain/branch_source.go index d0a2120f82..561ba3f98a 100644 --- a/services/legacy/netsync/header_source.go +++ b/services/blockchain/branch_source.go @@ -1,4 +1,4 @@ -package netsync +package blockchain import ( "context" @@ -6,47 +6,54 @@ import ( "github.com/bsv-blockchain/go-bt/v2/chainhash" "github.com/bsv-blockchain/teranode/errors" "github.com/bsv-blockchain/teranode/model" - "github.com/bsv-blockchain/teranode/services/blockchain" "github.com/bsv-blockchain/teranode/util" ) -// cachedHeader is what the header cache keeps about one header it holds above -// the committed tip, so the difficulty calculator can read it as it reads a -// stored block: the header, its height and its cumulative chain work, 32 bytes -// big-endian, the form the store keeps chain_work in. -type cachedHeader struct { - header *model.BlockHeader - height int32 - chainWork [32]byte +// BranchHeader is what a branch holds about one header above the trunk, so the +// difficulty calculator can read it as it reads a stored block: the header, its +// height and its cumulative chain work, 32 bytes big-endian, the form the store +// keeps chain_work in. +type BranchHeader struct { + Header *model.BlockHeader + Height uint32 + ChainWork []byte } -// branchSource is the blockchain.HeaderSource the header rules judge a cached -// header with: lookup answers for headers held above the committed tip, and -// trunk, the store or the blockchain client, answers for everything committed. -// A walk that starts in the cache crosses into the trunk at the first parent the -// cache does not hold, which is where every cached run is rooted. +// branchSource is a HeaderSource over headers that are not stored: lookup +// answers for the headers the branch holds, and trunk, the store or the +// blockchain client, answers for everything committed. A walk that starts in +// the branch crosses into the trunk at the first parent the branch does not +// hold, which is where every branch is rooted. type branchSource struct { - trunk blockchain.HeaderSource - lookup func(hash chainhash.Hash) (*cachedHeader, bool) + trunk HeaderSource + lookup func(hash chainhash.Hash) (*BranchHeader, bool) } -var _ blockchain.HeaderSource = (*branchSource)(nil) +var _ HeaderSource = (*branchSource)(nil) -func (s *branchSource) meta(c *cachedHeader) *model.BlockHeaderMeta { - return &model.BlockHeaderMeta{Height: uint32(c.height), ChainWork: append([]byte(nil), c.chainWork[:]...)} //nolint:gosec // a cached height is above the tip, never negative +// NewBranchSource returns a HeaderSource that answers from lookup for the +// headers it holds and from trunk for the rest. It lets CalcNextWorkRequiredFrom +// judge a header whose ancestry is partly not stored yet, such as one in the +// legacy header cache or one fetched from a peer during catchup. +func NewBranchSource(trunk HeaderSource, lookup func(hash chainhash.Hash) (*BranchHeader, bool)) HeaderSource { + return &branchSource{trunk: trunk, lookup: lookup} } -// GetBlockHeader answers from the cache, then the trunk. +func (s *branchSource) meta(c *BranchHeader) *model.BlockHeaderMeta { + return &model.BlockHeaderMeta{Height: c.Height, ChainWork: append([]byte(nil), c.ChainWork...)} +} + +// GetBlockHeader answers from the branch, then the trunk. func (s *branchSource) GetBlockHeader(ctx context.Context, hash *chainhash.Hash) (*model.BlockHeader, *model.BlockHeaderMeta, error) { if c, ok := s.lookup(*hash); ok { - return c.header, s.meta(c), nil + return c.Header, s.meta(c), nil } return s.trunk.GetBlockHeader(ctx, hash) } // GetBlockHeaders returns hash and then its ancestors, newest first, taking -// what the cache holds and asking the trunk for the rest in one call. +// what the branch holds and asking the trunk for the rest in one call. func (s *branchSource) GetBlockHeaders(ctx context.Context, hash *chainhash.Hash, count uint64) ([]*model.BlockHeader, []*model.BlockHeaderMeta, error) { headers := make([]*model.BlockHeader, 0, count) metas := make([]*model.BlockHeaderMeta, 0, count) @@ -59,9 +66,9 @@ func (s *branchSource) GetBlockHeaders(ctx context.Context, hash *chainhash.Hash break } - headers = append(headers, c.header) + headers = append(headers, c.Header) metas = append(metas, s.meta(c)) - cur = *c.header.HashPrevBlock + cur = *c.Header.HashPrevBlock } if remaining := count - uint64(len(headers)); remaining > 0 { @@ -77,7 +84,7 @@ func (s *branchSource) GetBlockHeaders(ctx context.Context, hash *chainhash.Hash return headers, metas, nil } -// GetHashOfAncestorBlock walks depth parents back through the cache and hands +// GetHashOfAncestorBlock walks depth parents back through the branch and hands // whatever depth is left to the trunk, which returns errors.ErrNotFound when the // chain is shorter, exactly as it would for a stored block. func (s *branchSource) GetHashOfAncestorBlock(ctx context.Context, hash *chainhash.Hash, depth int) (*chainhash.Hash, error) { @@ -89,7 +96,7 @@ func (s *branchSource) GetHashOfAncestorBlock(ctx context.Context, hash *chainha return s.trunk.GetHashOfAncestorBlock(ctx, &cur, depth) } - cur = *c.header.HashPrevBlock + cur = *c.Header.HashPrevBlock depth-- } @@ -97,9 +104,9 @@ func (s *branchSource) GetHashOfAncestorBlock(ctx context.Context, hash *chainha } // GetSuitableBlock is the store's GetSuitableBlock (stores/blockchain/sql/ -// GetSuitableBlock.go) over the cache: hash and its two parents, in the store's +// GetSuitableBlock.go) over the branch: hash and its two parents, in the store's // order (oldest first, its ORDER BY depth DESC), sorted by time with the same -// sorting network, and the middle one returned. A hash the cache does not hold +// sorting network, and the middle one returned. A hash the branch does not hold // is the trunk's to answer whole. func (s *branchSource) GetSuitableBlock(ctx context.Context, hash *chainhash.Hash) (*model.SuitableBlock, error) { if _, ok := s.lookup(*hash); !ok { diff --git a/services/blockvalidation/BlockValidation.go b/services/blockvalidation/BlockValidation.go index 4c1631f689..80b3f6e8ff 100644 --- a/services/blockvalidation/BlockValidation.go +++ b/services/blockvalidation/BlockValidation.go @@ -2228,12 +2228,13 @@ func (u *BlockValidation) ValidateBlockWithOptions(ctx context.Context, block *m // is true for EVERY height in 1..highest — and the two places that were supposed to cover // the gap do not: // - // - Catch-up's header precheck (validateCatchupHeaderDifficulty) deliberately DEFERS every - // header whose full 144-block window is not inside the fetched run — the first header - // and roughly its first 146 successors — plus every pre-DAA header, and it returns - // immediately on chains with no difficulty adjustment. Its own doc comment names this - // check as the downstream cover for exactly those headers, so skipping here removed the - // only check they ever got. + // - Catch-up's header precheck (validateCatchupHeaderDifficulty) used to DEFER every header + // whose full 144-block window was not inside the fetched run — the first header and + // roughly its first 146 successors — plus every pre-DAA header, and it returned + // immediately on chains with no difficulty adjustment, naming this check as the cover for + // exactly those headers. It now judges every header against the store + // (#2084), but only for blocks that arrive through catch-up, and this + // check stays authoritative for the rest. // - Catch-up routes blocks certified by a checkpoint verified IN THIS RUN to // quickValidateBlock, which gates on catchupCtx.highestCheckpointHeight — the verified // height, not the configured one. That routing is not absolute: tryQuickValidation falls diff --git a/services/blockvalidation/catchup.go b/services/blockvalidation/catchup.go index 5b70ce7647..18d8099ebc 100644 --- a/services/blockvalidation/catchup.go +++ b/services/blockvalidation/catchup.go @@ -350,8 +350,10 @@ func (u *Server) catchup(ctx context.Context, blockUpTo *model.Block, peerID, ba // Step 9.5: Validate header-chain difficulty (DAA) before fetching full blocks. // Header-fetch validation only checks each header's PoW against its own nBits; this - // recomputes the DAA-required nBits from the header chain and rejects a peer that - // supplied a self-consistent low-difficulty chain. + // computes the required nBits for every header, reading the window below the common + // ancestor from the store, and rejects a peer that supplied a header mined against a + // target of its own choosing. It must stay ahead of step 10, whose subtree prewarm + // fetches block data before any block is validated. if err = u.validateCatchupHeaderDifficulty(ctx, catchupCtx); err != nil { return err } diff --git a/services/blockvalidation/catchup_ancestor_link_test.go b/services/blockvalidation/catchup_ancestor_link_test.go index 19cfc8e513..62fdaafcca 100644 --- a/services/blockvalidation/catchup_ancestor_link_test.go +++ b/services/blockvalidation/catchup_ancestor_link_test.go @@ -102,6 +102,7 @@ type ancestorLinkRun struct { err error headersRequests int blocksRequests int + otherRequests int // requests for anything but headers and /blocks/, such as /subtree_data minedClears int64 breakerFailures int // failures counted on the peer's circuit breaker after the catchup errorType string // previousCatchupAttempt.ErrorType, how releaseCatchupLock classified err @@ -130,6 +131,9 @@ const ( // header the batch before it ended on. The target is f2 on f1, which the peer never serves, so // reaching the target never ends the fetch. offerReplayBetweenBatches + // offerWrongBits: h1..h5 linked on b5, where h1 is mined against bits other than its parent's. + // Regtest never adjusts difficulty, so the bits every block must carry are its parent's. + offerWrongBits ) // runAncestorLinkCatchup runs catchup() against a real sqlitememory blockchain store holding the @@ -191,6 +195,11 @@ func runAncestorLinkCatchup(t *testing.T, shape offerShape) ancestorLinkRun { offered = forkHeadersFrom(t, ancestor, 5, 6, 1000) case offerFirstOnOther: offered = forkHeadersFrom(t, otherBlockB, 5, 6, 1000) + case offerWrongBits: + h1 := forkHeadersFrom(t, ancestor, 1, 6, 1000)[0] + h1.Bits = nBitsFrom(t, "1f7fffff") + testhelpers.MineHeader(h1) + offered = append([]*model.BlockHeader{h1}, forkHeadersFrom(t, h1, 4, 7, 1000)...) case offerBreakAtSecond: offered = append(forkHeadersFrom(t, ancestor, 1, 6, 1000), forkHeadersFrom(t, otherBlockB, 4, 7, 2000)...) case offerBreakBetweenBatches, offerReplayBetweenBatches: @@ -259,6 +268,13 @@ func runAncestorLinkCatchup(t *testing.T, shape offerShape) ancestorLinkRun { }) httpmock.RegisterResponder("GET", `=~^http://peer/blocks/.*`, httpmock.NewBytesResponder(500, nil)) + var otherRequests atomic.Int64 + + httpmock.RegisterNoResponder(func(req *http.Request) (*http.Response, error) { + otherRequests.Add(1) + return httpmock.ConnectionFailure(req) + }) + // Add the headers only the later batches carry, so the target is the newest header offered and // the end-state check covers every offered block. switch shape { @@ -295,6 +311,7 @@ func runAncestorLinkCatchup(t *testing.T, shape offerShape) ancestorLinkRun { errorType: errorType, headersRequests: int(headersRequests.Load()), blocksRequests: blocksRequests, + otherRequests: int(otherRequests.Load()), minedClears: recorder.clears.Load(), client: client, mainChain: mainChain, @@ -488,3 +505,19 @@ func TestBuildHeaderCache_RejectsFirstHeaderNotLinkedToAncestor(t *testing.T) { require.Contains(t, err.Error(), "does not link to common ancestor") }) } + +// TestCatchup_HeaderWithWrongDifficultyIsRejectedBeforeAnyBodyFetch covers the #2084 fix +// against a real sqlitememory blockchain store. The peer offers a fork of depth 5 on b5 whose first +// header declares bits other than the ones the chain requires. Its whole adjustment window is in +// the store, which the header precheck used to skip, leaving the subtree prewarm and the block +// fetch to run first. Catchup must reject it at the header precheck, charge the peer as malicious, +// and request nothing but headers. +func TestCatchup_HeaderWithWrongDifficultyIsRejectedBeforeAnyBodyFetch(t *testing.T) { + run := runAncestorLinkCatchup(t, offerWrongBits) + + require.Error(t, run.err) + require.True(t, errors.IsMaliciousResponseError(run.err), "wrong difficulty bits are a malicious peer: %v", run.err) + require.Contains(t, run.err.Error(), "incorrect difficulty bits") + require.Zero(t, run.otherRequests, "nothing but headers may be requested for a rejected offer") + requireRejectedWithoutSideEffects(t, run) +} diff --git a/services/blockvalidation/catchup_difficulty.go b/services/blockvalidation/catchup_difficulty.go index 4eee4a346a..e6214f9d11 100644 --- a/services/blockvalidation/catchup_difficulty.go +++ b/services/blockvalidation/catchup_difficulty.go @@ -6,13 +6,13 @@ import ( "encoding/binary" "math/big" - "github.com/bsv-blockchain/go-wire" + "github.com/bsv-blockchain/go-bt/v2/chainhash" "github.com/bsv-blockchain/teranode/errors" "github.com/bsv-blockchain/teranode/model" "github.com/bsv-blockchain/teranode/services/blockchain" "github.com/bsv-blockchain/teranode/services/blockchain/work" "github.com/bsv-blockchain/teranode/settings" - "github.com/bsv-blockchain/teranode/util" + "github.com/bsv-blockchain/teranode/ulogger" ) // validateCatchupHeaderDifficulty verifies that the headers fetched during catchup carry @@ -21,9 +21,9 @@ import ( // // Header-fetch validation (validateBatchHeaders) only checks each header's proof-of-work // against the target encoded in its own nBits, so a peer could otherwise feed us a -// self-consistent low-difficulty chain. This step recomputes the DAA-required nBits from -// the fetched header chain itself and rejects mismatches before we spend resources -// fetching and fully validating the corresponding blocks. +// self-consistent low-difficulty chain. This step computes the required nBits for every +// fetched header and rejects mismatches before we spend resources fetching the +// corresponding blocks, including the subtree prewarm that fetchAndValidateBlocks starts. func (u *Server) validateCatchupHeaderDifficulty(ctx context.Context, catchupCtx *CatchupContext) error { u.logger.Debugf("[catchup][%s] Step 9.5: Validating header chain difficulty (DAA)", catchupCtx.blockUpTo.Hash().String()) @@ -35,7 +35,7 @@ func (u *Server) validateCatchupHeaderDifficulty(ctx context.Context, catchupCtx // The chain is anchored on the common ancestor, so this must run on the untrimmed header list, // before fetchAndValidateBlocks drops the stored front. - if err := validateHeaderChainDifficulty(u.settings, catchupCtx.commonAncestorMeta, catchupCtx.blockHeaders); err != nil { + if err := validateHeaderChainDifficulty(ctx, u.logger, u.settings, u.blockchainClient, catchupCtx.commonAncestorHash, catchupCtx.blockHeaders); err != nil { if errors.IsMaliciousResponseError(err) { if prometheusCatchupErrors != nil && catchupCtx.peerID != "" { prometheusCatchupErrors.WithLabelValues(catchupCtx.peerID, "invalid_difficulty").Inc() @@ -51,142 +51,94 @@ func (u *Server) validateCatchupHeaderDifficulty(ctx context.Context, catchupCtx return nil } -// validateHeaderChainDifficulty checks the DAA-required nBits for every header whose full -// difficulty-adjustment window lies within the in-memory header chain (anchored at the -// common ancestor). +// validateHeaderChainDifficulty checks the required nBits of every header in a fetched run. // // The headers are contiguous and ascending, with headers[0] a child of the common ancestor -// (anchor). Cumulative chainwork is reconstructed forward from the anchor's stored chainwork, -// mirroring stores/blockchain calculateAndPrepareChainWork; the expected target is then -// computed with the exact same arithmetic as the store-backed path via blockchain.ComputeTarget -// and the same median-of-three "suitable block" selection as GetSuitableBlock. -// -// Headers closer to the anchor than the window depth (whose window would reach blocks below -// the anchor, i.e. already-stored blocks on our own chain) are skipped here; the downstream -// full-block DAA check (BlockValidation.ValidateBlock via GetNextWorkRequired) covers those -// using the store. This step exists to reject deep, self-consistent low-difficulty chains -// early — exactly the region an attacker fully controls. -// Pre-DAA headers are also deferred to full-block validation and its checkpoint checks: -// this in-memory precheck only implements the modern DAA, not historical retargets/EDA. +// (anchorHash), which trunk holds. The expected nBits come from +// blockchain.Difficulty.CalcNextWorkRequiredFrom, the calculator full-block validation uses, +// reading ancestry through blockchain.NewBranchSource: the fetched headers answer for +// themselves and trunk answers for the anchor and everything below it. So a header whose +// adjustment window reaches below the anchor is judged against the stored blocks, and +// pre-DAA, testnet minimum-difficulty, STN and no-adjustment chains follow the same rules +// as full-block validation. Cumulative chain work for the fetched headers is reconstructed +// forward from the anchor's stored chain work, mirroring stores/blockchain +// calculateAndPrepareChainWork. // // Parameters: +// - ctx: context for the trunk reads +// - logger: logger for the difficulty calculator // - tSettings: chain settings (DAA rules, target spacing, pow limit) -// - anchor: metadata of the common ancestor (supplies Height and cumulative ChainWork) +// - trunk: source for the committed chain, the blockchain client in production +// - anchorHash: hash of the common ancestor // - headers: fetched headers after the common ancestor, ascending and contiguous // -// Returns a NetworkPeerMaliciousError if any checked header's nBits differ from the DAA -// requirement, or nil if all checked headers are correct (or none can be checked yet). -func validateHeaderChainDifficulty(tSettings *settings.Settings, anchor *model.BlockHeaderMeta, headers []*model.BlockHeader) error { - // Regtest and similar: difficulty is never adjusted, so any nBits is acceptable. - if tSettings.ChainCfgParams.NoDifficultyAdjustment { +// Returns a NetworkPeerMaliciousError if any header's nBits differ from the requirement, a +// ServiceError if the requirement cannot be computed, or nil if every header is correct. +// Failing closed matters: anything this lets through goes on to have its blocks fetched. +func validateHeaderChainDifficulty(ctx context.Context, logger ulogger.Logger, tSettings *settings.Settings, trunk blockchain.HeaderSource, + anchorHash *chainhash.Hash, headers []*model.BlockHeader) error { + if len(headers) == 0 { return nil } - if anchor == nil || len(headers) == 0 { - return nil + if anchorHash == nil { + return errors.NewProcessingError("[catchup:difficulty] no common ancestor to anchor the header chain on") } - window := blockchain.DifficultyAdjustmentWindow + anchor, anchorMeta, err := trunk.GetBlockHeader(ctx, anchorHash) + if err != nil || anchor == nil || anchorMeta == nil { + return errors.NewServiceError("[catchup:difficulty] failed to get common ancestor %s", anchorHash.String(), err) + } - // Cumulative chainwork per header, reconstructed from the anchor's chainwork. - // ChainWork is stored big-endian (see stores/blockchain/sql calculateAndPrepareChainWork), - // so SetBytes reconstructs the value directly. - cumWork := make([]*big.Int, len(headers)) - acc := new(big.Int).SetBytes(anchor.ChainWork) + branch := make(map[chainhash.Hash]*blockchain.BranchHeader, len(headers)) + acc := new(big.Int).SetBytes(anchorMeta.ChainWork) + prevHash := anchorHash for i, h := range headers { - bits := binary.LittleEndian.Uint32(h.Bits.CloneBytes()) - acc = new(big.Int).Add(acc, work.CalcBlockWork(bits)) - cumWork[i] = acc - } - - targetSpacing := int64(tSettings.ChainCfgParams.TargetTimePerBlock.Seconds()) + // Step 8 already verified this; the source walks HashPrevBlock, so a gap would make it + // read the wrong ancestry rather than fail. + if !h.HashPrevBlock.IsEqual(prevHash) { + return errors.NewProcessingError("[catchup:difficulty] header %d (%s) does not link to %s", i, h.Hash().String(), prevHash.String()) + } - // suitable builds a SuitableBlock view of the header at the given in-memory index. - suitable := func(idx int) *model.SuitableBlock { - h := headers[idx] + acc.Add(acc, work.CalcBlockWork(binary.LittleEndian.Uint32(h.Bits.CloneBytes()))) - return &model.SuitableBlock{ - NBits: h.Bits.CloneBytes(), - Time: h.Timestamp, - ChainWork: cumWork[idx].Bytes(), - Height: anchor.Height + 1 + uint32(idx), + branch[*h.Hash()] = &blockchain.BranchHeader{ + Header: h, + Height: anchorMeta.Height + 1 + uint32(i), //nolint:gosec // bounded by the fetched run + ChainWork: acc.FillBytes(make([]byte, chainhash.HashSize)), } + prevHash = h.Hash() } - // median3 returns the median-by-time of the three headers ending at idx, mirroring - // stores/blockchain/sql getMedianBlock. Order candidates oldest-first (depth DESC in - // the store) so the unstable sort's tie-break matches the store path exactly. - median3 := func(idx int) *model.SuitableBlock { - s := []*model.SuitableBlock{suitable(idx - 2), suitable(idx - 1), suitable(idx)} - util.SortForDifficultyAdjustment(s) + src := blockchain.NewBranchSource(trunk, func(hash chainhash.Hash) (*blockchain.BranchHeader, bool) { + bh, ok := branch[hash] + return bh, ok + }) - return s[1] + // A fresh calculator per run: its last-hash cache must never outlive the branch it read. + difficulty, err := blockchain.NewDifficulty(nil, logger, tSettings) + if err != nil { + return errors.NewProcessingError("[catchup:difficulty] failed to create difficulty calculator", err) } - powLimitBits := powLimitNBit(tSettings) - - for i := 1; i < len(headers); i++ { - // The DAA target for header i is derived from its parent (index i-1): the last - // suitable block ends at the parent, and the first suitable block ends at the - // parent's ancestor `window` blocks back. That ancestor's own median lookback of - // two more blocks must also be in memory, so the first fully-checkable header is - // the one whose (parentIdx - window) >= 2. - parentIdx := i - 1 - firstIdx := parentIdx - window - - if firstIdx < 2 { - continue - } - - parentHeight := anchor.Height + 1 + uint32(parentIdx) - if parentHeight < tSettings.ChainCfgParams.DaaForkHeight && tSettings.ChainCfgParams.Net != wire.STN { - continue - } - - var expected *model.NBit - - switch { - case tSettings.ChainCfgParams.ReduceMinDifficulty && - int64(headers[i].Timestamp) > int64(headers[parentIdx].Timestamp)+2*targetSpacing: - // Testnet minimum-difficulty rule: a block more than 2*spacing after its - // parent may be mined at the pow limit. - expected = powLimitBits - - case parentHeight < uint32(window)+4: - // Not enough history for a difficulty adjustment yet. - expected = powLimitBits - - default: - lastSuitable := median3(parentIdx) - firstSuitable := median3(firstIdx) - - nBits, err := blockchain.ComputeTarget(tSettings, firstSuitable, lastSuitable) - if err != nil { - return errors.NewProcessingError("[catchup:difficulty] failed to compute expected nBits at height %d", parentHeight+1, err) - } + parent, parentHeight := anchor, anchorMeta.Height - expected = nBits + for _, h := range headers { + expected, err := difficulty.CalcNextWorkRequiredFrom(ctx, src, parent, parentHeight, int64(h.Timestamp)) + if err != nil || expected == nil { + return errors.NewServiceError("[catchup:difficulty] failed to compute expected nBits at height %d", parentHeight+1, err) } - if headers[i].Bits != *expected { + if h.Bits != *expected { return errors.NewNetworkPeerMaliciousError( - "[catchup:difficulty] header at height %d has incorrect difficulty bits: got %s, expected %s", - parentHeight+1, headers[i].Bits.String(), expected.String(), + "[catchup:difficulty] header %s at height %d has incorrect difficulty bits: got %s, expected %s", + h.Hash().String(), parentHeight+1, h.Bits.String(), expected.String(), ) } + + parent, parentHeight = h, parentHeight+1 } return nil } - -// powLimitNBit returns the minimum-difficulty (pow limit) nBits for the chain, matching -// how blockchain.NewDifficulty derives its powLimitnBits. -func powLimitNBit(tSettings *settings.Settings) *model.NBit { - b := make([]byte, 4) - binary.LittleEndian.PutUint32(b, tSettings.ChainCfgParams.PowLimitBits) - - nb, _ := model.NewNBitFromSlice(b) - - return nb -} diff --git a/services/blockvalidation/catchup_difficulty_test.go b/services/blockvalidation/catchup_difficulty_test.go index 00615c7949..bbbdb9f2be 100644 --- a/services/blockvalidation/catchup_difficulty_test.go +++ b/services/blockvalidation/catchup_difficulty_test.go @@ -1,8 +1,8 @@ package blockvalidation import ( - "encoding/hex" - "math/big" + "context" + "net/url" "os" "testing" @@ -12,6 +12,8 @@ import ( "github.com/bsv-blockchain/teranode/model" "github.com/bsv-blockchain/teranode/services/blockchain" "github.com/bsv-blockchain/teranode/settings" + blockchain_store "github.com/bsv-blockchain/teranode/stores/blockchain" + "github.com/bsv-blockchain/teranode/ulogger" "github.com/bsv-blockchain/teranode/util/test" "github.com/stretchr/testify/require" ) @@ -29,273 +31,216 @@ func daaSettings(t *testing.T, base chaincfg.Params) *settings.Settings { return tSettings } -func mustDecodeHex(t *testing.T, s string) []byte { +// newDifficultyStore returns an empty sqlitememory blockchain store for tSettings' chain; the +// store inserts the genesis block itself. +func newDifficultyStore(t *testing.T, tSettings *settings.Settings) blockchain_store.Store { t.Helper() - b, err := hex.DecodeString(s) + store, err := blockchain_store.NewStore(ulogger.TestLogger{}, &url.URL{Scheme: "sqlitememory"}, tSettings) require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, store.Close(context.Background())) }) - return b + return store } -// buildHeader builds a minimal header carrying only the fields the DAA check reads -// (timestamp and difficulty bits). Linkage/PoW are validated elsewhere. -func buildHeader(ts uint32, bits *model.NBit) *model.BlockHeader { - return &model.BlockHeader{ - Version: 1, - HashPrevBlock: &chainhash.Hash{}, - HashMerkleRoot: &chainhash.Hash{}, - Timestamp: ts, - Bits: *bits, - Nonce: 0, - } -} - -// buildConstantChain builds n contiguous headers spaced exactly spacing seconds apart, -// all carrying bits. Anchored at a common ancestor at height H with arbitrary chainwork -// (only chainwork differences within the window matter to the DAA). -func buildConstantChain(n int, spacing uint32, bits *model.NBit) (*model.BlockHeaderMeta, []*model.BlockHeader) { - anchor := &model.BlockHeaderMeta{ - Height: 200000, - ChainWork: new(big.Int).Lsh(big.NewInt(1), 200).Bytes(), - } - - baseTime := uint32(1_600_000_000) - headers := make([]*model.BlockHeader, n) +// storeDifficultyHeaders stores headers in order as main-chain blocks; the store takes each height +// from the parent. Only the header fields matter to the difficulty rules, so every block reuses the +// genesis coinbase, changed so the store does not take it for a second genesis. +func storeDifficultyHeaders(t *testing.T, tSettings *settings.Settings, store blockchain_store.Store, headers []*model.BlockHeader) { + t.Helper() - for i := range n { - headers[i] = buildHeader(baseTime+uint32(i+1)*spacing, bits) - } + genesis, err := model.NewBlockFromMsgBlock(tSettings.ChainCfgParams.GenesisBlock, nil) + require.NoError(t, err) - return anchor, headers -} + genesis.CoinbaseTx.LockTime = 1 -func TestValidateHeaderChainDifficulty_HistoricalTestnet(t *testing.T) { - tSettings := test.CreateBaseTestSettings(t) - params := chaincfg.TestNetParams - tSettings.ChainCfgParams = ¶ms - data, err := os.ReadFile("../blockchain/testdata/testnet_headers_0_547.bin") - require.NoError(t, err) - require.Len(t, data, 548*80) - headers := make([]*model.BlockHeader, 547) - for i := range headers { - headers[i], err = model.NewBlockHeaderFromBytes(data[(i+1)*80 : (i+2)*80]) + for _, h := range headers { + _, _, err = store.StoreBlock(t.Context(), &model.Block{Header: h, CoinbaseTx: genesis.CoinbaseTx, TransactionCount: 1}, "test") require.NoError(t, err) } - require.NoError(t, validateHeaderChainDifficulty(tSettings, &model.BlockHeaderMeta{Height: 0}, headers)) } -func TestValidateHeaderChainDifficulty_STNPreservesExistingRules(t *testing.T) { - s := test.CreateBaseTestSettings(t) - params := chaincfg.StnParams - s.ChainCfgParams = ¶ms - bits, err := model.NewNBitFromString("180a097a") - require.NoError(t, err) - anchor, headers := buildConstantChain(300, 600, bits) - anchor.Height = 1000 // STN used DAA here before this historical testnet fix. - require.NoError(t, validateHeaderChainDifficulty(s, anchor, headers)) - headers[200].Bits = *powLimitNBit(s) - require.True(t, errors.IsMaliciousResponseError(validateHeaderChainDifficulty(s, anchor, headers))) -} +// honestDifficultyChain builds n headers on top of genesis, each carrying the bits the +// store-backed calculator demands for it, and returns them indexed by height (genesis at 0). +// +// The calculator reads ancestry through the store's SQL, not through the branch source the code +// under test uses, so it is an independent oracle for what every header must carry. timeAt gives +// the timestamp at each height. +func honestDifficultyChain(t *testing.T, tSettings *settings.Settings, n int, timeAt func(height uint32, prev []*model.BlockHeader) uint32) []*model.BlockHeader { + t.Helper() -func TestValidateHeaderChainDifficulty_DAAActivation(t *testing.T) { - tSettings := daaSettings(t, chaincfg.MainNetParams) - bits, err := model.NewNBitFromString("180a097a") - require.NoError(t, err) - for _, count := range []int{200, 201} { - anchor, headers := buildConstantChain(count, 600, bits) - tSettings.ChainCfgParams.DaaForkHeight = anchor.Height + 200 - headers[count-1].Bits = *powLimitNBit(tSettings) - err := validateHeaderChainDifficulty(tSettings, anchor, headers) - if count == 200 { - require.NoError(t, err, "historical targets are deferred to full-block validation") - } else { - require.True(t, errors.IsMaliciousResponseError(err), "modern DAA must run at its parent-height activation") - } - } -} + store := newDifficultyStore(t, tSettings) -// TestValidateHeaderChainDifficulty_ValidConstantChain proves the DAA check accepts a -// steady-state chain: constant difficulty with blocks mined exactly on the target spacing -// must reproduce the same target, so no header is rejected. -func TestValidateHeaderChainDifficulty_ValidConstantChain(t *testing.T) { - tSettings := daaSettings(t, chaincfg.MainNetParams) + d, err := blockchain.NewDifficulty(store, ulogger.TestLogger{}, tSettings) + require.NoError(t, err) - startBits, err := model.NewNBitFromString("180a097a") + genesis, err := model.NewBlockFromMsgBlock(tSettings.ChainCfgParams.GenesisBlock, nil) require.NoError(t, err) - anchor, headers := buildConstantChain(300, 600, startBits) + headers := []*model.BlockHeader{genesis.Header} - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) -} + for height := uint32(1); height <= uint32(n); height++ { //nolint:gosec // a fixture size + parent := headers[height-1] + ts := timeAt(height, headers) -// TestValidateHeaderChainDifficulty_RejectsEasyBits feeds an otherwise-valid chain a single -// header (deep enough that its full window is in memory) with artificially easy nBits, and -// asserts it is rejected as malicious — the core protection this change adds. -func TestValidateHeaderChainDifficulty_RejectsEasyBits(t *testing.T) { - tSettings := daaSettings(t, chaincfg.MainNetParams) + bits, err := d.CalcNextWorkRequired(t.Context(), parent, height-1, int64(ts)) + require.NoError(t, err) - startBits, err := model.NewNBitFromString("180a097a") - require.NoError(t, err) + h := &model.BlockHeader{ + Version: 1, + HashPrevBlock: parent.Hash(), + HashMerkleRoot: &chainhash.Hash{}, + Timestamp: ts, + Bits: *bits, + Nonce: height, + } - anchor, headers := buildConstantChain(300, 600, startBits) + storeDifficultyHeaders(t, tSettings, store, []*model.BlockHeader{h}) - // Baseline must be valid so the failure below is attributable to the tampering. - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) + headers = append(headers, h) + } - easyBits, err := model.NewNBitFromString("207fffff") - require.NoError(t, err) + return headers +} - // Index 200 is far past the 144-block window, so its DAA target is computed entirely - // from earlier (untampered) in-memory headers. - headers[200].Bits = *easyBits +// irregularTime spaces blocks unevenly, on average faster than the target so the DAA moves off the +// pow limit, and every eleventh block repeats the timestamp of the block two back while the one +// between is later. That is the equal-time median-of-three case whose tie-break must match the +// store's ordering. +func irregularTime(height uint32, prev []*model.BlockHeader) uint32 { + if height >= 2 && height%11 == 0 { + return prev[height-2].Timestamp + } - err = validateHeaderChainDifficulty(tSettings, anchor, headers) - require.Error(t, err) - require.True(t, errors.IsMaliciousResponseError(err), "expected malicious response error, got %v", err) + return prev[height-1].Timestamp + 300 + (height*137)%400 } -// TestValidateHeaderChainDifficulty_RegtestSkips confirms that on a no-adjustment network -// (regtest) any difficulty is accepted, matching CalcNextWorkRequired's behaviour. -func TestValidateHeaderChainDifficulty_RegtestSkips(t *testing.T) { - tSettings := test.CreateBaseTestSettings(t) // RegressionNetParams: NoDifficultyAdjustment - require.True(t, tSettings.ChainCfgParams.NoDifficultyAdjustment) +// withTamperedBits returns a copy of h declaring bits instead of its own. +func withTamperedBits(t *testing.T, h *model.BlockHeader, bits string) *model.BlockHeader { + t.Helper() - easyBits, err := model.NewNBitFromString("207fffff") + nBits, err := model.NewNBitFromString(bits) require.NoError(t, err) + require.NotEqual(t, h.Bits, *nBits, "fixture precondition: the tampered bits must differ") - anchor, headers := buildConstantChain(300, 600, easyBits) + tampered := *h + tampered.Bits = *nBits - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) + return &tampered } -// TestValidateHeaderChainDifficulty_ShortChainSkips confirms that when the chain is shorter -// than a full adjustment window, no header is checked here (the downstream full-block check -// covers those), so even wrong bits are not rejected at this stage. -func TestValidateHeaderChainDifficulty_ShortChainSkips(t *testing.T) { +// daaFixture is an honest chain of 170 headers on mainnet rules with the modern DAA from genesis, +// and a node that has stored the first 160 of them. +func daaFixture(t *testing.T) (*settings.Settings, blockchain_store.Store, []*model.BlockHeader) { + t.Helper() + tSettings := daaSettings(t, chaincfg.MainNetParams) + chain := honestDifficultyChain(t, tSettings, 170, irregularTime) - easyBits, err := model.NewNBitFromString("207fffff") - require.NoError(t, err) + powLimit := chain[0].Bits + require.NotEqual(t, powLimit, chain[170].Bits, "fixture precondition: the DAA must have moved off the pow limit") - // 100 < window(144) + median lookback, so nothing is checkable in memory. - anchor, headers := buildConstantChain(100, 600, easyBits) + trunk := newDifficultyStore(t, tSettings) + storeDifficultyHeaders(t, tSettings, trunk, chain[1:161]) - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) + return tSettings, trunk, chain } -// TestValidateHeaderChainDifficulty_TestnetMinDifficulty confirms the testnet -// minimum-difficulty rule is honoured: a block mined more than 2*spacing after its parent -// is expected to carry the pow-limit target, and is accepted when it does. -func TestValidateHeaderChainDifficulty_TestnetMinDifficulty(t *testing.T) { - base := chaincfg.MainNetParams - base.ReduceMinDifficulty = true +func validateFixtureRun(t *testing.T, tSettings *settings.Settings, trunk blockchain.HeaderSource, anchor *model.BlockHeader, headers []*model.BlockHeader) error { + t.Helper() - tSettings := daaSettings(t, base) + return validateHeaderChainDifficulty(t.Context(), ulogger.TestLogger{}, tSettings, trunk, anchor.Hash(), headers) +} - startBits, err := model.NewNBitFromString("180a097a") - require.NoError(t, err) +// TestValidateHeaderChainDifficulty_HonestRunAcrossTheStoredWindowPasses checks a run whose +// adjustment windows lie almost entirely in the store: every header gets the bits the store-backed +// calculator gave it. +func TestValidateHeaderChainDifficulty_HonestRunAcrossTheStoredWindowPasses(t *testing.T) { + tSettings, trunk, chain := daaFixture(t) - // Make the delayed block the last header so nothing downstream depends on its - // irregular timestamp; every earlier header is a steady-state block. - anchor, headers := buildConstantChain(201, 600, startBits) - last := len(headers) - 1 + require.NoError(t, validateFixtureRun(t, tSettings, trunk, chain[160], chain[161:])) +} - powLimit := powLimitNBit(tSettings) +// TestValidateHeaderChainDifficulty_HeaderExtendingTheTipWithEasyBitsIsRejected is the attack +// #2084 closes: a single header on top of our tip, mined against a target the peer +// chose. Its whole adjustment window is in the store, which the check used to skip. +func TestValidateHeaderChainDifficulty_HeaderExtendingTheTipWithEasyBitsIsRejected(t *testing.T) { + tSettings, trunk, chain := daaFixture(t) - // The last block arrives more than 2*spacing after its parent, so the min-difficulty - // rule requires it to carry the pow-limit target. - headers[last].Timestamp = headers[last-1].Timestamp + 3*600 - headers[last].Bits = *powLimit + err := validateFixtureRun(t, tSettings, trunk, chain[160], []*model.BlockHeader{withTamperedBits(t, chain[161], "207fffff")}) + require.Error(t, err) + require.True(t, errors.IsMaliciousResponseError(err), "expected malicious response error, got %v", err) + require.ErrorContains(t, err, "expected "+chain[161].Bits.String(), "the expected bits must be the store-backed calculator's") +} + +// TestValidateHeaderChainDifficulty_ShortForkWithEasyBitsIsRejected offers a three-header fork +// rooted three blocks below the tip whose last header declares the pow limit. +func TestValidateHeaderChainDifficulty_ShortForkWithEasyBitsIsRejected(t *testing.T) { + tSettings, trunk, chain := daaFixture(t) - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) + fork := []*model.BlockHeader{chain[158], chain[159], withTamperedBits(t, chain[160], chain[0].Bits.String())} - // If that same delayed block does NOT carry the pow-limit target, reject it. - headers[last].Bits = *startBits - err = validateHeaderChainDifficulty(tSettings, anchor, headers) + err := validateFixtureRun(t, tSettings, trunk, chain[157], fork) require.Error(t, err) require.True(t, errors.IsMaliciousResponseError(err), "expected malicious response error, got %v", err) } -// TestValidateHeaderChainDifficulty_EqualTimestampMedianTie exercises the specific -// case fixed in ChiR1: two blocks two apart at a window boundary sharing a timestamp -// (with the middle one higher), which triggers the unstable sort's tie-break. With the -// fixed oldest-first ordering, median3 selects the same block as the store would, -// ensuring DAA parity. -func TestValidateHeaderChainDifficulty_EqualTimestampMedianTie(t *testing.T) { - tSettings := daaSettings(t, chaincfg.MainNetParams) +// TestValidateHeaderChainDifficulty_UnjudgeableRunFailsClosed checks that a run the calculator +// cannot judge is refused without blaming the peer: an anchor the store does not hold, and a run +// that does not link to its anchor. +func TestValidateHeaderChainDifficulty_UnjudgeableRunFailsClosed(t *testing.T) { + tSettings, trunk, chain := daaFixture(t) - // Build blocks for indices 0-149 (the first block where DAA check runs is i=147). - // We construct timestamps where blocks 5 and 7 (examined by median3(7)) share a - // timestamp, to test the tie-break ordering in a checkable region. - // Height of block at index i in catchup is anchor.Height + 1 + i. - anchor := &model.BlockHeaderMeta{ - Height: 200000, - ChainWork: new(big.Int).Lsh(big.NewInt(1), 200).Bytes(), - } + err := validateFixtureRun(t, tSettings, trunk, chain[165], chain[166:]) + require.Error(t, err, "an anchor the store does not hold") + require.False(t, errors.IsMaliciousResponseError(err)) + + err = validateFixtureRun(t, tSettings, trunk, chain[160], chain[162:]) + require.Error(t, err, "a run that does not link to its anchor") + require.False(t, errors.IsMaliciousResponseError(err)) +} + +// TestValidateHeaderChainDifficulty_HistoricalTestnet runs real testnet headers from genesis, across +// the pre-DAA rules and testnet's minimum-difficulty rule, against a store holding only genesis. +func TestValidateHeaderChainDifficulty_HistoricalTestnet(t *testing.T) { + tSettings := test.CreateBaseTestSettings(t) + params := chaincfg.TestNetParams + tSettings.ChainCfgParams = ¶ms - startBits, err := model.NewNBitFromString("180a097a") + data, err := os.ReadFile("../blockchain/testdata/testnet_headers_0_547.bin") require.NoError(t, err) + require.Len(t, data, 548*80) - baseTime := uint32(1_600_000_000) - headers := make([]*model.BlockHeader, 150) - - // Build blocks 0-149 with timestamps that create the tie at 5, 6, 7. - // Blocks 5 and 7 share a timestamp, block 6 is higher (triggering sort's tie-break). - for i := range 150 { - var ts uint32 - if i < 5 { - ts = baseTime + uint32((i+1)*600) - } else if i == 5 { - ts = baseTime + 1 + uint32((5+1)*600) // time T - } else if i == 6 { - ts = baseTime + 1 + uint32((5+1)*600) + 300 // time T+300 - } else if i == 7 { - ts = baseTime + 1 + uint32((5+1)*600) // time T again - } else { - ts = baseTime + uint32((i+1)*600) - } - headers[i] = buildHeader(ts, startBits) + headers := make([]*model.BlockHeader, 548) + for i := range headers { + headers[i], err = model.NewBlockHeaderFromBytes(data[i*80 : (i+1)*80]) + require.NoError(t, err) } - // With the fixed oldest-first ordering, median3(7) examines blocks [5,6,7] ordered - // as [5,6,7] (oldest-first). The unstable sort will preserve this order since it - // only compares by time and has two equal values; it selects s[1]=block 6 as median. - // With the buggy newest-first ordering [7,6,5], the tie-break might reorder them - // differently, potentially selecting a different median. This test validates that - // the fix produces consistent results. - // - // We use constant bits throughout since all blocks are deep before the full 144-block - // DAA window is needed (DAA checks start at i=147), so the checkable headers in this - // shortened chain don't trigger full DAA recalculation. - require.NoError(t, validateHeaderChainDifficulty(tSettings, anchor, headers)) + trunk := newDifficultyStore(t, tSettings) + + require.NoError(t, validateFixtureRun(t, tSettings, trunk, headers[0], headers[1:])) + + headers[547] = withTamperedBits(t, headers[547], "207fffff") + err = validateFixtureRun(t, tSettings, trunk, headers[0], headers[1:]) + require.True(t, errors.IsMaliciousResponseError(err), "expected malicious response error, got %v", err) } -// TestComputeTarget_MatchesDifficultyMethod is a guard on the ComputeTarget extraction: -// the exported free function must return exactly what the store-backed path produced. -func TestComputeTarget_MatchesDifficultyMethod(t *testing.T) { - tSettings := daaSettings(t, chaincfg.MainNetParams) +// TestValidateHeaderChainDifficulty_NoAdjustmentChain checks a chain that never adjusts difficulty +// (regtest) the way full-block validation does: each header must repeat its parent's bits. +func TestValidateHeaderChainDifficulty_NoAdjustmentChain(t *testing.T) { + tSettings := test.CreateBaseTestSettings(t) + require.True(t, tSettings.ChainCfgParams.NoDifficultyAdjustment) - firstBits, _ := model.NewNBitFromString("1808de5f") - first := &model.SuitableBlock{ - NBits: firstBits.CloneBytes(), - Time: 1704647599, - ChainWork: mustDecodeHex(t, "0000000000000000000000000000000000000000014fde9a5605193885731ee4"), - } + chain := honestDifficultyChain(t, tSettings, 5, func(height uint32, prev []*model.BlockHeader) uint32 { + return prev[height-1].Timestamp + 600 + }) - lastBits, _ := model.NewNBitFromString("180a097a") - last := &model.SuitableBlock{ - NBits: lastBits.CloneBytes(), - Time: 1704738562, - ChainWork: mustDecodeHex(t, "0000000000000000000000000000000000000000014fed8ff37cff135c70f4bb"), - } + trunk := newDifficultyStore(t, tSettings) + storeDifficultyHeaders(t, tSettings, trunk, chain[1:3]) - got, err := blockchain.ComputeTarget(tSettings, first, last) - require.NoError(t, err) + require.NoError(t, validateFixtureRun(t, tSettings, trunk, chain[2], chain[3:])) - // Same inputs and expectation as blockchain.TestCalcNextRequiredDifficulty, proving the - // extracted free function matches the store-backed path exactly. - expected, err := model.NewNBitFromString("180a2268") - require.NoError(t, err) - require.Equal(t, expected.String(), got.String()) + err := validateFixtureRun(t, tSettings, trunk, chain[2], []*model.BlockHeader{withTamperedBits(t, chain[3], "1d00ffff")}) + require.True(t, errors.IsMaliciousResponseError(err), "expected malicious response error, got %v", err) } diff --git a/services/blockvalidation/catchup_malicious_peer_test.go b/services/blockvalidation/catchup_malicious_peer_test.go index 654aa3c035..687bd85956 100644 --- a/services/blockvalidation/catchup_malicious_peer_test.go +++ b/services/blockvalidation/catchup_malicious_peer_test.go @@ -216,10 +216,13 @@ func TestCatchup_SybilAttack(t *testing.T) { // Mock UTXO store block height mockUTXOStore.On("GetBlockHeight").Return(uint32(1000)) - // The common ancestor is a real mainnet header. That is all the mainnet fixtures are needed for - // here: it is already in our chain, so it is never re-validated. - mainnetHeaders := testhelpers.GetMainnetHeadersRange(t, 0, 21) - bestBlockHeader := mainnetHeaders[0] + // The common ancestor is the real regtest genesis header, followed by a synthetic run on the + // same 207fffff target. It is already in our chain, so it is never re-validated, but catch-up's + // header precheck does read its bits: the fixture chain never adjusts difficulty, so the honest + // chain must repeat them, which a mainnet ancestor's 1d00ffff would rule out. + anchorGenesis := regtestGenesisHeader(t) + anchorChain := append([]*model.BlockHeader{anchorGenesis}, testhelpers.CreateSyntheticChainFromAtHeight(anchorGenesis, 20, 1)...) + bestBlockHeader := anchorChain[0] // The honest chain is SYNTHETIC rather than a replay of mainnet headers. The honest peer serves // coinbase-only bodies for these headers, and a body that declares no subtrees must carry the @@ -236,7 +239,7 @@ func TestCatchup_SybilAttack(t *testing.T) { // Create fake headers that branch from the common ancestor // These are synthetic headers with valid PoW that will fail other validation // CreateAdversarialFork returns headers including the fork point at index 0 - adversarialFork, err := testhelpers.CreateAdversarialFork(mainnetHeaders, 0, 21) + adversarialFork, err := testhelpers.CreateAdversarialFork(anchorChain, 0, 21) require.NoError(t, err, "Failed to create adversarial fork") // Remove the common ancestor from adversarial fork since it's already at index 0 @@ -257,7 +260,7 @@ func TestCatchup_SybilAttack(t *testing.T) { mockBlockchainClient.On("GetBlockHeader", mock.Anything, targetHash). Return(targetBlock.Header, &model.BlockHeaderMeta{Height: targetBlock.Height}, nil).Maybe() - // bestBlockHeader is already set from mainnetHeaders[0] above + // bestBlockHeader is already set from anchorChain[0] above mockBlockchainClient.On("GetBestBlockHeader", mock.Anything). Return(bestBlockHeader, &model.BlockHeaderMeta{Height: 1000}, nil) @@ -268,7 +271,7 @@ func TestCatchup_SybilAttack(t *testing.T) { // CheckHeaderContextual requires GetBlockHeaders(parent) to actually return the // parent's chain (issue 1467). Registered before the catch-all below so testify // matches these first. - registerParentChainHeaders(mockBlockchainClient, mainnetHeaders, 1000) + registerParentChainHeaders(mockBlockchainClient, anchorChain, 1000) // Mock GetBlockHeaders for common ancestor finding mockBlockchainClient.On("GetBlockHeaders", mock.Anything, mock.Anything, mock.Anything). @@ -388,7 +391,7 @@ func TestCatchup_SybilAttack(t *testing.T) { ) } - // 1 honest peer - returns the mainnet chain including common ancestor + // 1 honest peer - returns the honest chain including common ancestor // The peer returns from common ancestor to target honestChainWithAncestor := append([]*model.BlockHeader{bestBlockHeader}, honestHeaders...) honestHeaderBytes := testhelpers.HeadersToBytes(honestChainWithAncestor) diff --git a/services/blockvalidation/catchup_test.go b/services/blockvalidation/catchup_test.go index 8c7c02047b..4778e7513c 100644 --- a/services/blockvalidation/catchup_test.go +++ b/services/blockvalidation/catchup_test.go @@ -3329,6 +3329,9 @@ func setupTestCatchupServer(t *testing.T) (*Server, *blockchain.Mock, *utxo.Mock BIP0034Height: 100000000, BIP0065Height: 100000000, BIP0066Height: 100000000, + // The stubbed client has no ancestry for the catch-up difficulty precheck to read, so the + // synthetic chains never adjust: each header must repeat its parent's bits. + NoDifficultyAdjustment: true, } mockBlockchainClient := &blockchain.Mock{} @@ -3444,6 +3447,9 @@ func setupTestCatchupServerWithConfig(t *testing.T, config *testhelpers.TestServ BIP0034Height: 100000000, BIP0065Height: 100000000, BIP0066Height: 100000000, + // The stubbed client has no ancestry for the catch-up difficulty precheck to read, so the + // synthetic chains never adjust: each header must repeat its parent's bits. + NoDifficultyAdjustment: true, } mockBlockchainClient := &blockchain.Mock{} diff --git a/services/blockvalidation/checkpoint_prefix_nbits_test.go b/services/blockvalidation/checkpoint_prefix_nbits_test.go index bf78d9d1ac..e1654b0bdc 100644 --- a/services/blockvalidation/checkpoint_prefix_nbits_test.go +++ b/services/blockvalidation/checkpoint_prefix_nbits_test.go @@ -12,6 +12,7 @@ import ( "github.com/bsv-blockchain/teranode/services/blockchain" "github.com/bsv-blockchain/teranode/settings" blockchainoptions "github.com/bsv-blockchain/teranode/stores/blockchain/options" + "github.com/bsv-blockchain/teranode/ulogger" "github.com/bsv-blockchain/teranode/util/test" "github.com/stretchr/testify/require" ) @@ -112,14 +113,15 @@ func difficulty1ChildOfHonestParent(ctx context.Context, t *testing.T, client bl // that were supposed to cover it does: // // - the direct peer path has no header-level difficulty pipeline at all; -// - catch-up has one, but validateHeaderChainDifficulty DEFERS every header whose full 144-block -// window is not inside the fetched run — the first header and roughly its first 146 successors — -// naming this very check as the downstream cover for them. Narrowing the exemption to catch-up -// therefore composed the two skips into a hole rather than closing it. +// - catch-up has one, but validateHeaderChainDifficulty used to DEFER every header whose full +// 144-block window was not inside the fetched run — the first header and roughly its first 146 +// successors — naming this very check as the downstream cover for them. Narrowing the exemption +// to catch-up therefore composed the two skips into a hole rather than closing it. // -// Both subtests below assert the same thing from the two delivery shapes, and the catch-up one -// asserts the composition directly: the real precheck is run over the fetched header run and shown -// to PASS the bad chain, after which the body validator must be the thing that rejects it. +// Both subtests below assert the same thing from the two delivery shapes. The catch-up precheck now +// judges every header (#2084) and rejects this chain itself; the catch-up subtest +// asserts that, and that the body validator still rejects it too, so neither check leans on the +// other. func TestValidateBlock_BelowCheckpoint_ExpectedNBitsAlwaysEnforced(t *testing.T) { initPrometheusMetrics() @@ -143,7 +145,7 @@ func TestValidateBlock_BelowCheckpoint_ExpectedNBitsAlwaysEnforced(t *testing.T) requireNothingPersisted(ctx, t, client, child.Hash()) }) - t.Run("catch-up delivery inside the precheck's skipped window is rejected too", func(t *testing.T) { + t.Run("catch-up delivery near the anchor is rejected by both the precheck and the body check", func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -153,19 +155,14 @@ func TestValidateBlock_BelowCheckpoint_ExpectedNBitsAlwaysEnforced(t *testing.T) parent := storeHonestPrefixParent(ctx, t, client, tSettings) child := difficulty1ChildOfHonestParent(ctx, t, client, parent) - _, anchorMeta, err := client.GetBlockHeader(ctx, tSettings.ChainCfgParams.GenesisHash) - require.NoError(t, err) + // This fetched run is two headers long, so every header in it is closer to the anchor than + // the 144-block window depth. The precheck reads that window from the store and must still + // reject the second header for carrying the wrong difficulty bits. + err := validateHeaderChainDifficulty(ctx, ulogger.TestLogger{}, tSettings, client, tSettings.ChainCfgParams.GenesisHash, + []*model.BlockHeader{parent.Header, child.Header}) + require.True(t, errors.IsMaliciousResponseError(err), "the header precheck must cover near-anchor headers, got %v", err) - // The composition, asserted against the REAL precheck rather than described. This fetched - // run is two headers long, so every header in it is closer to the anchor than the 144-block - // window depth and validateHeaderChainDifficulty skips all of them — it returns nil for a - // chain whose second header carries the wrong difficulty bits. Catch-up's own doc comment - // names the body-level check as the cover for exactly these headers. - require.NoError(t, - validateHeaderChainDifficulty(tSettings, anchorMeta, []*model.BlockHeader{parent.Header, child.Header}), - "fixture precondition: the header precheck does NOT cover this block, which is why the body check must") - - // So the body validator has to be what rejects it, even on the catch-up path. + // And the body validator rejects it independently, even on the catch-up path. err = bv.ValidateBlockWithOptions(ctx, child, "http://localhost", &ValidateBlockOptions{ CachedHeaders: []*model.BlockHeader{parent.Header}, IsCatchupMode: true, @@ -174,7 +171,7 @@ func TestValidateBlock_BelowCheckpoint_ExpectedNBitsAlwaysEnforced(t *testing.T) require.Error(t, err) require.True(t, errors.Is(err, errors.ErrBlockInvalid)) require.ErrorContains(t, err, "incorrect difficulty bits", - "catch-up must not be exempt: its header precheck defers near-anchor headers to this check") + "catch-up must not be exempt from the body check either") requireNothingPersisted(ctx, t, client, child.Hash()) }) diff --git a/services/legacy/netsync/header_cache.go b/services/legacy/netsync/header_cache.go index 9ac1f241d0..bc4dce6dfc 100644 --- a/services/legacy/netsync/header_cache.go +++ b/services/legacy/netsync/header_cache.go @@ -158,7 +158,7 @@ type headerCache struct { // The header is kept as its six wire fields, not as a wire.BlockHeader, whose // time.Time timestamp costs 24 bytes against the wire's 4 and pads the struct. // All six are kept, merkle root and nonce included, although no rule reads -// those two: the branchSource hands the difficulty calculator and the median +// those two: the branch source hands the difficulty calculator and the median // time past walk model headers, and both check each header's Hash() against // the hash they asked for (services/blockchain/Difficulty.go and // medianTimePast), which needs every byte of the 80. Fields are ordered so @@ -711,28 +711,25 @@ func (c *headerCache) judge(ctx context.Context, plan fillPlan, headers []*wire. nodes := make([]*headerNode, 0, plan.cut-plan.start) var ( - source *branchSource + source blockchain.HeaderSource pending map[chainhash.Hash]*headerNode ) if rules != nil { pending = make(map[chainhash.Hash]*headerNode, plan.cut-plan.start) - source = &branchSource{ - trunk: rules.trunk, - lookup: func(hash chainhash.Hash) (*cachedHeader, bool) { - node, ok := pending[hash] - if !ok { - // Read without mu: see judge's doc. - node, ok = c.index[hash] - } + source = blockchain.NewBranchSource(rules.trunk, func(hash chainhash.Hash) (*blockchain.BranchHeader, bool) { + node, ok := pending[hash] + if !ok { + // Read without mu: see judge's doc. + node, ok = c.index[hash] + } - if !ok { - return nil, false - } + if !ok { + return nil, false + } - return node.cached(), true - }, - } + return node.cached(), true + }) } prev := plan.anchor @@ -869,9 +866,9 @@ func trunkFork(ctx context.Context, rules *headerRules, checkpoints []chaincfg.C return fillResult{} } -// cached is the node in the form branchSource reads. -func (n *headerNode) cached() *cachedHeader { - return &cachedHeader{header: n.modelHeader(), height: n.height, chainWork: n.chainWork} +// cached is the node in the form blockchain.NewBranchSource reads. +func (n *headerNode) cached() *blockchain.BranchHeader { + return &blockchain.BranchHeader{Header: n.modelHeader(), Height: uint32(n.height), ChainWork: append([]byte(nil), n.chainWork[:]...)} //nolint:gosec // a cached height is above the tip, never negative } // installLocked holds the judged nodes and moves owner's branch to the batch's