perf(validator): skip BDK script validation for trusted legacy IBD blocks#951
Merged
freemans13 merged 1 commit intoMay 27, 2026
Conversation
…BD below checkpoint PreValidateTransactions is only reached via the quickValidationMode path, which only runs when the block height is at or below the highest hard-coded checkpoint for the active network. PoW plus checkpoint linkage establish the chain as canonical, so re-running BDK transaction validation (script execution, sigops, standardness, consensus checks) is pure overhead. Profiling mainnet IBD showed BDK.ValidateTransaction (the cgo call into gobdk) accounting for ~22% of teranode's CPU during legacy catchup, even though the surrounding code path already trusts the chain via the checkpoint mechanism. Adds an Options.SkipScriptValidation flag plus WithSkipScriptValidation constructor. When set, TxValidator.ValidateTransaction returns immediately before calling the BDK adapter. The flag is only wired from PreValidateTransactions, which is documented (and structurally constrained via quickValidationAllowed) to run only on trusted checkpoint-anchored blocks. The non-quickValidationMode pre-warm path (validateTransactions) deliberately does not set it. Includes unit tests for the option setter, the default value, and a counting BDK validator that asserts the bdk adapter is not invoked when the flag is set.
Contributor
|
🤖 Claude Code Review Status: Complete Current Review: No issues found. This PR adds a performance optimization that skips BDK script validation during legacy IBD for blocks at or below hard-coded checkpoints. The implementation is well-structured with appropriate safety guards: Correctness verified:
|
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-26 16:52 UTC |
icellan
approved these changes
May 26, 2026
freemans13
added a commit
to freemans13/teranode
that referenced
this pull request
May 26, 2026
…deploy branch # Conflicts: # services/legacy/netsync/handle_block_test.go
oskarszoon
approved these changes
May 26, 2026
oskarszoon
left a comment
Contributor
There was a problem hiding this comment.
Approve. Consensus-safe — semantically equivalent to svnode's -assumevalid script-skip with a stricter gate (compiled-in checkpoints vs operator opt-in).
Three independent constraints enforce the trust boundary: flag only set in PreValidateTransactions:888 → only reached via ValidateTransactionsLegacyMode when quickValidationMode == true (handle_block.go:398) → quickValidationAllowed true iff blockHeight ≤ highest checkpoint (mainnet 945000, testnet 1730000, regtest/STN false). Wrong-hash peers disconnected at manager.go:1622-1629; FSM RUN gate refuses RUN until tip ≥ highest checkpoint; merkle-root still verified.
Still enforced independent of the skip: coinbase rejection (TxValidator.go:140-142), coinbase-prevout shape (checkInputs:309-322), BIP68 sequence-lock (Validator.go:1495 — mainnet CSVHeight 419328 ≪ 945000 checkpoint), coinbase maturity at UTXO-store level, block merkle-root. Only thing actually skipped: BDK script execution / sigops / standardness / consensus at TxValidator.go:164.
Worth considering (not blocking):
- Runtime guard refusing flag when
blockHeight > highest checkpoint— currently honor-system gated. - Prom counter on skip site for IBD visibility.
- BIP68 layering test (flag=true + non-final seq +
height ≥ CSVHeightmust reject). assert.Equal→require.EqualatTxValidator_test.go:133.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Options.SkipScriptValidationflag andWithSkipScriptValidationconstructor inservices/validator/options.go.TxValidator.ValidateTransaction, returns immediately before calling the BDK adapter when the flag is set.validator.WithSkipScriptValidation(true)into the per-txValidatecall insidePreValidateTransactions(services/legacy/netsync/handle_block.go).Motivation
PreValidateTransactionsis only ever reached via thequickValidationModepath (prepareSubtrees→ValidateTransactionsLegacyMode), and that path only runs when the block height is at or below the highest hard-coded checkpoint for the active network (quickValidationAllowed). PoW plus checkpoint linkage already establish the chain as canonical, so re-running BDK transaction validation (script execution, sigops, standardness, consensus checks) is pure overhead.CPU profiling of mainnet IBD showed
gobdk.TxValidator.ValidateTransaction(the cgo call) accounting for ~22% of teranode's CPU, even though the surrounding code path already trusts the chain via the checkpoint mechanism. This change eliminates that work on the trusted path.Safety
validateTransactions) deliberately does not set the new flag.SkipScriptValidationis documented as MUST NOT be used on non-trusted paths, and the call-site comment explains why the legacy IBD path can safely set it.Validator.validateTransactionare unaffected — they are gated separately onSkipPolicyChecksand CSV activation height (CSV is post-checkpoint on mainnet).Test plan
go test ./services/validator/... ./services/legacy/netsync/...— 461 tests passgo vet ./services/validator/... ./services/legacy/netsync/...— no issuesTestWithSkipScriptValidation— option setter round-tripTestSkipScriptValidationDefault— default is falseTestTxValidatorSkipsBDKWhenSkipScriptValidationSet— counting bdk validator confirms the BDK adapter is not invoked when the flag is set