cardano-testnet | Refactor EpochStateView to use TVar instead of IORef by carbolymer · Pull Request #6538 · IntersectMBO/cardano-node

carbolymer · 2026-04-16T15:28:49Z

Checklist

Commit sequence broadly makes sense and commits have useful messages
New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- roundtrip tests
- integration tests
  See Running tests for more details
Any changes are noted in the CHANGELOG.md for affected package
The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-9.6 and ghc-9.12
Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

palas

Nice, we don't have to worry about the polling interval anymore 👍

palas · 2026-04-16T22:43:12Z

+    state <- readTVar epochStateView
+    case state of
+      Right _ -> pure state
+      Left (EpochStateFoldError _) -> pure state
+      Left EpochStateNotInitialised -> do
+        timedOut <- readTVar timedOutVar
+        guard timedOut
+        pure state


Suggested change

state <- readTVar epochStateView

case state of

Right _ -> pure state

Left (EpochStateFoldError _) -> pure state

Left EpochStateNotInitialised -> do

timedOut <- readTVar timedOutVar

guard timedOut

pure state

state <- readTVar epochStateView

timedOut <- readTVar timedOutVar

guard (state /= Left LeftEpochStateNotInitialised || timedOut)

pure state

I am just showing off

nice! We'd need some new Eq instances for that, but we'd have to write them manually for some ledger types.

Copilot

Pull request overview

Refactors cardano-testnet’s EpochStateView to use STM (TVar) so testnet wait/retry helpers can block on state updates instead of polling, improving responsiveness and reducing unnecessary wakeups.

Changes:

Replaced EpochStateView’s IORef storage with TVar and introduced a monotonic version counter for update notification.
Updated retry logic to await epoch-state updates (STM) rather than threadDelay polling.
Added changelog fragments documenting the refactor; normalized JSON fixture formatting (newline at EOF).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`cardano-testnet/src/Testnet/Components/Query.hs`	Switches epoch state storage to STM and adds STM-based update waiting used by retry loops.
`cardano-testnet/test/cardano-testnet-test/files/calculatePlutusScriptCost.json`	Formatting-only change (newline/EOF normalization).
`cardano-testnet/changelog.d/20260416_160000_mgalazyn_stm_epoch_state_view.md`	Documents the STM refactor and change-notification approach.
`cardano-testnet/changelog.d/20260416_124922_mgalazyn_refactor_retry_functions.md`	Updates changelog wording to match the refactor history.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

carbolymer · 2026-04-21T08:34:24Z

+awaitStateUpdateTimeout timeout EpochStateView{epochStateVersion, epochStateView} = liftIO $ do
+  version <- readTVarIO epochStateVersion
+  timedOutVar <- registerDelay . round $ timeout * 1_000_000
+  atomically $ do
+    v <- readTVar epochStateVersion
+    timedOut <- readTVar timedOutVar
+    case (v /= version, timedOut) of
+      (True, _) -> Just <$> readTVar epochStateView
+      (_, True) -> pure Nothing
+      _         -> STM.retry


Thanks. Flagging this as investigated but not acted on, with reasoning:

Mechanism: registerDelay doesn't spawn a thread per call.
It registers an entry in GHC's IO-manager timer queue (one long-lived manager per capability), so the accumulating cost is a priority-queue entry plus a TVar Bool per pending timeout, not a forked thread.

Scale: in steady-state retry loops (awaitStateUpdate fires roughly once per epoch; fallback is 300s), we'd have tens of pending entries at any time, low KB of memory.
Negligible at testnet-test scale.

Load-bearing: the 300s fallback here isn't redundant with retryUntilRightM's outer deadline.
That outer deadline is computed from getCurrentEpochNo/getSlotNumber/getBlockNumber, which read the same EpochStateView TVar that the fallback guards.
If the background fold thread hangs, the outer deadline stops advancing and the retry loop would block forever without this inner timeout.

Cancellable alternative: wrapping atomically in System.Timeout.timeout would cancel the timer on early return, but adds forkIO/killThread per call.
Trading one small overhead for another without a clear win.

Addressed the fast-path timer in getEpochStateDetails (line 323 comment) since that one accumulates without bound.

Copilot

Pull request overview

Refactors EpochStateView in cardano-testnet to use STM (TVar) and introduces an STM-based change-notification mechanism so retry loops can block on state updates rather than polling.

Changes:

Replaced IORef-backed EpochStateView with TVar plus a monotonic version counter, and updated the background fold thread to write via STM.
Updated retry logic (retryUntilRightM) and epoch-state getters to block via STM (awaitStateUpdate) instead of threadDelay polling.
Added changelog entries and normalized a JSON test fixture (likely newline/formatting).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File	Description
cardano-testnet/src/Testnet/Components/Query.hs	Core refactor: `EpochStateView` now uses `TVar` + version counter; retry loops block on STM updates.
cardano-testnet/changelog.d/20260416_160000_mgalazyn_stm_epoch_state_view.md	Documents the STM-based `EpochStateView` refactor and version counter rationale.
cardano-testnet/changelog.d/20260416_124922_mgalazyn_refactor_retry_functions.md	Adjusts changelog wording to reflect the new `EpochStateView` shape.
cardano-testnet/test/cardano-testnet-test/files/calculatePlutusScriptCost.json	Formatting-only tweak (appears to fix final newline/line accounting).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR refactors cardano-testnet’s EpochStateView to use STM (TVar) instead of IORef, enabling retry loops to block on state changes rather than polling, and updates documentation/changelog artifacts accordingly.

Changes:

Replaced IORef-backed EpochStateView with TVar plus a monotonic version counter for STM-based change notification.
Updated retryUntilRightM to wait via STM (awaitStateUpdateTimeout) instead of threadDelay polling.
Updated changelog entries and a JSON test fixture formatting/newline.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File	Description
cardano-testnet/src/Testnet/Components/Query.hs	Refactors `EpochStateView` to `TVar`, adds versioning + STM wait helpers, and updates retry behavior to block on state updates.
cardano-testnet/test/cardano-testnet-test/files/calculatePlutusScriptCost.json	Minor formatting/newline adjustment in a test fixture.
cardano-testnet/changelog.d/20260416_160000_mgalazyn_stm_epoch_state_view.md	Documents the STM-based `EpochStateView` refactor and new notification mechanism.
cardano-testnet/changelog.d/20260416_124922_mgalazyn_refactor_retry_functions.md	Updates a prior changelog entry to reflect the finalized `EpochStateView` field removal wording.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR refactors cardano-testnet’s EpochStateView to use STM primitives (TVar) rather than IORef, enabling retry/wait loops to block on state changes instead of polling with threadDelay.

Changes:

Replaced EpochStateView’s underlying mutable storage from IORef to TVar, and added a monotonic epochStateVersion TVar.
Updated retryUntilRightM to await STM-based state updates (with a fallback timeout) instead of polling.
Added/updated changelog entries documenting the maintenance refactor.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
cardano-testnet/src/Testnet/Components/Query.hs	Migrates epoch-state tracking to STM (`TVar`) with a version counter and STM-based wakeups in retry loops.
cardano-testnet/changelog.d/20260416_160000_mgalazyn_stm_epoch_state_view.md	Documents the STM refactor and the new change-notification mechanism.
cardano-testnet/changelog.d/20260416_124922_mgalazyn_refactor_retry_functions.md	Updates prior changelog entry to reflect removal of unused `EpochStateView` fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

palas

Looks good 👍

Jimbo4350

LGTM, just two comments to address. I am opening a PR to document this a bit more as the complexity increases with using STM.

Jimbo4350 · 2026-04-23T14:57:46Z

  -> AnyNewEpochState
  -> Maybe EpochNo
 maybeExtractGovernanceActionExpiry txid (AnyNewEpochState sbe newEpochState _) =
  caseShelleyToBabbageOrConwayEraOnwards


Can we remove the use of caseShelleyToBabbageOrConwayEraOnwards?

will make a follow-up PR

carbolymer requested a review from a team as a code owner April 16, 2026 15:28

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch 3 times, most recently from 1d2609d to 0b7d0eb Compare April 16, 2026 19:18

palas approved these changes Apr 16, 2026

View reviewed changes

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch from 0b7d0eb to f872c06 Compare April 17, 2026 08:06

carbolymer requested a review from Copilot April 17, 2026 08:06

Copilot started reviewing on behalf of carbolymer April 17, 2026 08:06 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch from f872c06 to e839e00 Compare April 17, 2026 08:17

carbolymer requested review from a team as code owners April 17, 2026 08:17

carbolymer force-pushed the mgalazyn/refactor/remove-watch-epoch-state-update branch from 6bd49c2 to 82f0f20 Compare April 17, 2026 10:44

carbolymer self-assigned this Apr 20, 2026

carbolymer force-pushed the mgalazyn/refactor/remove-watch-epoch-state-update branch from 82f0f20 to 75aece7 Compare April 21, 2026 07:47

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch 2 times, most recently from d9ddb0e to 2a7327a Compare April 21, 2026 09:27

carbolymer requested review from Copilot and palas April 21, 2026 09:27

Copilot started reviewing on behalf of carbolymer April 21, 2026 09:30 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread cardano-testnet/src/Testnet/Components/Query.hs

Comment thread cardano-testnet/src/Testnet/Components/Query.hs Outdated

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch 2 times, most recently from cdf2031 to 35413fc Compare April 21, 2026 13:42

carbolymer requested a review from Copilot April 21, 2026 13:43

Copilot started reviewing on behalf of carbolymer April 21, 2026 13:43 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread cardano-testnet/src/Testnet/Components/Query.hs

Comment thread cardano-testnet/src/Testnet/Components/Query.hs Outdated

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch from 35413fc to 7db627e Compare April 21, 2026 16:25

carbolymer requested a review from Copilot April 21, 2026 16:26

Copilot started reviewing on behalf of carbolymer April 21, 2026 16:27 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread cardano-testnet/src/Testnet/Components/Query.hs

Comment thread cardano-testnet/src/Testnet/Components/Query.hs

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch 3 times, most recently from 010681d to 4499504 Compare April 21, 2026 16:51

palas approved these changes Apr 21, 2026

View reviewed changes

carbolymer force-pushed the mgalazyn/refactor/remove-watch-epoch-state-update branch from 75aece7 to 47aa730 Compare April 22, 2026 12:06

Jimbo4350 approved these changes Apr 23, 2026

View reviewed changes

This was referenced Apr 23, 2026

cardano-testnet | Remove redundant watchEpochStateUpdate #6535

Merged

cardano-testnet | Extract EpochStateView into its own module #6547

Open

carbolymer force-pushed the mgalazyn/refactor/remove-watch-epoch-state-update branch from 47aa730 to 344c313 Compare April 24, 2026 06:57

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch 2 times, most recently from 7bcedd3 to 5cbd71a Compare April 24, 2026 07:15

cardano-testnet | Refactor EpochStateView to use TVar instead of IORef

a205cac

carbolymer force-pushed the mgalazyn/refactor/remove-epoch-state-polling branch from 5cbd71a to a205cac Compare April 24, 2026 07:52

Base automatically changed from mgalazyn/refactor/remove-watch-epoch-state-update to master April 24, 2026 08:16

carbolymer merged commit 50ffe15 into master Apr 24, 2026
29 of 31 checks passed

carbolymer deleted the mgalazyn/refactor/remove-epoch-state-polling branch April 24, 2026 09:33

Conversation

carbolymer commented Apr 16, 2026

Checklist

Note on CI

Uh oh!

palas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

palas Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carbolymer Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

carbolymer Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

palas left a comment

Choose a reason for hiding this comment

Uh oh!

Jimbo4350 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jimbo4350 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

carbolymer Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

carbolymer Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

palas Apr 16, 2026 •

edited

Loading

carbolymer Apr 17, 2026 •

edited

Loading