TL;DR

Validator's in-memory Median Time Past cache (mtpStore) had no synchronisation. Two concurrent populate calls — followed by a third reader — could each see an empty cache, both fetch ~1.45 M entries, race on the append, and corrupt the slice. Once corrupted, the BIP68 guard in validateTransaction rejected every transaction, and ValidateSubtreeInternal's outer retry loop re-issued the 1.45 M-entry fetch on every retry — pegging CPU and stalling sync indefinitely.

This PR adds a sync.RWMutex around mtpStore, so populates serialise (the second caller fast-paths out instead of re-fetching) and readers don't observe torn state during a populate.

Why this only surfaced at one block on testnet

The bug is not block-specific — there is nothing about block 1,451,504 that triggers it. The trigger is a timing window during the first EnsureMTPLoaded call after validator startup, which exists at every block ≥ CSVHeight (testnet CSVHeight = 770,112).

What aligned on the testnet stall:

1. The validator was cold-started (empty mtpStore) and was being driven by a legacy sync.
2. The first legacy CheckSubtreeFromBlock RPC for a post-CSV block kicked off EnsureMTPLoaded, which had to fetch ~1.45 M MTP entries from the blockchain store. That fetch is the only slow operation in the call.
3. While the fetch was still in flight, the legacy client's request timeout fired and it retried CheckSubtreeFromBlock. The retry entered EnsureMTPLoaded with len(mtpStore) still 0, started its own 1.45 M-entry fetch, and raced the original on the slice append.
4. The corruption tripped the BIP68 height guard in validateTransaction. ValidateSubtreeInternal's outer retry loop re-issued the fetch on every retry — minutes per attempt — producing the multi-hour CPU peg observed on the box.

Block 1,451,504 is simply where the validator happened to be when these timings aligned. Mainnet has not surfaced it because mainnet validators tend to be long-lived (the cache populates once and the race window closes), and mainnet legacy peers are less prone to the retry-on-timeout pattern testnet sees.

Detail: the two races

1. Same-block writer-writer. Two concurrent CheckSubtreeFromBlock RPCs (legacy retries a call while the first is still in flight) both observed an empty mtpStore, both fetched ~1.45 M MTP entries, and both raced on the append — corrupting the store. The corruption tripped the guard in validateTransaction, which surfaced an error to the outer retry loop in ValidateSubtreeInternal. Each retry repeated the 1.45 M-entry fetch, producing the multi-hour CPU peg and sync stall observed on testnet around block 1,451,504.
2. Cross-block reader-writer. While block N's per-transaction goroutines read mtpStore inside validateTransaction, block N+1's EnsureMTPLoaded can run concurrently. The append re-allocates the backing array and the overlap patch mutates entries in place, so unsynchronised readers observe torn slice headers / cell values.

Fix

services/validator/Validator.go:

• mtpMu sync.RWMutex on Validator.
• EnsureMTPLoaded acquires the write lock for the full body. The second same-block caller fast-paths out after acquiring the lock when the range it needs is already populated (no second gRPC fetch).
• validateTransaction reads the MTP values via a small readMTPsLocked helper that takes the read lock only for the lookups themselves and releases it before ValidateBIP68 runs. Cross-block writers serialise behind any in-flight readers; the per-tx ECDSA / sequence-lock work is unconstrained by the lock.

services/validator/Validator_test.go:

• TestEnsureMTPLoaded_ConcurrentCallsNeitherHangsNorRaces: two same-block EnsureMTPLoaded calls against an empty store. Mock is .Once() and the test asserts mockClient.AssertExpectations(t), so a regression that re-fetches without racing (the production failure mode) fails the assertion. Fails under -race without the write lock; passes with it.
• TestEnsureMTPLoaded_CrossBlockReadersAndWritersDoNotRace: eight readers indexing mtpStore via the validateTransaction pattern while a writer extends the store to a later block height. Fails under -race without the read lock; passes with it. Asserts mock expectations to defend against accidental over-fetch.

Test plan

• go vet ./services/validator/... — clean
• go test -race ./services/validator/ — 277 passed, 0 failed
• go test -race -run TestEnsureMTPLoaded ./services/validator/ — both targeted race tests pass
• golangci-lint run ./services/validator/... — clean
• staticcheck ./services/validator/... — clean
• Reviewer to confirm no other call sites read mtpStore outside validateTransaction (already grepped — only EnsureMTPLoaded itself touches it elsewhere, under the write lock)

Commits

• cdec5b3d3 — original fix: sync.RWMutex + reader / writer locking + both race tests
• d334b5f77 — review feedback: extract readMTPsLocked to drop the read lock before ValidateBIP68; tighten test mock expectations (Once() + AssertExpectations); replace require.* in goroutines with main-goroutine assertion

Notes for reviewer

The mtpStore field and EnsureMTPLoaded were introduced in #547 ("Re-implement BIP68"); the race has been latent since that PR.

EnsureMTPLoaded intentionally holds the write lock through GetMedianTimePastRange — releasing for the fetch reintroduces the same-block redundant-fetch race. The cold-start fetch (~1.45 M entries) runs once at startup before any per-tx readers; per-block extension fetches are tiny (12-overlap + new heights) and fast. A golang.org/x/sync/singleflight wrapper would be a cleaner alternative if read-blocking ever shows up in profiling. (Open thread: r3213721754.)