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.

Sign up for GitHub

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

Jump to bottom

Adjustments to PBFT ByronConfig #795

Merged
merged 3 commits into from
Jul 19, 2019
Merged

Adjustments to PBFT ByronConfig #795

merged 3 commits into from
Jul 19, 2019

Conversation

deepfire
Copy link
Contributor

@deepfire deepfire commented Jul 18, 2019 •
edited by dcoutts

Three cleanups:

  • remove a confusing and unused part of the ByronConfig
  • add TODOs about things relevant only for the demo (that were confusing people)
  • allow nodes to be not a BFT slot leader by making the leader credentials optional

@deepfire deepfire force-pushed the dcoutts/pfbt-config branch 2 times, most recently from 1076d50 to b2000ca Compare July 19, 2019 00:15
@deepfire
Copy link
Contributor Author

Dropped the byron-proxy elimination changes, to make CI pass.

@dcoutts dcoutts force-pushed the dcoutts/pfbt-config branch 2 times, most recently from d08a13b to 770556b Compare July 19, 2019 00:41
@dcoutts
Remove unused pbftCoreNodes mapping from ByronConfig
573d504 
The pbftCoreNodes was a bi-directional mapping between the core node
BFT index and the node's operational verification key.

This was never used. It was also confusing since as static configuration
it is not useful since the real mapping can change when new heavyweight
delegation certificates are posted.
@dcoutts
Add TODOs about pbftSecrets and tx elaboration.
c73f6a7 
The demo supports submitting transactions which are specified in a nice
simple manner. To support this we have to support elaborating mock
transactions into real transaction, which needs access to the signing
keys of the available wallet accounts. This is clearly a demo-only kind
of thing. The TODOs note this and note that the pbftSecrets can be
removed once transaction submission is handled differently, which is
good since the pbftSecrets is a confusing thing to keep around in the
real system.
@dcoutts
Allow not being a PBFT slot leader
cbb930e 
Currently the NodeConfig for PBft makes it non-optional to be a core
node, with corresponding credentials. This patch makes it optional so we
can have nodes without core node credentials.

We introduce a PBftIsLeader type that holds the various keys needed for
core nodes. We make the NodeConfig contain a Maybe one of these.

We take advantage of the IsLeader "proof" type and for PBft we make it
carry the PBftIsLeader. Of course this does not prove that n `mod` m = i
but it does at least hold all the credentials which are needed for
signing blocks. So it makes a lot of sense for checkIsLeader to return
these credentials.

The rest is just following through the consequences.
@dcoutts dcoutts changed the title Resolve confusions around ByronConfig Adjustments to PBFT ByronConfig Jul 19, 2019
@deepfire deepfire merged commit 49da3b5 into master Jul 19, 2019
@iohk-bors iohk-bors bot deleted the dcoutts/pfbt-config branch July 19, 2019 15:48
@mrBliss mrBliss mentioned this pull request Jul 22, 2019
Closed
@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nc6 nc6 nc6 approved these changes

Assignees
No one assigned
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@deepfire @nc6 @mrBliss @dcoutts