-
Notifications
You must be signed in to change notification settings - Fork 550
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
Lucy genesis ledger and keys refactor #12391
Merged
Merged
Conversation
This file contains 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
081b821
to
6271167
Compare
…nd not lists, also genesis keys are in maps
57e4e03
to
b257b66
Compare
QuiteStochastic
commented
Feb 24, 2023
@@ -0,0 +1,150 @@ | |||
open Core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this entire test is just a stub/placeholder for now. it works fine, it's just a bit redundant with payments_test. this test will be completed the next PR
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
lk86
approved these changes
Mar 3, 2023
!ci-build-me |
1 similar comment
!ci-build-me |
!ci-build-me |
psteckler
approved these changes
Mar 9, 2023
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.
This refactor makes the management of the genesis ledger and initial account keys to be more explicit within a Lucy test, thereby giving the test writer more control. I think this new format is more intuitive and less conflating of distinct concepts for those who are familiar with how the blockchain works.
*Short explanation: *
A generic test configuration would look something like this under the old config format:
the same hypothetical test config will now look like this:
A further important change is that before, if you had multiple block producers, you could access these block producers from a list, with each node not having any specific name, merely a position within the list. Now, all nodes as well as keys are stored in maps, and can be fetched out of the map by the name given to them in the config, which of course is the key to the map.
Infrastructure wise, there are a number of changes to the o1-integration terraform module, but this module effects only lucy.
Of greater importance is that in the course of making this PR work, I ended up changing a number of the variables in kubernetes/testnet, which is a module also used by o1-testnet, which if I'm mistaken is still the main entry level TF module that we use to make testnet deployments. Therefore, changes to kubernetes/testnet require corresponding changes in o1-testnet or else things will break.
The main change to kubernetes/testnet is within the variable
block_producer_configs
, which is changed to allow for the account keypair to be named consistently with what it's name is within the test_executive, to keep the test_executive and infra to be more in accord with each other.block_producer_key_pass
was removed because it was actually only being used by the seed node for the seed node's libp2p key, which is blatantly not what the variable is supposed to be doing. The rest of the changes to infra were to either make the above changes function, or deletions/streamlining of old, deprecated, or redundant variables which were extremely confusing and very bug prone.