Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Max Block Height Service #113

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

boray
Copy link
Member

@boray boray commented Jan 9, 2025

This PR introduces a new feature to fetch the maximum block height information from an archive node. This information can then be used to check the synchronization of archive nodes with Mina nodes.

The query below returns canonicalMaxBlockHeight and pendingMaxBlockHeight:

query maxBlockHeightInfo {
  block {
    canonicalMaxBlockHeight
    pendingMaxBlockHeight
    }
}

closes #111

@boray boray marked this pull request as ready for review January 10, 2025 09:59
@boray boray requested review from 45930 and deepthiskumar January 10, 2025 10:08
Copy link
Contributor

@45930 45930 left a comment

Choose a reason for hiding this comment

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

Implementation looks good, but I'd like to see more testing, and maybe update the name of the query to allow for expansion in the future.

@@ -78,4 +83,5 @@ type ActionOutput {
type Query {
events(input: EventFilterOptionsInput!): [EventOutput]!
actions(input: ActionFilterOptionsInput!): [ActionOutput]!
block: MaxBlockHeightInfo!
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would make more sense in the long run for this query to be called blockchainMetadata or something, so there is scope to add additional data in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe networkState?

});
});

test("Fetching the max block height should return the max block height", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to add more serious testing to this. For example, if we progress the state of the blockchain, does the block height advance? Does the pending height stay higher than the canonical height?

Copy link
Contributor

Choose a reason for hiding this comment

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

You could also use the mina grapqhQL endpoint as a source of truth to compare and make sure we are actually returning the max block height.

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.

[feature request] latest known blockHeight
2 participants