Skip to content

go/control: Show local checkpoint heights#6265

Open
martintomazic wants to merge 2 commits intomasterfrom
martin/feature/show-checkpoints
Open

go/control: Show local checkpoint heights#6265
martintomazic wants to merge 2 commits intomasterfrom
martin/feature/show-checkpoints

Conversation

@martintomazic
Copy link
Copy Markdown
Contributor

@martintomazic martintomazic commented Jul 15, 2025

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 15, 2025

Deploy Preview for oasisprotocol-oasis-core canceled.

Name Link
🔨 Latest commit 66596ab
🔍 Latest deploy log https://app.netlify.com/projects/oasisprotocol-oasis-core/deploys/68acd47718a6c500078a0041

@martintomazic martintomazic force-pushed the martin/feature/show-checkpoints branch from 627efc8 to 747cdd3 Compare July 15, 2025 08:28
Comment thread go/consensus/cometbft/full/common.go Outdated
@martintomazic martintomazic force-pushed the martin/feature/show-checkpoints branch from 747cdd3 to 0587cae Compare July 15, 2025 08:41
@martintomazic martintomazic force-pushed the martin/feature/show-checkpoints branch from 0587cae to 1bcd02f Compare July 28, 2025 23:40
@martintomazic martintomazic marked this pull request as ready for review July 30, 2025 21:14
@martintomazic martintomazic changed the title go/control: Show local checkpoints height and block hash go/control: Show local checkpoint heights Jul 30, 2025
Comment thread go/consensus/api/api.go Outdated
Comment thread go/consensus/cometbft/full/common.go Outdated
Comment thread go/consensus/cometbft/full/common.go Outdated
@martintomazic martintomazic force-pushed the martin/feature/show-checkpoints branch 2 times, most recently from 970e3aa to 2222a19 Compare July 31, 2025 15:37
Comment thread go/consensus/cometbft/full/common.go Outdated
Comment thread go/consensus/cometbft/full/common.go Outdated
Comment on lines +844 to +845

status.Checkpoint = n.fetchCheckpointStatus(ctx)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you previously had checkpointer enabled (and you have some checkpoints), then you restart a node and disable checkpoints, you would still show old checkpoints. Could this confuse node operators?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could also show information on whether the checkpointer is currently enabled or not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You could also show information on whether the checkpointer is currently enabled or not.

You mean to only show it if the checkpointer is enabled? I think this would be good.

Alternative is to show the status of the checkpointer directly:
consensus.checkpointer.enabled/heights or possibly consensus.state.checkpointer.enabled/heights?

I notice for the consensus we tend to display "domain view" (current solution), whereas for the runtimes we tend to show status of independent components (alternative solution).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have kept is simple and only show it if checkpointer is enabled.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 4, 2025

Codecov Report

❌ Patch coverage is 52.38095% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.36%. Comparing base (2a01cbe) to head (66596ab).
⚠️ Report is 287 commits behind head on master.

Files with missing lines Patch % Lines
go/consensus/cometbft/full/common.go 52.38% 8 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6265       +/-   ##
===========================================
+ Coverage        0   64.36%   +64.36%     
===========================================
  Files           0      697      +697     
  Lines           0    67854    +67854     
===========================================
+ Hits            0    43674    +43674     
- Misses          0    19171    +19171     
- Partials        0     5009     +5009     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@martintomazic martintomazic force-pushed the martin/feature/show-checkpoints branch from 1bc6907 to e4e7863 Compare August 25, 2025 21:20
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.

control status: Show checkpoint height(s)

3 participants