[DEFER] feat(derivation): batch verification + L1 reorg detection#950
[DEFER] feat(derivation): batch verification + L1 reorg detection#950curryxbo wants to merge 12 commits into
Conversation
Replace the challenge mechanism with a batch-data-as-source-of-truth model: - When local L2 blocks don't match L1 batch data, rollback and re-derive from L1 instead of issuing a state challenge - Add L1 reorg detection for non-finalized confirmation modes (latest/safe) by tracking L1 block hashes and comparing on each derivation loop - L1 reorg only triggers DB cleanup and re-derivation; L2 rollback is only triggered when batch data comparison actually fails Key changes: - Remove validator/challenge dependency from derivation - Add verifyBlockContext() and verifyBatchRoots() for batch data comparison - Add detectReorg() with configurable check depth (default 64 blocks) - Add rollbackLocalChain() stub (TODO: geth SetHead API integration) - Add L1 block hash tracking in DB for reorg detection - Add metrics: l1_reorg_detected_total, l2_rollback_total, block_mismatch_total - Add --derivation.reorgCheckDepth CLI flag Made-with: Cursor
Bug fixes: - Fix detectReorg traversal direction: iterate oldest-to-newest to find the earliest divergence point, not the latest - Make rollbackLocalChain return error instead of nil to prevent silent fall-through to NewSafeL2Block on an already-existing block number - Handle edge case in handleL1Reorg when reorgAtL1Height <= startHeight Optimizations: - Skip recordL1Blocks in finalized mode (reorg detection is disabled, recording L1 hashes is unnecessary overhead) Cleanup: - Remove unused BatchIndex/L2EndBlock fields from DerivationL1Block - Add batch-internal tx count consistency check in verifyBlockContext - Use Info instead of Error for L1 reorg detection logs (expected in latest mode) - Update DERIVATION_REFACTOR.md with review feedback changes Made-with: Cursor
- Handle startHeight==0 edge case in handleL1Reorg by writing 0 instead of skipping the reset - Change recordL1Blocks to return on first failure instead of continue, preventing gaps in L1 block hash tracking that could cause missed reorgs - Return immediately after L1 reorg handling instead of continuing the same derivation loop, avoiding recording unstable L1 hashes during ongoing reorgs - Clarify verifyBlockContext tx count check is batch-internal consistency, not local-vs-L1 comparison (local-vs-L1 covered by state root in verifyBatchRoots) Made-with: Cursor
…cording fails recordL1Blocks now returns error. If any L1 header fetch fails mid-range, derivationBlock returns early without calling WriteLatestDerivationL1Height. This prevents permanent gaps in L1 block hash tracking that would make reorgs in the gap range undetectable. Made-with: Cursor
…reorg check, fix baseFee nil handling
1. Add `halted` flag: when rollback stub fails on batch mismatch, derivation
stops instead of infinitely retrying the same batch with wasted L1 RPCs.
2. Optimize detectReorg: check newest saved block first — if it matches,
skip the full scan (1 RPC instead of 64 in the common no-reorg case).
3. Fix verifyBlockContext BaseFee: explicitly error when one side is nil
and the other is not, instead of silently skipping the comparison.
4. Fix doc: DerivationL1Block field list now matches code ({Number, Hash}).
Made-with: Cursor
Expose morphnode_derivation_halted gauge (0/1) so operators can set up alerts when derivation halts due to unrecoverable batch mismatch. All three code paths that set d.halted=true now also call metrics.SetHalted(). Made-with: Cursor
…ix doc env var 1. Add nil check for lastHeader after derive() returns — if blockContexts is empty, skip the batch instead of panicking on lastHeader.Number. 2. Fix DERIVATION_REFACTOR.md: env var is MORPH_NODE_DERIVATION_REORG_CHECK_DEPTH (was missing NODE_ prefix). Made-with: Cursor
…ment halted metric Made-with: Cursor
Made-with: Cursor
Extract newly added functions into dedicated files for clarity: - verify.go: rollbackLocalChain, verifyBatchRoots, verifyBlockContext - reorg.go: detectReorg, handleL1Reorg, recordL1Blocks Existing batch parsing code stays in derivation.go to keep the diff scoped to this PR's changes only. No logic changes — pure file split. Made-with: Cursor
…org-detection Resolve derivation conflicts in favor of main's cleaner state: - Drop the L2Next/nextClient upgrade-switch plumbing (already removed on main): revert RetryableClient, executor, derivation/config, and flags to main; remove switchTime/useZktrie skip path in verifyBatchRoots. - Keep this branch's batch verification (verifyBatchRoots / verifyBlockContext), L1 reorg detection + handler, halted gauge, and rollback-then-rederive flow on root mismatch. - Reintegrate main's validator.ChallengeState path on root mismatch before attempting rollback; restore validator wiring in NewDerivationClient and node main.
📝 WalkthroughWalkthroughThis pull request refactors the derivation module to prioritize L1 batch data as the source of truth for L2 state. It introduces L1 reorg detection with persistent block-hash storage, batch/block verification flows, and halts derivation on unrecoverable mismatches. The validator/challenge-based mismatch handling is removed. ChangesL1 Reorg Detection and Batch Verification Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
node/derivation/config.go (1)
117-119: 💤 Low valueConsider adding validation for ReorgCheckDepth if zero is not a valid configuration.
Other configuration values in
SetCliContextvalidate for zero (lines 100-116). IfReorgCheckDepth = 0is invalid, consider adding a similar check here. If zero is intentionally allowed (e.g., to disable reorg detection), this can be ignored.🛡️ Proposed validation
if ctx.GlobalIsSet(flags.DerivationReorgCheckDepth.Name) { c.ReorgCheckDepth = ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name) + if c.ReorgCheckDepth == 0 { + return errors.New("invalid reorgCheckDepth") + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node/derivation/config.go` around lines 117 - 119, SetCliContext currently assigns c.ReorgCheckDepth from ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name) without validating zero; if zero is invalid ensure you add the same validation pattern used for other fields: after the assignment in SetCliContext check if c.ReorgCheckDepth == 0 and handle it (return an error or call log/exit with a clear message referencing ReorgCheckDepth and flags.DerivationReorgCheckDepth.Name), or if zero is intended to disable behavior add an explicit comment and/or document that case instead.node/derivation/derivation.go (1)
67-67: 💤 Low valueStale comment on
halted— rollback is implemented in this PR.The comment says "rollback is not yet implemented" but
haltedis now set precisely when an implemented rollback or re-derive fails (lines 301, 321, 636). Update the comment to reflect actual semantics so future readers don't think this is still a placeholder.📝 Proposed wording
- halted bool // set when an unrecoverable mismatch is detected but rollback is not yet implemented + halted bool // set when rollback or post-rollback re-derive/verify fails; blocks further derivation until manual intervention🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node/derivation/derivation.go` at line 67, Update the stale comment on the halted field to reflect current semantics: replace "rollback is not yet implemented" with a brief description that halted is set when an unrecoverable mismatch is detected after an attempted rollback or re-derive (i.e., when those recovery attempts fail), so readers know halted represents a permanent/terminal failure state rather than a placeholder for unimplemented rollback.node/derivation/reorg.go (1)
75-93: 💤 Low value
handleL1Reorgreturnserrorbut can never produce one — drop the return value.
DeleteDerivationL1BlocksFromandWriteLatestDerivationL1Heightdon't return errors, so the function always returnsnil. Either propagate real errors from the DB layer (if they're plausible) or simplify the signature so callers don't write deadif err != nilbranches at the call site inderivationBlock(line 203‑205).♻️ Proposed simplification
-func (d *Derivation) handleL1Reorg(reorgAtL1Height uint64) error { +func (d *Derivation) handleL1Reorg(reorgAtL1Height uint64) { d.logger.Info("L1 reorg detected, cleaning DB records and restarting derivation from reorg point", "reorgAtL1Height", reorgAtL1Height) @@ - return nil }And in
derivation.go:- if err := d.handleL1Reorg(*reorgAt); err != nil { - d.logger.Error("handle L1 reorg failed", "err", err) - } + d.handleL1Reorg(*reorgAt)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node/derivation/reorg.go` around lines 75 - 93, The handleL1Reorg function currently returns an error but never produces one; change its signature to return nothing (remove error) and update all callers (e.g., derivationBlock which checks err after calling handleL1Reorg) to stop treating it as error-returning; leave the internal calls to DeleteDerivationL1BlocksFrom and WriteLatestDerivationL1Height as-is since they don't return errors, or if you prefer to propagate DB errors instead, modify those DB methods to return errors and then have handleL1Reorg return/propagate them — but do not keep a nil-only error return in handleL1Reorg.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@node/db/keys.go`:
- Around line 10-11: The file fails gofmt due to formatting around the
constant/variable declarations for derivationL1HeightKey and
derivationL1BlockPrefix; run gofmt -w on the file (or adjust spacing so the
declarations for derivationL1HeightKey and derivationL1BlockPrefix are formatted
according to gofmt) and save the file so the pipeline passes.
In `@node/derivation/reorg.go`:
- Around line 36-64: The fast and slow reorg checks calling
d.l1Client.HeaderByNumber must treat ethereum.NotFound as a reorg (not an RPC
error): update both HeaderByNumber error branches in the logic around
newestHeader and header to check errors.Is(err, ethereum.NotFound) and, if true,
call handleL1Reorg (or return the same signal the function uses to indicate a
reorg) instead of returning the error; for non-NotFound errors continue to
return them as RPC failures. Add the imports "errors" and
"github.com/morph-l2/go-ethereum" so errors.Is and ethereum.NotFound are
available, and ensure the code uses the same identifiers (HeaderByNumber,
d.l1Client, handleL1Reorg, ethereum.NotFound) referenced in the diff.
---
Nitpick comments:
In `@node/derivation/config.go`:
- Around line 117-119: SetCliContext currently assigns c.ReorgCheckDepth from
ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name) without validating zero;
if zero is invalid ensure you add the same validation pattern used for other
fields: after the assignment in SetCliContext check if c.ReorgCheckDepth == 0
and handle it (return an error or call log/exit with a clear message referencing
ReorgCheckDepth and flags.DerivationReorgCheckDepth.Name), or if zero is
intended to disable behavior add an explicit comment and/or document that case
instead.
In `@node/derivation/derivation.go`:
- Line 67: Update the stale comment on the halted field to reflect current
semantics: replace "rollback is not yet implemented" with a brief description
that halted is set when an unrecoverable mismatch is detected after an attempted
rollback or re-derive (i.e., when those recovery attempts fail), so readers know
halted represents a permanent/terminal failure state rather than a placeholder
for unimplemented rollback.
In `@node/derivation/reorg.go`:
- Around line 75-93: The handleL1Reorg function currently returns an error but
never produces one; change its signature to return nothing (remove error) and
update all callers (e.g., derivationBlock which checks err after calling
handleL1Reorg) to stop treating it as error-returning; leave the internal calls
to DeleteDerivationL1BlocksFrom and WriteLatestDerivationL1Height as-is since
they don't return errors, or if you prefer to propagate DB errors instead,
modify those DB methods to return errors and then have handleL1Reorg
return/propagate them — but do not keep a nil-only error return in
handleL1Reorg.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b5b8058-c8cb-4d12-8a36-52e5f897e512
📒 Files selected for processing (10)
node/db/keys.gonode/db/store.gonode/derivation/DERIVATION_REFACTOR.mdnode/derivation/config.gonode/derivation/database.gonode/derivation/derivation.gonode/derivation/metrics.gonode/derivation/reorg.gonode/derivation/verify.gonode/flags/flags.go
| derivationL1HeightKey = []byte("LastDerivationL1Height") | ||
| derivationL1BlockPrefix = []byte("derivL1Block") |
There was a problem hiding this comment.
Fix formatting to pass gofmt check.
The pipeline reports a gofmt failure at line 10. Please run gofmt -w node/db/keys.go to resolve the formatting issue.
🧰 Tools
🪛 GitHub Actions: Node / 0_check.txt
[error] 10-10: golangci-lint (gofmt): File is not properly formatted. Run gofmt on this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node/db/keys.go` around lines 10 - 11, The file fails gofmt due to formatting
around the constant/variable declarations for derivationL1HeightKey and
derivationL1BlockPrefix; run gofmt -w on the file (or adjust spacing so the
declarations for derivationL1HeightKey and derivationL1BlockPrefix are formatted
according to gofmt) and save the file so the pipeline passes.
| // Fast path: check the newest block first. If it matches, no reorg occurred. | ||
| newest := savedBlocks[len(savedBlocks)-1] | ||
| newestHeader, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(newest.Number))) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", newest.Number, err) | ||
| } | ||
| if newestHeader.Hash() == common.BytesToHash(newest.Hash[:]) { | ||
| return nil, nil | ||
| } | ||
|
|
||
| // Slow path: reorg detected. Scan oldest-to-newest to find the earliest divergence. | ||
| for i := 0; i < len(savedBlocks); i++ { | ||
| block := savedBlocks[i] | ||
| header, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(block.Number))) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", block.Number, err) | ||
| } | ||
| savedHash := common.BytesToHash(block.Hash[:]) | ||
| if header.Hash() != savedHash { | ||
| d.logger.Info("L1 block hash mismatch detected", | ||
| "height", block.Number, | ||
| "savedHash", savedHash.Hex(), | ||
| "currentHash", header.Hash().Hex(), | ||
| ) | ||
| return &block.Number, nil | ||
| } | ||
| } | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What error does morph-l2 go-ethereum ethclient.HeaderByNumber return when the requested block number is not found, and is errors.Is(err, ethereum.NotFound) the recommended check?
💡 Result:
morph-l2/go-ethereum’s ethclient.Client.HeaderByNumber returns ethereum.NotFound only when the RPC returns JSON null for eth_getBlockByNumber; specifically it decodes into a *types.Header and if that pointer is nil, it sets err = ethereum.NotFound [1]. When the requested block number is “not found” because the underlying RPC returns null, HeaderByNumber therefore returns ethereum.NotFound, and errors.Is(err, ethereum.NotFound) will be true (this is exactly how ethclient implements the check) [1]. Notes / common confusion: if you request “pending” (e.g., number -1) rather than a historical block number, ethclient issues show that HeaderByNumber can return other errors such as "missing required field 'miner' for Header" instead of NotFound, due to header unmarshal/field requirements for pending headers [2][3]. So the NotFound check is recommended for actual missing blocks (null result), but not for malformed/unmarshal errors that arise from special tags like pending [2]. Recommendation: for missing historical blocks, check errors.Is(err, ethereum.NotFound) (it matches ethclient’s own behavior) [1].
Citations:
- 1: https://github.com/ethereum/go-ethereum/blob/master/ethclient/ethclient.go
- 2: ethclient.Client#HeaderByNumber fails for pending block ethereum/go-ethereum#22507
- 3: ethclient using HeaderByNumber and BlockByNumber return error "missing required field 'miner' for Header" ethereum/go-ethereum#21696
🏁 Script executed:
# Check the file structure and imports
head -35 node/derivation/reorg.goRepository: morph-l2/morph
Length of output: 1080
🏁 Script executed:
# Check if ethereum.NotFound is imported
rg "ethereum.NotFound|errors.Is" node/derivation/reorg.goRepository: morph-l2/morph
Length of output: 40
🏁 Script executed:
# Get the complete detectReorg function including lines before 36
sed -n '20,70p' node/derivation/reorg.goRepository: morph-l2/morph
Length of output: 1876
🏁 Script executed:
# Check all imports in the file to see if ethereum or errors packages are available
head -15 node/derivation/reorg.goRepository: morph-l2/morph
Length of output: 343
🏁 Script executed:
# Search for any other error handling or ethereum.NotFound usage in the derivation package
rg "ethereum\.|errors\." node/derivation/ | head -20Repository: morph-l2/morph
Length of output: 986
🏁 Script executed:
# Check if there are other functions in reorg.go that might handle this case
rg "ethereum.NotFound|errors.Is" node/derivation/Repository: morph-l2/morph
Length of output: 212
🏁 Script executed:
# Check what ethereum.NotFound actually is in morph-l2/go-ethereum
rg "NotFound" node/ --type go -A 2 | head -30Repository: morph-l2/morph
Length of output: 1414
🏁 Script executed:
# Search across the entire codebase to see how ethereum.NotFound is used elsewhere
rg "ethereum\.NotFound" --type goRepository: morph-l2/morph
Length of output: 4147
Chain-shortening reorgs are not detected — HeaderByNumber errors must distinguish missing blocks from RPC failures.
When L1 reorgs to a shorter chain (saved newest height beyond current L1 tip), d.l1Client.HeaderByNumber() returns ethereum.NotFound. The current code treats this as a generic RPC failure, propagates the error, and halts derivation until manual intervention. The reorg is never detected.
Both the fast path (line 41) and slow path (line 51) need to check errors.Is(err, ethereum.NotFound) separately from other errors. When a block is not found, treat it as reorg evidence and invoke handleL1Reorg. For other errors, return them as actual RPC failures.
You'll also need to add these imports: "errors" and "github.com/morph-l2/go-ethereum".
Suggested fix sketch
newestHeader, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(newest.Number)))
- if err != nil {
+ if errors.Is(err, ethereum.NotFound) {
+ // L1 chain shortened: fall through to slow path to find the reorg point
+ } else if err != nil {
return nil, fmt.Errorf("failed to get L1 header at %d: %w", newest.Number, err)
- }
- if newestHeader.Hash() == common.BytesToHash(newest.Hash[:]) {
- return nil, nil
+ } else if newestHeader.Hash() == common.BytesToHash(newest.Hash[:]) {
+ return nil, nil
}
for i := 0; i < len(savedBlocks); i++ {
block := savedBlocks[i]
header, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(block.Number)))
+ if errors.Is(err, ethereum.NotFound) {
+ return &block.Number, nil
+ }
if err != nil {
return nil, fmt.Errorf("failed to get L1 header at %d: %w", block.Number, err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fast path: check the newest block first. If it matches, no reorg occurred. | |
| newest := savedBlocks[len(savedBlocks)-1] | |
| newestHeader, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(newest.Number))) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", newest.Number, err) | |
| } | |
| if newestHeader.Hash() == common.BytesToHash(newest.Hash[:]) { | |
| return nil, nil | |
| } | |
| // Slow path: reorg detected. Scan oldest-to-newest to find the earliest divergence. | |
| for i := 0; i < len(savedBlocks); i++ { | |
| block := savedBlocks[i] | |
| header, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(block.Number))) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", block.Number, err) | |
| } | |
| savedHash := common.BytesToHash(block.Hash[:]) | |
| if header.Hash() != savedHash { | |
| d.logger.Info("L1 block hash mismatch detected", | |
| "height", block.Number, | |
| "savedHash", savedHash.Hex(), | |
| "currentHash", header.Hash().Hex(), | |
| ) | |
| return &block.Number, nil | |
| } | |
| } | |
| return nil, nil | |
| } | |
| // Fast path: check the newest block first. If it matches, no reorg occurred. | |
| newest := savedBlocks[len(savedBlocks)-1] | |
| newestHeader, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(newest.Number))) | |
| if errors.Is(err, ethereum.NotFound) { | |
| // L1 chain shortened: fall through to slow path to find the reorg point | |
| } else if err != nil { | |
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", newest.Number, err) | |
| } else if newestHeader.Hash() == common.BytesToHash(newest.Hash[:]) { | |
| return nil, nil | |
| } | |
| // Slow path: reorg detected. Scan oldest-to-newest to find the earliest divergence. | |
| for i := 0; i < len(savedBlocks); i++ { | |
| block := savedBlocks[i] | |
| header, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(block.Number))) | |
| if errors.Is(err, ethereum.NotFound) { | |
| return &block.Number, nil | |
| } | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get L1 header at %d: %w", block.Number, err) | |
| } | |
| savedHash := common.BytesToHash(block.Hash[:]) | |
| if header.Hash() != savedHash { | |
| d.logger.Info("L1 block hash mismatch detected", | |
| "height", block.Number, | |
| "savedHash", savedHash.Hex(), | |
| "currentHash", header.Hash().Hex(), | |
| ) | |
| return &block.Number, nil | |
| } | |
| } | |
| return nil, nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node/derivation/reorg.go` around lines 36 - 64, The fast and slow reorg
checks calling d.l1Client.HeaderByNumber must treat ethereum.NotFound as a reorg
(not an RPC error): update both HeaderByNumber error branches in the logic
around newestHeader and header to check errors.Is(err, ethereum.NotFound) and,
if true, call handleL1Reorg (or return the same signal the function uses to
indicate a reorg) instead of returning the error; for non-NotFound errors
continue to return them as RPC failures. Add the imports "errors" and
"github.com/morph-l2/go-ethereum" so errors.Is and ethereum.NotFound are
available, and ensure the code uses the same identifiers (HeaderByNumber,
d.l1Client, handleL1Reorg, ethereum.NotFound) referenced in the diff.
Scope
This PR carries the work originally on PR #948 that falls outside the current narrow SPEC-005 scope (morph-l2/morph-specs#19, spec PR #18). Whether to merge is undecided.
Includes the early "L2 state tiering + L1 reorg detection + chain-head rollback executor" work:
Relationship to the SPEC-005 narrow PR
The current SPEC-005 scope (per spec PR #18) is limited to:
Everything in this PR falls under SPEC-005 §3 (non-goals) or §8 (future work). Whether and when to merge is decided independently and does not block PR #951.
Code overlap with the narrow PR
Origin
Split out of PR #948 (now closed). Commit range: `8e559f60` through `b1784972` ("Phase A").
Test plan