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

Require either a root snapshot file, or Dynamic Startup flags #5938

Merged

Conversation

jordanschalm
Copy link
Member

@jordanschalm jordanschalm commented May 16, 2024

The current Dynamic Startup behaviour is based on precedence:

  • if the state is bootstrapped, we ignore the root snapshot file and Dynamic Startup flags
  • else if a root snapshot file is provided, we use that and ignore Dynamic Startup flags
  • else we use Dynamic Startup flags

Since Dynamic Startup is now more frequently used by existing operators, who already have a root snapshot file (from the spork), this arrangement leads to operators attempting to use Dynamic Startup, but inadvertently bootstrapping from an old root snapshot file instead.

This PR:

  • Requires that strictly one of either a root snapshot file or Dynamic Startup flags are provided. If both are provided, we will exit and force the operator to indicate which bootstapping path they would like to use.
  • Logs the root block ID and height after a successful Dynamic Startup
  • Removes unused snapshot utilities (since merging the Protocol KVStore branch, a snapshot for any block can be used as a root snapshot).

- fail if both a snapshot file and dynamic startup flags are provided
- log the root height and ID once dynamic startup completes
@codecov-commenter
Copy link

codecov-commenter commented May 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 55.60%. Comparing base (c4d25c7) to head (a0e67e7).
Report is 12 commits behind head on master.

Files Patch % Lines
cmd/dynamic_startup.go 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5938      +/-   ##
==========================================
- Coverage   55.61%   55.60%   -0.02%     
==========================================
  Files        1128     1128              
  Lines       88928    88947      +19     
==========================================
- Hits        49458    49455       -3     
- Misses      34758    34778      +20     
- Partials     4712     4714       +2     
Flag Coverage Δ
unittests 55.60% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@jordanschalm jordanschalm added this pull request to the merge queue May 20, 2024
Merged via the queue into master with commit 55cf969 May 20, 2024
55 checks passed
@jordanschalm jordanschalm deleted the jord/dynamic-bootstrap-error-if-snapshot-file-exists branch May 20, 2024 11:44
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.

4 participants