Skip to content

fix(staking): replaying a block keeps a pool registration a registration - #1335

Open
satran004 wants to merge 4 commits into
mainfrom
fix/pool-registration-replay
Open

satran004 wants to merge 4 commits into
mainfrom
fix/pool-registration-replay

Conversation

@satran004

@satran004 satran004 commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Part of #1328 (fault-injection / auto-recovery fixes). Found in the re-test with all fix PRs.

Problem

yaci-store processes blocks at least once: after a restart it processes blocks again from the cursor, and the rows they wrote the first time can still be in the database.

PoolStatusProcessor.handlePoolRegistration decides between REGISTRATION and UPDATE by looking up the pool's latest REGISTRATION row up to the current epoch. When the block is processed again, that lookup finds the row the block itself wrote the first time. The registration is then saved again (same primary key) as an UPDATE: amount 0, and the active epoch is +3 instead of +2. The pool's next certificate then finds no registration and becomes the REGISTRATION.

Seen on preprod after scenario D10 (Postgres killed during the initial sync), pool 7cc911f0…:

slot epoch fault-free instance after the replay
62368918 148 REGISTRATION, 500 ADA, active 150 UPDATE, 0 ADA, active 151
64580884 153 UPDATE, 0 ADA, active 156 REGISTRATION, 500 ADA, active 155

Adapot's DepositSnapshotService sums pool.amount per epoch, so the 500 ADA deposit is counted in epoch 153 instead of 148. Nothing is logged.

Impact

  1. Deposits: adapot's DepositSnapshotService sums pool.amount per epoch, so the 500 ADA deposit is counted in the wrong (later) epoch.

  2. Stake and rewards: StakeSnapshotService drops delegations made before the pool's registration_slot. The replay moves registration_slot forward (in the example, from 62368918 to 64580884). So delegations to the pool made between those two slots, and never moved, are left out of epoch_stake. That makes these wrong:

    • the pool's stake;
    • rewards for the pool and its delegators;
    • the SPO voting stake.

    This follows from the code. It was not measured, because adapot was not enabled on the test instances.

  3. Pool API: the first registration shows as an UPDATE, and the active epochs are off by one.

There is no error or warning. Only a resync, or a comparison with another source, shows it.

How often it can happen

It needs both of these:

  • a crash or failure after a block's data is saved but before its cursor is saved, so the block is processed again on restart;
  • a pool's first registration (or a re-registration after retirement) among the blocks that are processed again.

At the tip: usually only the last block is processed again. On preprod there was about 1 such certificate per epoch over the last 20 epochs, and an epoch has about 21,600 blocks. So a crash at the tip hits this very rarely.

During the initial sync: a restart can process a whole batch again (100 blocks by default, with several batches running in parallel). Preprod has 711 such certificates in about 5.4 million blocks, about 1 in 7,500 blocks. So a crash during the initial sync hits one roughly 1–5% of the time, and more often in busy epochs (preprod epoch 28 has 44). On mainnet, the first Shelley epochs (208 onward) had far more pool registrations, so a crash there is much more likely to hit it.

In our test it happened once in 43 initial-sync fault scenarios.

Fix

The lookup now only considers registrations that come before the current certificate in chain order (slot, then tx index, then cert index). A replayed block gets the same answer as the first time.

Tests

PoolStatusProcessorReplayIT (H2), 5 tests. Without the fix, the 2 replay tests fail.

  • Replaying a first registration keeps it a REGISTRATION with the same row.
  • Replaying a registration and a later update keeps both rows unchanged.
  • A registration and an update in the same transaction (cert 0 and cert 1), processed and then replayed, keep their order. Exactly one REGISTRATION is returned, and that list is the count published in StakingDepositEvent.
  • A re-registration after the pool retired stays a REGISTRATION when replayed.
  • An update in the same block as the registration is still an update.

All 32 staking tests pass. Reviewed by Codex and a review agent; neither found anything that must change.

Not in this PR

  • Existing databases that already hit this keep the wrong rows. A resync fixes them, or replaying the affected blocks.
  • DRep has a similar lookup (DRepStatusProcessor, findRecentByDRepIdAndEpoch). Replaying a deregistration can pick up a later re-registration in the same epoch.
  • The RETIRED row written at the epoch boundary copies the retirement certificate's tx index. A re-registration in the first block of that epoch can then sort before it. This predates this PR.

🤖 Generated with Claude Code

When a block is processed again after a restart, the row it wrote the first
time is still in the pool table. PoolStatusProcessor looked up the latest
REGISTRATION of the pool up to the current epoch, found that row, and saved
the registration again as an UPDATE (amount 0, active epoch +3). The pool's
next certificate then became the REGISTRATION, so adapot counted the 500 ADA
deposit in the wrong epoch.

Look only at registrations that come before the current certificate
(slot, tx index, cert index), so a replay gets the same answer as the first
pass. Found by the fault-injection re-test (#1328).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown

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: 4da900f7b04dc18e044d09ce42ae109e4fdb8d57

No issues found in this first check.

Not tested: I did not run the build, tests, or a live chain replay.

satran004 and others added 3 commits November 8, 2026 19:06
…itEvent

PoolStatusProcessor caches pool registration events until pre-commit and
publishes the number of REGISTRATION rows in StakingDepositEvent. When a
batch failed before pre-commit, the cache kept its events. A block processed
again then sat in the cache twice. Now that a replayed registration stays a
registration, both copies were counted: 2 in the event, 1 row in the database.

- A block processed again replaces its earlier cached event (same slot and
  block hash), so each certificate is counted once.
- A rollback clears all four caches. A restart publishes a rollback to the
  cursor before it processes blocks again, so events of the failed batch,
  including blocks the node no longer serves, are not counted or saved at the
  next pre-commit. This also resets the stake key registration counts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
…PoolRegistrationBefore

PoolStorage can be replaced by a custom bean. A provider compiled against the
previous interface does not have the new method and failed with
AbstractMethodError at the first pool registration. The method now has a
default that throws UnsupportedOperationException, naming the class and the
method to implement. It does not fall back to findRecentPoolRegistration,
because that query sees the certificate's own row and would turn a replayed
registration into an update again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
…er block

PoolRegistrationProcessor publishes one PoolRegistrationEvent per transaction. Replacing a cached event by
block dropped the other transactions of the same block, so their pool status rows were not written and the
registration count was too low. Replace only the copy of the same transaction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJrQUCWhJpLCQR5Asv7nDb
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

@satran004

Copy link
Copy Markdown
Member Author

Updated after an external review of the 2.0.x backport #1347: 15f7a3d, 132f4fb, 1e85144. Each finding, its verdict and its test are in the reply on #1339.

@satran004

Copy link
Copy Markdown
Member Author

Verdict: APPROVED

reviewer1 — original-PR review at 1e851441600c92bdc5c3afc52e8e824cac550d9e.

F1 and F2 from #1347 are resolved. The changed files are identical to the approved backport: transaction-level cache replacement prevents duplicate counts, rollback clears pending caches, and custom PoolStorage providers get an explicit implementation requirement instead of an incorrect fallback.

The numbered findings refer to the counterpart review of #1347. 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
satran004 requested a review from huylevt November 9, 2026 00:00
@satran004

Copy link
Copy Markdown
Member Author

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:

  • pool, pool_registration, pool_retirement, delegation and stake_registration match the reference exactly after 104 scenarios of restarts, rollbacks and replays.
  • The previous combined run, before this fix, differed in pool at 3 slots.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.0.0-beta6 Planned for 3.0.0-beta6 fault-injection Found or verified by the auto-recovery fault-injection test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants