Skip to content

Replace getLocalChainTip with in-session queries in runQueryKesPeriodInfoCmd - #1435

Closed
Jimbo4350 wants to merge 2 commits into
masterfrom
fix/kes-period-info-hang
Closed

Replace getLocalChainTip with in-session queries in runQueryKesPeriodInfoCmd#1435
Jimbo4350 wants to merge 2 commits into
masterfrom
fix/kes-period-info-hang

Conversation

@Jimbo4350

@Jimbo4350 Jimbo4350 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

runQueryKesPeriodInfoCmd called getLocalChainTip to get the chain tip, which opens a fresh connection to the node. This replaces it with queryChainBlockNo/queryChainPoint, which retrieve the same information through the already-open state query session — one fewer connection per invocation.

Changelog

- description: |
    Replace getLocalChainTip with queryChainBlockNo/queryChainPoint in runQueryKesPeriodInfoCmd to avoid opening a redundant node connection
  type:
    - optimisation

Copilot AI lite review requested due to automatic review settings September 1, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression where cardano-cli ... query kes-period-info can hang indefinitely by avoiding a second node connection (chain-sync) while a local-state-query mux connection is still active, and instead deriving the ChainTip from local-state queries.

Changes:

  • Replace getLocalChainTip usage in runQueryKesPeriodInfoCmd with queryChainBlockNo/queryChainPoint and makeChainTip.
  • Add a changelog fragment documenting the bugfix.

Reviewed changes

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

File Description
cardano-cli/src/Cardano/CLI/EraBased/Query/Run.hs Switch KES-period-info tip acquisition to local-state queries to prevent the mux deadlock.
.changes/20260901_fix_kes_period_info_hang.yml Add release note entry for the hang fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +384 to +386
mChainBlockNo <- lift queryChainBlockNo & onLeft (left . QueryCmdUnsupportedNtcVersion)
mChainPoint <- lift queryChainPoint & onLeft (left . QueryCmdUnsupportedNtcVersion)
let chainTip = makeChainTip mChainBlockNo mChainPoint
Comment on lines +1 to +2
project: cardano-cli
pr: 0
Comment on lines +384 to +386
mChainBlockNo <- lift queryChainBlockNo & onLeft (left . QueryCmdUnsupportedNtcVersion)
mChainPoint <- lift queryChainPoint & onLeft (left . QueryCmdUnsupportedNtcVersion)
let chainTip = makeChainTip mChainBlockNo mChainPoint
getLocalChainTip was called inside executeLocalStateQueryExpr, opening
a second node connection while the state query mux was still active.
The state query thread could not fill its completionVar (and thus close
connection 1) because it was blocked waiting for the second connection
to return. Deadlock.

Fix: use queryChainBlockNo/queryChainPoint within the existing session
and reconstruct ChainTip via makeChainTip — the same pattern used by
runQueryTipCmd.

Fixes: #1434
@Jimbo4350
Jimbo4350 force-pushed the fix/kes-period-info-hang branch from 53511a5 to 7b12808 Compare September 1, 2026 15:42
@Jimbo4350 Jimbo4350 changed the title Fix query kes-period-info hanging indefinitely (fixes #1434) Remove redundant network id validation query (optimisation) Sep 1, 2026
@Jimbo4350 Jimbo4350 changed the title Remove redundant network id validation query (optimisation) Remove redundant network id validation query Sep 1, 2026
@Jimbo4350 Jimbo4350 changed the title Remove redundant network id validation query Replace getLocalChainTip with in-session queries in runQueryKesPeriodInfoCmd Sep 1, 2026
@Jimbo4350 Jimbo4350 closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants