refactor(config/node)!: Remove genesisHash from config #3595
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cason
reviewed
Jul 31, 2024
.changelog/unreleased/breaking-changes/3595-node-remove-genesishash.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Daniel <daniel.cason@informal.systems>
This is a good point and you are right. But it is probably overkill to do it for this change. We could however introduce a changelog entry under features so that it is clear this is there? |
Actually no, this is not a feature visible to the users, it is internal for us as developers or people who fork the repo perhaps so I not sure it falls under the classic feature category. |
cason
approved these changes
Jul 31, 2024
mergify bot
pushed a commit
that referenced
this pull request
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
melekes
added a commit
that referenced
this pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3547 .
This PR removes the
GenesisHashfrom 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 ofNewNodeas this is probably used by the community more and introduceNewNodeWithCliParams.NewNodeWithCliParamsallows 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 theGenesisHashbut it was done this way so that we can in the future extend it with new parameters without breaking APIs.