Skip to content

consensus/babe: load BABE authorities from genesis#603

Merged
noot merged 27 commits intodevelopmentfrom
noot/genesis-auth
Feb 15, 2020
Merged

consensus/babe: load BABE authorities from genesis#603
noot merged 27 commits intodevelopmentfrom
noot/genesis-auth

Conversation

@noot
Copy link
Copy Markdown
Contributor

@noot noot commented Feb 9, 2020

Changes

  • update DecodeCustom to work inside of structs
  • fix decode of BabeConfiguration
  • set genesis authority data when spawning babe session from core
  • update gssmr0.json to have some test authority keys, maybe we should think about testnet keypairs soon? or a test keyring like substrate has

depends on #597 #595

Tests:

go test ./... -short

Issues:

closes #598

@noot noot marked this pull request as ready for review February 11, 2020 16:48
@noot noot self-assigned this Feb 11, 2020
Copy link
Copy Markdown
Contributor

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

Comment thread codec/decode.go
Comment thread consensus/babe/types.go
Copy link
Copy Markdown
Contributor

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. Looks good to me.

Comment thread core/service.go Outdated
msgRec <-chan p2p.Message // receive messages from p2p service
epochDone <-chan struct{} // receive from this channel when BABE epoch changes
msgSend chan<- p2p.Message // send messages to p2p service
babeAuthority bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe isAuthority ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, we might have an option for grandpaAuthority in the future, so I wanted to be a bit more specific. maybe isBabeAuthority?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Yea, maybe isBabeAuthority to imply a bool...? Might not be necessary though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with the pattern is before the bool.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will update

Copy link
Copy Markdown
Contributor

@edwardmack edwardmack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work, looks good to me.

@noot noot merged commit 1e7b07d into development Feb 15, 2020
@noot noot deleted the noot/genesis-auth branch February 15, 2020 17:43
ryanchristo pushed a commit that referenced this pull request Jun 24, 2020
* update DecodeCustom to work inside of structs
* fix decode of BabeConfiguration
* set genesis authority data when spawning babe session from core
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.

babe, gossamer testnet: set authorities in genesis file

4 participants