fix(legacy): resolve in-block parents at candidate height during legacy subtree validation#1065
Conversation
…cy subtree validation Legacy sync wedges on testnet at block 1730003 — the first block past the highest checkpoint (1730000) containing an in-block tx chain. Past the checkpoint, netsync leaves quickValidationMode and validates subtrees via CheckSubtreeFromBlock, which called ValidateSubtreeInternal with a nil block accumulator. A child spending a same-block parent then resolved the parent through the UTXO-store fallback, whose BlockHeights stay empty until SetMinedMulti runs after block acceptance, so the validator stamped the unconfirmedParentHeight sentinel. In consensus mode the sentinel translates to MEMPOOL_HEIGHT at the BDK boundary and BDK rejects the block with bad-txns-unconfirmed-input-in-block. Pass the knowledge the caller already has: netsync collects, per subtree, the input-parent txids that live in the same block (from the block txMap) and sends them as a new in_block_parent_hashes field on CheckSubtreeFromBlockRequest. The server seeds a block-scoped parentMetadataAccumulator from the list at the candidate block height, so children resolve in-block parents through ParentMetadata instead of the sentinel. Stateless per request: no server-side per-block state, safe with multiple subtreevalidation instances behind load balancing, and covers cross-subtree parents (the child's request carries the parent hash regardless of which instance validated the parent's subtree). An empty list preserves the prior behaviour exactly, so the proto change is backward compatible.
|
🤖 Claude Code Review Status: Complete Current Review: No blocking issues found. The final design is narrow and well-reasoned: a single validator option ( Things I verified:
Main operational risk (already documented by the author, restating for visibility): wire skew. The beta deployment sent this flag as field 11, which post-merge is Existing inline threads (handle_block.go:536, Server.go:853) reference superseded revisions and have author replies explaining the evolution; no action needed. |
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-06-11 11:09 UTC |
…rshalling Sonar flagged the new lines in Server.go (legacy gRPC handler branch) and Client.go (hash marshalling loop) as uncovered. Adds a handler-level test driving CheckSubtreeFromBlock end-to-end through the legacy branch — request hint reaching per-tx validation Options, malformed hash rejected as invalid argument — and a client test asserting the in-block parent hashes round-trip into the request.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… cross-subtree e2e Addresses ctnguyen's review on bsv-blockchain#1065: - R4: the "merkle root enforces the hint" rationale was wrong — the hinted height feeds BDK's per-input protocol-era flag selection, so a wrong assertion can change script validity without touching block content. Rewrote the safety rationale everywhere (proto, helper, interface, netsync collector): the hint is trusted, not verified; it is safe only because the sole producer derives it locally from the node's own txMap of a PoW-checked block, and the field MUST never be populated from peer-forwarded or untrusted data. - R5: added TestValidate_ConsensusAcceptsHintedInBlockParent — the accept-counterpart of TestValidate_ConsensusRejectsUnconfirmedParent, running the real TxValidator + GoBDK with the same fixtures: same unconfirmed parent, ParentMetadata at the candidate height, validation succeeds. Together the pair pins the consensus outcome, not just the options wiring. - R6: added a cross-subtree handler test — parent in subtree 0, child in subtree 1, two sequential CheckSubtreeFromBlock calls; the child's request hint resolves the parent. - R7: renamed the shadowed err to accErr in the legacy branch. - R9: documented the pre-extension coupling (hint supplies height, not body; legacy txs are pre-extended so no UTXO-store body fetch occurs; no Phase-3 backstop on this path).
This comment was marked as outdated.
This comment was marked as outdated.
…solution option The in_block_parent_hashes request list had an unbounded-payload flaw: per-subtree parent hashes approach one hash per input on heavily-chained blocks (a 1M-leaf subtree → ~32MB), past gRPC message limits exactly where legacy sync needs it most — mainnet IBD through big chained blocks. Replace it with validator.WithUnconfirmedParentsAtCandidateHeight(true), set only by the legacy branch of checkSubtreeFromBlock. The validator resolves the unconfirmedParentHeight sentinel to the candidate block height before the BDK call and the BIP68/MTP lookups. On this path the candidate height is the parent's true height, so the substitution is exact — cross-subtree parents included, no shared state, no payload, safe with multiple subtreevalidation instances. Consensus-safety trade, documented on the option and proto field: this is fail-open at tx level for unconfirmed parents that are NOT in the block; the membership backstop is block validation's checkParentsExistOnChain, which legacy netsync runs on every block before acceptance. The flag's contract (locally-held PoW-checked block, membership check before acceptance, block assembly disabled) holds on the legacy path; the peer-facing branch never sets it, pinned by test. The option travels as an optional bool on ValidateTransactionRequest (field 11) so it survives the gRPC hop to a remote validator; the client-build → server-reconstruction mapping is pinned by a round-trip test. Real-BDK accept/reject pair and the cross-subtree sequential handler test carry over from the previous design.
|
Design reworked — the in_block_parent_hashes list is gone. The list had an unbounded-payload flaw we caught after the last round: per-subtree in-block parent hashes approach one hash per input on heavily-chained blocks (a 1M-leaf subtree → ~32MB of hashes), past gRPC message limits exactly where legacy sync needs it most — mainnet IBD through big chained blocks. Replacement: a single validator option, WithUnconfirmedParentsAtCandidateHeight(true), set only by the legacy branch of checkSubtreeFromBlock. The validator resolves the unconfirmedParentHeight sentinel to the candidate block height before the BDK call and the BIP68/MTP lookups. On this path the candidate height is the parent's true height, so the substitution is exact — cross-subtree parents included, no shared state, no payload. Consensus-safety trade, stated explicitly in the option/proto docs: this is fail-open at tx level for mempool floaters; the membership backstop is block validation's checkParentsExistOnChain, which legacy netsync runs on every block before acceptance. The flag's contract (locally-held PoW-checked block, block-level membership check before acceptance, block assembly disabled) holds on the legacy path and the peer-facing branch never sets it — pinned by test. Your R4 concern shaped this: rather than documenting why a trusted hint list mustn't be misused, the trust decision is now a single bool whose misuse surface is one option contract. Carry-overs from the previous round: the real-BDK accept/reject pair (TestValidate_ConsensusRejectsUnconfirmedParent / TestValidate_ConsensusAcceptsUnconfirmedParentAtCandidateHeight, same fixtures, R5), the cross-subtree two-sequential-calls handler test (R6), and a wire round-trip test for the new proto bool (field 11 on ValidateTransactionRequest — needed so the option survives the hop to a remote validator). |
… lag invariant Pass-2 review findings on the reworked design: - P2 (validator/Server.go:485): the fail-open contract was enforced only by convention — the validator honored the flag even with AddTXToBlockAssembly=true (the default). Now validateTransaction hard-errors on that combination, covering both the in-process and gRPC/HTTP validator paths from one enforcement point. The legacy branch pairs the flag with an unconditional WithAddTXToBlockAssembly(false), replacing the FSM-conditional append (a per-subtree gRPC round-trip to derive a value that is always false for this branch's only caller). - P2 (options.go:113): "unconfirmed parent IS a same-block parent" was missing its justification — the sentinel also covers a parent from block N-1 whose async SetMinedMulti has not landed. That window is closed by netsync's waitForPreviousBlockMined running before prepareSubtrees (all ancestors mined-set by induction); documented on the option, with a warning that other callers must establish the same invariant. - Nits: backstop comment now says checkParentsExistOnChain fails the block in validOrderAndBlessed (not "rejects" with an implied invalid marking); proto field 11 is only put on the wire when the flag is set. Tests: guard case added to the real-BDK accept test (flag without assembly-off errors, runs before the accepting call so the store stays clean); handler test asserts the legacy branch pairs flag with assembly-off; wire round-trip asserts unset flag stays off the wire.
Verification ScopeCitation-verified conclusions:
Conclusions that would need a live patch application and build/test run:
The consensus/parity conclusions are citation-verified. I do not claim the PR's test suite is green. Calibration RuleSeverity and author-work happiness measure different things:
Current open high-severity ordering:
Prior P1 is now resolved for the concrete merge-blocking concern: the unsafe block-assembly combination is enforced in both the legacy caller and the validator. I still list it first because it was the previous top finding, but its current severity is low. Overall JudgmentThe author materially improved the PR after the prior review. The old highest-priority concern, "fail-open scope is documented but not code-enforced", is now resolved for block assembly: the legacy branch always disables assembly with the new flag, and the validator hard-errors if any caller combines the flag with assembly enabled. I do not see a direct BDK/bitcoin-sv validation-parity flaw in the core fix. Resolving the teranode sentinel to the candidate height before BDK and before BIP68/MTP consumers is the right shape for a same-block parent in a block-validation context. The remaining concerns are around invalid/floater edge-case side effects, deployment skew, and PR-specific coverage of post-CSV BIP68 behavior. I would be close to comfortable merging this version. The most valuable remaining change would be a focused external-floater regression test that proves final block non-acceptance and documents any validator-store side effects. Detailed Concerns and Prior-Finding Status1. Prior P1 resolved: fail-open block-assembly scope is now code-enforcedStatus: resolved for the previous high-severity concern; residual severity is low. The refreshed code fixes the earlier gap. The legacy branch now constructs More importantly, the validator now enforces the dangerous combination centrally: if This is a substantial improvement. The only residual low-severity nit is that the guard is option-level, not computed from the global 2. Prior P2 partially addressed: external-floater side effects are still not pinnedStatus: partially addressed; current highest open issue. The new commits address the largest part of this concern by preventing block-assembly contamination. A transaction accepted through the fail-open height resolution can no longer be placed into our own block template when the PR's safety contract is followed, and the validator hard-errors if a caller forgets the pairing. The remaining issue is UTXO/txmeta side effects before the block-level parent-membership backstop. With this option set, a parent that exists in the UTXO store with empty That means final block acceptance still appears protected, but the PR does not pin what happens to validator-store mutations from an invalid external-floater case. The comments now say the backstop "fails the block" (PR diff lines 20-27 and 467-479), but the precise behavior is more nuanced: it may be incomplete/retry rather than invalid, and side effects may already exist by that point. Recommended test: construct a legacy block/subtree case where the child spends a parent that exists in the UTXO store with empty 3. Prior P3 partially addressed: rollback and mixed-version deployment remain liveness-sensitiveStatus: partially addressed; second-highest open issue. The refreshed client helper is a good improvement. Deployment skew still matters:
The PR description includes the right deploy note: deploy services together. I would still prefer that note in a durable release/deployment document or near the proto field comment, because this is operationally consensus-adjacent even though it is primarily a liveness failure. 4. Low item resolved/affirmed: MEMPOOL_HEIGHT and BDK era-flag reasoning is correctStatus: resolved/affirmed. The core parity argument checks out. In BDK, consensus script verification rejects For a same-block parent in a block being validated at height 5. Low item open: BIP68/MTP behavior is still not PR-specifically pinnedStatus: open, but lower risk than before because the code placement and comments are good. The implementation substitutes the sentinel before the BIP68/MTP phase, so the intended data flow is correct (PR diff lines 318-342; current source returns into BIP68 only after the CSV gate at The new validator accept test does not exercise BIP68/MTP. It uses mainnet height 257727 (PR diff lines 379-386), and mainnet Recommended coverage: add one post-CSV, pre-Genesis test using 6. Low item partially addressed: test coverage improved, but the legacy handler test is still mostly option propagationStatus: partially addressed. Coverage is meaningfully better now:
The remaining gap is narrower: the new cross-subtree handler test uses 7. Low item resolved: gRPC/proto plumbing and default-false compatibility are now solidStatus: resolved. The new helper avoids putting an explicit false on the wire (PR diff lines 251-262), the server still reconstructs absent/nil as false (PR diff lines 299-305), and the round-trip test covers both set and default cases (PR diff lines 520-555). This makes nil and explicit-false behavior equivalent on the server while reducing unnecessary unknown-field traffic to old validators. The proto change is additive at tag 11 (PR diff lines 644-648), so old receivers ignore it and new receivers default absent to false. That is the safe/fail-closed compatibility stance for a consensus-adjacent flag. The only remaining nit is generated-code churn: the generated comments show 8. Low item partially addressed: maintainability and clarity are much better, with one API sharp edgeStatus: partially addressed. The option comment is unusually detailed, and the new validator-side guard makes the main safety contract executable. That is appropriate for consensus-boundary code. The new helper name also makes the wire-default behavior clearer. The remaining clarity nit is the call shape in the validator hunk: Summary Table
GLOBAL HAPPINESS: 86% |
…the substitution Pass-3 review items: - Floater side effects (current P1): new real-validator integration test (TestLegacyUnconfirmedParent_RealValidatorIntegration) runs the legacy gRPC handler against the real validator + GoBDK + sqlitememory store — unmined parent, child through the genuine UTXO fallback and option — and asserts the post-blessing store state: child meta unmined, parent output spent by the child. The validator-level accept test pins the same contract. Backstop comments corrected: checkParentsExistOnChain returns BlockIncompleteError (retry/catchup-ordering semantics, issue bsv-blockchain#1031, pinned in model/Block_test.go), not an invalid-block marking; the store state equals a policy-mode mempool admission of the same txs and is handled by the same unmined-tx cleanup machinery. - Deployment skew (current P2): full skew matrix documented on the proto field comment — new→old ignores the field (wedge persists), old→new reconstructs false, mixed validator fleets pass-or-wedge per instance, downgrade reintroduces the wedge but corrupts nothing. - Call-shape clarity (P7): resolveUnconfirmedParentsAtCandidateHeight wraps substituteUnconfirmedHeights(…, false) so the consensus-context call site no longer reads as a policy-mode translation. Deferred deliberately: PR-specific post-CSV BIP68 pinning (needs post-CSV pre-Genesis sequence-locked fixtures; the substitution-before- BIP68 data flow and generic MTP-path coverage already exist).
|
Pass-3 items addressed in a0ac819. Current P1 (floater side effects) — pinned twice, plus comment corrections:
Current P2 (deployment skew) — the full skew matrix now lives on the proto field comment (new→old ignores the field and keeps the wedge, old→new reconstructs false, mixed fleets behind LB pass-or-wedge per instance, downgrade reintroduces the wedge but corrupts nothing; upgrade together). Durable and next to the thing that ships. P4 (post-CSV BIP68 pin) — deferred, deliberately. The data flow is covered (substitution happens before the BIP68/MTP phase; generic post-CSV MTP coverage exists in TestValidateTransaction_BIP68PathReadsMTPStore), and pinning it PR-specifically needs post-CSV pre-Genesis sequence-locked fixtures that don't exist yet. Will pick it up as a follow-up rather than hold the wedge fix on it. P5 (real UTXO integration) — covered by the new integration test above. P7 (call-shape sharp edge) — added Residual nit from your item 1 (option-level guard vs global BlockAssembly.Disabled): keeping option-level — as you note, the conservative API contract is the point; a globally-disabled-assembly caller passing the flag without WithAddTXToBlockAssembly(false) should still be told explicitly. protoc version note: regenerated locally (v7.34.1 vs the repo files' v6.33.2). Wire-identical apart from the version comment; if the repo pins a generator in CI I'll regenerate from that — pointer welcome. |
…s keep pre-change behaviour The previous commit's justification for the unconditional WithAddTXToBlockAssembly(false) was wrong: the legacy branch is not exclusive to legacy sync. handleBlockMsg calls HandleBlockDirect unconditionally and block listeners are enabled whenever the FSM is not LEGACYSYNCING, so tip blocks arriving over the legacy bridge while RUNNING also reach this branch — where the old code left assembly enabled. Forcing it false there was an unintended behaviour change (blessed txs from a reorged-out block would vanish from our template). Restore the FSM gate: WithUnconfirmedParentsAtCandidateHeight(true) and WithAddTXToBlockAssembly(false) are paired inside the LEGACYSYNCING/CATCHINGBLOCKS conditional. RUNNING preserves the pre-change behaviour exactly — fail-closed sentinel, assembly enabled. An in-block tx chain in a RUNNING-state legacy-bridge block still fails this branch (same as before this PR); the authoritative tip path is blockvalidation's CheckBlockSubtrees with its accumulator. Tests: fsmStateOverrideClient pins the FSM state (LocalClient always reports RUNNING); new RUNNING-state subtest asserts no flag + assembly enabled; legacy-sync subtests and the real-validator integration test run under LEGACYSYNCING.
|
…ING catch-up wedged Field test on testnet (beta deploy) wedged immediately at 1740437: a restarted node has its FSM restored to RUNNING and catches up the gap over the legacy bridge (handleBlockMsg → HandleBlockDirect runs in every FSM state), so the FSM-gated flag was inactive for exactly the blocks it exists to fix. On a node whose only peers are legacy nodes there is no CheckBlockSubtrees fallback path either. FSM state was the wrong predicate. The consensus-safety conditions (locally-held PoW-checked block; block-level membership check before acceptance) hold for every legacy-branch call, so WithUnconfirmedParentsAtCandidateHeight(true) is now unconditional on the legacy branch. The assembly option returns to exactly upstream's FSM-conditional behaviour: disabled during LEGACYSYNCING/CATCHINGBLOCKS, enabled in RUNNING (reorg resilience). The validator's flag+assembly hard-error is removed — it made the two requirements above mutually exclusive in RUNNING. The combination is safe: a floater child blessed at the candidate height is the same tx policy-mode admission would put into assembly anyway (policy substitutes tip+1 for unconfirmed parents, equal to the candidate height at the tip; era flags cannot differ post-Genesis), and accepted-block txs are mined-removed from assembly as always. Tests: RUNNING subtest now pins flag=on + assembly=on; legacy-sync subtests pin flag=on + assembly=off; guard subtest removed; docs on the option and proto field updated to the compatibility stance.
ordishs
left a comment
There was a problem hiding this comment.
Approve — correct, minimal, and a good architectural choice: a stateless per-request hint that reuses the existing parentMetadataAccumulator / filterParentMetadataForInputs machinery from the CheckBlockSubtrees path rather than introducing server-side per-block state. Backward compatible (empty list = prior behaviour), and the trust model is unchanged (a lying hint yields a block that fails acceptance, never a wrongly-accepted one).
Verified locally: affected packages build; TestCollectInBlockParentHashes, TestBuildInBlockParentAccumulator, and TestLegacySubtreeInBlockParentHint pass under -race. The hand-edited .pb.go rawDesc is internally consistent (message length +53 bytes matches the appended field-6 descriptor).
Minor follow-ups (non-blocking):
-
Wiring test gap — the tests exercise
buildInBlockParentAccumulator+validateSubtreeInternalImpldirectly, but not theServer.checkSubtreeFromBlockglue (request.InBlockParentHashes→ accumulator → impl). That path is only covered by compilation; consider one end-to-end test through the gRPC server method with a populatedInBlockParentHashes. -
Perf sanity check —
collectInBlockParentHashesdoes a mutex-guardedtxMap.Getper input per tx, and on a typical post-checkpoint block most inputs are prior-block (mostly-miss) lookups. It only runs in non-quickValidationMode, so likely fine, but worth confirming the pre-pass adds no meaningful sync latency on a large real block. -
Naming nit — the function collects both intra- and cross-subtree in-block parents (a superset); the comment could say so rather than implying cross-subtree only.
Make sure the rolling-upgrade-skew note (deploy netsync + subtreevalidation together) makes it into the deploy runbook, not just the PR description.
…mber resolution flag to proto field 12 Conflicts resolved: - validator_api.proto: bsv-blockchain#1073 took field 11 for in_block (merged upstream first, owns the number); unconfirmed_parents_at_candidate_height moves to field 12. Both fields kept. - validator_api.pb.go: regenerated from the resolved proto. - validator/Client.go: buildValidateTxRequest carries both InBlock (upstream) and UnconfirmedParentsAtCandidateHeight (this branch). WIRE SKEW WARNING for the existing beta deployment: the beta build sends the resolution flag as field 11, which post-merge means in_block. A beta subtreevalidation talking to a post-merge validator would silently set the wrong option (wedge returns, wrong provenance). Replace beta deployments wholesale; do not mix beta and post-merge services.
|
…cy subtree validation (#1065)
…r with candidate-height resolution Replaces the per-block ParentMetadata accumulator (bsv-blockchain#1013) on the block-validation path with the WithUnconfirmedParentsAtCandidateHeight validator option (introduced for the legacy path in bsv-blockchain#1065), then deletes the accumulator machinery. In-block parents resolve the unconfirmedParentHeight sentinel to the candidate block height inside validateTransaction — identical outcome to the accumulator for genuine in-block parents, without the hot-path seeding, per-tx metadata filtering, per-level mutex merges, or per-tx ParentMetadata wire payload. On a 5.24M-tx chained block this removes ~34% end-to-end overhead. Safety: - PoW (nBits + HasMetTargetDifficulty) now runs before subtree blessing in ValidateBlock, so the fail-open option cannot be triggered by a no-PoW header. - Floater backstop: a parent that is unconfirmed and not in the block fails open at tx level, then block.Valid surfaces ErrBlockIncomplete. When caught up (FSM not CATCHINGBLOCKS/LEGACYSYNCING) that is a genuine floater (SetMinedMulti->MinedSet invariant), so the block is invalidated/rolled back at every acceptance sink (optimistic background, non-optimistic, revalidation worker); while syncing it stays incomplete and is retried, preserving bsv-blockchain#1031. - Recorded parent BlockHeights are untouched (sentinel exact-match only), so fork/revalidation height resolution is unchanged. Deprecates proto field 10 (parent_metadata) via reserved; deletes ParentTxMetadata. Preserves the in_block provenance option (bsv-blockchain#1073, field 11); unconfirmed-parents resolution stays field 12. Closes bsv-blockchain#1066
…-legacysyncing Eleven upstream commits, several touching legacy code this branch also changed. Resolutions: - netsync/manager.go: upstream bsv-blockchain#1067 deleted the catchingBlocks early-return in handleBlockMsg (the swallowed orphan tip is the legacy batch-continuation signal; swallowing it stalled sync). Took upstream's always-request behaviour and updated the surrounding comments to post-LEGACYSYNCING wording. - subtreevalidation/Server.go: kept upstream bsv-blockchain#1065's richer rationale for disabling block-assembly additions, on this branch's CATCHINGBLOCKS-only condition. - Re-applied the LEGACYSYNCING removal to symbols upstream reintroduced: validator publishPolicyRejectedTx gate (bsv-blockchain#799) now checks CATCHINGBLOCKS only; the new legacy gate-pinning tests (bsv-blockchain#1065) drive CATCHINGBLOCKS instead of the removed state; comment references updated in adaptivefetch, Validator_test, and the Server.go incident note.
Problem
Legacy sync wedges on testnet at block 1730003 with:
1730003 is the first block past the highest testnet checkpoint (1730000) that contains an in-block tx chain (
0dc2a467…spends an output of35b7b758…in the same block). Past the checkpoint, netsync leaves quickValidationMode and validates subtrees throughCheckSubtreeFromBlock.A tx spending a same-block parent finds that parent in the UTXO store with empty BlockHeights —
SetMinedMultionly runs after block acceptance — so the validator stamps theunconfirmedParentHeightsentinel. In consensus mode the sentinel translates toMEMPOOL_HEIGHTat the BDK boundary and BDK rejects it (txvalidator.cpp:779): BDK needs the real UTXO height to select per-input script-era flags (pre/post-Genesis) and refuses to guess in consensus context. Every post-checkpoint block with an in-block chain fails the same way; sync is stuck at 1730002.Fix
New validator option
WithUnconfirmedParentsAtCandidateHeight(true), set only by the legacy branch ofcheckSubtreeFromBlock. When set,validateTransactionresolves the sentinel to the candidate block height before any consumer sees it — both the BDK era-flag selection and the BIP68/MTP lookups. On this path the candidate height is the parent's true height (the parent is in the block being validated), so the substitution is exact, including cross-subtree parents, and needs no shared state — safe with multiple subtreevalidation instances behind load balancing.Plumbing: one optional bool on
ValidateTransactionRequest(field 11) so the option survives the gRPC hop to a remote validator; client build + server reconstruction mapped and pinned by a round-trip test.Consensus safety
This is fail-open at tx level, deliberately scoped:
checkParentsExistOnChain(model/Block.go), which legacy netsync runs on every block before acceptance — an unmined external parent fails the block there.Tests
TestValidate_ConsensusRejectsUnconfirmedParent(existing) andTestValidate_ConsensusAcceptsUnconfirmedParentAtCandidateHeight(new) — real TxValidator + GoBDK, same fixtures: sentinel rejects without the option, validates with it. The pair pins the consensus outcome in both directions.TestCheckSubtreeFromBlockLegacyUnconfirmedParents— handler-level: legacy branch sets the flag on per-tx validation Options; cross-subtree case (parent subtree 0, child subtree 1, two sequential calls) validates; peer-facing path does NOT set the flag.TestUnconfirmedParentsAtCandidateHeight_WireRoundTrip— flag survives client build → proto marshal → server reconstruction; default stays false.go test -racegreen on validator, subtreevalidation, legacy/netsync, blockvalidation. vet / staticcheck / golangci-lint / gosec: no new findings.Deploy note
Rolling-upgrade skew: new subtreevalidation + old validator service → unknown proto field ignored → old (wedging) behaviour until the validator is upgraded too. Single-binary deployments (quickstart compose) are unaffected. Deploy services together.