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