cardano-testnet | Extract EpochStateView into its own module#6547
Open
cardano-testnet | Extract EpochStateView into its own module#6547
Conversation
Move EpochStateView, its background-thread setup, the STM-based wait primitive, and the retry loops out of Testnet.Components.Query and into a new module Testnet.Components.EpochStateView. The new module carries a top-down haddock that explains how the writer thread, the version counter, and awaitStateUpdateTimeout cooperate so that future readers can learn the mechanism from one place rather than piecing it together across Query.hs. Testnet.Components.Query re-exports the public API so existing callers do not need to change their imports.
carbolymer
force-pushed
the
mgalazyn/refactor/remove-epoch-state-polling
branch
3 times, most recently
from
5cbd71a to
a205cac
Compare
Base automatically changed from
mgalazyn/refactor/remove-epoch-state-polling
to
master
April 24, 2026 09:33
carbolymer
reviewed
Apr 24, 2026
Comment on lines
+38
to
+53
| 1. Samples the current version /before/ running the action, so that updates | ||
| landing during the action are not missed. | ||
| 2. Runs the action. On 'Right' it returns immediately. | ||
| 3. On 'Left' it checks whether the chain-unit deadline has been exceeded; if so | ||
| it returns the last 'Left'. | ||
| 4. Otherwise it blocks on STM until the version advances past the sampled | ||
| value, then loops. | ||
|
|
||
| The STM block is performed by an internal helper that combines a fast path | ||
| (return immediately if the version already differs — common when the action | ||
| took long enough that a block landed during it) with an awaited path (register | ||
| a fallback timer and block on STM until either the version advances or the | ||
| timer fires). The fallback is a stall-detection heartbeat, not a wait duration | ||
| — its only job is to guarantee no single STM transaction blocks indefinitely. | ||
| What actually terminates the loop is either a successful action or the | ||
| chain-unit deadline. |
Contributor
There was a problem hiding this comment.
This is too long - no one will read that. This basically turns the implementation into the prose and risks getting out of sync with the implementation. I suggest removing this.
carbolymer
reviewed
Apr 24, 2026
Comment on lines
+232
to
+249
| -- | Status of the 'EpochStateView' background thread when epoch state is not yet available | ||
| data EpochStateStatus | ||
| = EpochStateNotInitialised | ||
| -- ^ The background thread has not yet received any epoch state from the node | ||
| | EpochStateFoldError !FoldBlocksError | ||
| -- ^ The background thread encountered an error while folding blocks | ||
|
|
||
| -- | A read-only handle to an epoch state that is kept fresh by a background thread. | ||
| -- | ||
| -- The constructor is private. Reads go through the accessor functions exported from | ||
| -- this module ('getEpochState', 'getBlockNumber', 'getSlotNumber', | ||
| -- 'getEpochStateDetails', 'getCurrentEpochNo') so that callers cannot accidentally | ||
| -- race against the version-counter synchronisation contract described in the module | ||
| -- header. | ||
| data EpochStateView = EpochStateView | ||
| { epochStateView :: !(TVar (Either EpochStateStatus (AnyNewEpochState, SlotNo, BlockNo))) | ||
| , epochStateVersion :: !(TVar Word64) | ||
| } |
Contributor
There was a problem hiding this comment.
Those two datatypes should go to the top of the module.
carbolymer
reviewed
Apr 24, 2026
Comment on lines
+16
to
+30
| 'EpochStateView' holds two 'TVar's (private to this module): | ||
|
|
||
| * The current @(AnyNewEpochState, SlotNo, BlockNo)@, or a status value while the | ||
| view is initialising or after the background thread has errored. | ||
| * A monotonically-increasing @Word64@ version counter, bumped on every write to | ||
| the state 'TVar'. This counter is the synchronisation channel between the | ||
| writer thread and any waiter: a caller records the current version, performs | ||
| its check, and then blocks on STM until the version differs. | ||
|
|
||
| The writer is set up by 'getEpochStateView'. It launches a long-lived fold (via | ||
| 'asyncRegister_') that runs the chain-sync client against the node. For every | ||
| block the node streams, it writes the derived @NewEpochState@ into the state | ||
| 'TVar' and bumps the version. If the fold terminates with an error, it writes | ||
| the error status and bumps the version so any waiting threads can observe the | ||
| failure. |
Contributor
There was a problem hiding this comment.
The implementation details like TVar or asyncRegister_ are irrelevant here. They just add to the volume without providing useful info. Can you rewrite this focusing on behaviour instead?
carbolymer
reviewed
Apr 24, 2026
| void . retryUntilRightM epochStateView (WaitForBlocks numberOfBlocks) . pure $ Left () | ||
| getBlockNumber epochStateView | ||
|
|
||
| -- | Deadline for 'retryUntilRightM' and its wrappers, expressed in chain units |
Contributor
There was a problem hiding this comment.
for 'retryUntilRightM' and its wrappers,
the purpose is irrelevant here. What this type represents is what matters
carbolymer
reviewed
Apr 24, 2026
Comment on lines
+137
to
+139
| -- rather than wall-clock time. Each iteration of the loop only advances when the | ||
| -- chain advances, so the deadline measures how much chain progress the caller is | ||
| -- willing to wait for. |
Contributor
There was a problem hiding this comment.
Each iteration of the loop only advances when the
-- chain advances, so the deadline measures how much chain progress the caller is
-- willing to wait for.
redundant
carbolymer
reviewed
Apr 24, 2026
Comment on lines
+251
to
+252
| -- | Block until the epoch state version advances past the provided previously sampled | ||
| -- version, or until the fallback timeout expires. Returns immediately if the current |
Contributor
There was a problem hiding this comment.
Version concept is leaking into the doc. Mention epoch state update instead.
carbolymer
requested changes
Apr 24, 2026
Contributor
carbolymer
left a comment
carbolymer
left a comment
There was a problem hiding this comment.
Good work. Comments need more work. At the moment they're too lengthy, leak implementation details and mix them with the behaviour. Can you simplify them?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #6538. Moves
EpochStateView, its background fold setup, the STM-based wait primitive (awaitStateUpdateTimeout), and the retry loops (retryUntilRightM/retryUntilJustM/retryUntilM/waitForEpochs/waitForBlocks) out ofTestnet.Components.Queryand into a new moduleTestnet.Components.EpochStateView.The new module carries a top-down haddock overview explaining how the writer thread, the monotonic version counter, and the wait primitive cooperate — so future readers can learn the mechanism from one place instead of piecing it together across
Query.hs.Testnet.Components.Queryre-exports the public API, so existing callers need no import changes.Motivation
Testnet.Components.Queryhad two intertwined concerns:EpochStateViewsynchronisation primitive — background thread, sharedTVars, version counter, STM wait/retry logic.findAllUtxos,getGovState,checkDRepState,assertNewEpochState, etc.).The first concern is the subtle, dangerous part. Isolating it into its own module means the synchronisation contract (sampled version must match fresh state in the same STM transaction) lives in one place, with its own module-level documentation — and nothing outside the module can reach into the raw
TVars by mistake.What stayed in
Query.hsAll the state-consumer helpers (
findAllUtxos, UTXO helpers,getGovState,getTreasuryValue,checkDRepsNumber,checkDRepState,assertNewEpochState, protocol-parameter getters,getTxIx,waitUntilEpoch).Test plan
cabal build cardano-testnet:lib:cardano-testnetbuilds clean