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

[Enhancement] Introduce 'head' parameter for snapshot validation. #3107

Merged
merged 3 commits into from
Jul 3, 2023

Conversation

ruseinov
Copy link
Contributor

@ruseinov ruseinov commented Jul 3, 2023

Summary of changes

Changes introduced in this pull request:

  • Introduce HEAD param to make sure we can specify ranges when importing and validating a snapshot.

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

@ruseinov
Copy link
Contributor Author

ruseinov commented Jul 3, 2023

A follow-up issue: sort out snapshot validation parameters interface as it's quite confusing.

@lemmih
Copy link
Contributor

lemmih commented Jul 3, 2023

Using i64 (or ChainEpoch) might be easier.

@ruseinov
Copy link
Contributor Author

ruseinov commented Jul 3, 2023

Using i64 (or ChainEpoch) might be easier.
I agree that it's not the most eloquent of solutions.

It makes no sense to accept i64 though as we don't want to allow negative values for this one. Hence the u64.
In an ideal world we should have validation for this and all the other CLI params. Perhaps a good issue to create for when we'd want to re-vamp and move those out of the daemon?

@ruseinov ruseinov enabled auto-merge (squash) July 3, 2023 15:36
@ruseinov ruseinov merged commit 21e8a4b into main Jul 3, 2023
19 checks passed
@ruseinov ruseinov deleted the ru/feature/head-param branch July 3, 2023 15:41
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.

None yet

4 participants