-
Notifications
You must be signed in to change notification settings - Fork 636
refactor(config): Remove genesis hash from config #3547
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
Labels
Milestone
Comments
I would be best if this could make it into |
mergify bot
pushed a commit
that referenced
this issue
Jul 31, 2024
Closes #3547 . This PR removes the `GenesisHash` from the config file because this field is not meant to be updated by the operator. It was only introduced to avoid breaking an API and used to pass the genesis hash provided via cli to the node constructor. As this feature is going into an unreleased version, we decided to break the API of `DefaultNewNode`. We maintain the signature of `NewNode` as this is probably used by the community more and introduce `NewNodeWithCliParams`. `NewNodeWithCliParams` allows for passing different parameters from the cli to the node. The PR also introduces a new type `CliParams`. At the moment this struct contains just the `GenesisHash` but it was done this way so that we can in the future extend it with new parameters without breaking APIs. --------- Co-authored-by: Daniel <daniel.cason@informal.systems> (cherry picked from commit 11c5b25) # Conflicts: # node/node.go # node/node_test.go
alesforz
pushed a commit
that referenced
this issue
Jul 31, 2024
Closes #3547 . This PR removes the `GenesisHash` from the config file because this field is not meant to be updated by the operator. It was only introduced to avoid breaking an API and used to pass the genesis hash provided via cli to the node constructor. As this feature is going into an unreleased version, we decided to break the API of `DefaultNewNode`. We maintain the signature of `NewNode` as this is probably used by the community more and introduce `NewNodeWithCliParams`. `NewNodeWithCliParams` allows for passing different parameters from the cli to the node. The PR also introduces a new type `CliParams`. At the moment this struct contains just the `GenesisHash` but it was done this way so that we can in the future extend it with new parameters without breaking APIs. --------- Co-authored-by: Daniel <daniel.cason@informal.systems>
melekes
added a commit
that referenced
this issue
Aug 1, 2024
… (#3600) Closes #3547 . This PR removes the `GenesisHash` from the config file because this field is not meant to be updated by the operator. It was only introduced to avoid breaking an API and used to pass the genesis hash provided via cli to the node constructor. As this feature is going into an unreleased version, we decided to break the API of `DefaultNewNode`. We maintain the signature of `NewNode` as this is probably used by the community more and introduce `NewNodeWithCliParams`. `NewNodeWithCliParams` allows for passing different parameters from the cli to the node. The PR also introduces a new type `CliParams`. At the moment this struct contains just the `GenesisHash` but it was done this way so that we can in the future extend it with new parameters without breaking APIs. <hr>This is an automatic backport of pull request #3595 done by [Mergify](https://mergify.com). --------- Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Genesis hash saved in config file but read-only
#1324 saves the genesis hash in the config file to check it against the has from the DB. However, its sole purpose is to forward this config flag to be checked on node boot-up against the hash of the genesis file if it is the first boot or the hash stored in the DB if the node has already read in a genesis file before. The user is NOT supposed to put the hash into the config file manually. We decided at the time to not break any APIs and apply this approach.
Upon further discussion, we chose this to be a suboptimal way of doing things and bad UX. The flag should be passed as an argument, even if APIs are being broken.
Once a PR is open, we should however check whether we are causing major breaking changes for some users and let them know that this update is coming.
The text was updated successfully, but these errors were encountered: