[satran004]

Part of #1328.

Problem

BlockFetchService.onRollback ignored a rollback when the cursor was more than 2160 slots ahead of the rollback point. But 2160 is k, the security parameter, and k counts blocks. On mainnet and preprod, 2160 slots is only about 36 minutes, while k blocks is about 12 hours. So every rollback deeper than about 36 minutes was ignored.

Ignoring a rollback is not harmless. The session keeps going and writes the blocks that come after the rollback point on top of newer data. In the fault-injection test, an old session intersected 13,581 slots (about 680 blocks) behind the cursor. Its rollback was ignored, and it wrote an old block again. That left 11 spent outputs in address_utxo_unspent.

What changes

• Depth is counted in blocks. The block number of the rollback point is looked up in the cursor table and compared with k.
• Fallback when the point has no cursor row: use 3k/f slots, the window in which the chain normally has k blocks (129,600 slots on mainnet and preprod). A point can lack a row for two reasons: the range fetch writes one cursor row per batch, and old rows are cleaned up.
• A rollback that is too deep is refused, and the sync restarts from the cursor. Before, the session carried on and wrote old blocks over newer data.
• No cursor yet: nothing to roll back. Before, this was a NullPointerException.
• The genesis point with no hash (sent on some reconnects) is still ignored, as before.

Tests

New unit tests:

• a 680-block rollback is applied;
• exactly k blocks is applied, and k + 1 is refused and restarts;
• a one-block rollback across a slot gap on a sparse devnet (k = 10, f = 1) is applied;
• the slot fallback;
• no cursor;
• the genesis point.

All core tests pass. The rollback fault scenarios (RB01–RB11) will be re-run on preprod with all PRs of #1328, and the results will be posted here.

Note: the restart after a refused rollback is a normal restart request. With #1330, a request that is dropped by the debounce is retried.

🤖 Generated with Claude Code

[beanbot-gh]

Initial automated review · Aster

Hi @satran004, thanks for the PR! 🙏 This is an automatic first check by an AI reviewer. It does not replace maintainer review and may be wrong.

Reviewed commit: 3d7f3d3a13368c1eb5dba37536abca8444b946f8

No issues found in this first check.

Not tested: Code was read only; builds, tests, and rollback fault-injection scenarios were not run.

[satran004]

Fault-injection re-test result

We ran the same 104 fault scenarios again on preprod (benchmark-vm): 43 during the initial sync and 61 at the tip. This time the build had all the fix PRs together: main + #1330, #1331, #1313, #1332, #1271, #1333, #1266 (branch test/recovery-combined).

• All 104 scenarios recovered on their own. None needed a manual restart. In the first run, 7 got stuck.
• No duplicate rows.
• Missing blocks: only the 2 that B04 skips on purpose. B04 sends an undecodable block with continue-on-parse-error=true.
• address_utxo_unspent is exact: 0 missing and 0 extra rows, compared with address_utxo minus tx_input.

Details and the full list of findings: #1328

This PR in the test

• No stale writes: all 11 rollback scenarios at the tip (RB01–RB11) recovered on their own, in 0 to 149 s. No old session wrote behind the cursor. In the first run, those writes left 11 spent outputs in address_utxo_unspent; now there are 0 extra rows.
• The deep-rollback path itself was not run: no scenario rolls back more than 30 blocks, so the new "rollback deeper than k blocks → restart" code is covered only by the unit tests in this PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[satran004]

Updated after an external review of the 2.0.x backport #1341: 2348452. Each finding, its verdict and its test are in the reply on #1339.

[satran004]

Verdict: APPROVED

reviewer1 — original-PR review at 2348452a76fee3f4a42bd6411a95bcfd65bd4aa4.

The rollback fix and tests match approved #1341. The 3k/f fallback is correctly documented as a heuristic, and the sparse/no-cursor regression takes the restart path. No blocking finding remains. The end-to-end deep-rollback scenario remains an accepted coverage follow-up.

The numbered findings refer to the counterpart review of #1341. No additional blocking finding remains.

Validation on combined main: core 244 passed, 1 skipped at d9b1d37282351992da8f45a85e0e3dbe62e5e1e2; eight other relevant suites 197 passed at f0579f9a5f42ae98694d8f35cd0c998295cc1c20, with zero failures/errors in these successful runs. Exact commands, integration resolutions and scope. These are combined-build results, not a separate standalone run of this PR.

[satran004]

Fault-injection rerun on main with all seven fix PRs at their approved heads (test/recovery-combined d9b1d3728): 104 of 104 scenarios recovered, with no duplicate rows and no unexpected missing blocks. The full report is on #1328: #1328 (comment)

For this PR:

• All 11 rollback scenarios (RB01–RB11: rollbacks of 1, 3 and 30 blocks, combined with resets, kills and DB faults) recovered.
• After each rollback the chain checked continuous, apart from the block B04 skips on purpose.

The baseline is unpatched main at 6836c4dd6, run on the same matrix.

[huylevt]

LGTM. Block-based depth check and restarting instead of ignoring look right, and #1341 matches.