-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat(lib/babe): implement secondary vrf slot block production #2307
feat(lib/babe): implement secondary vrf slot block production #2307
Conversation
Finally saw that magical log message suggesting that secondary vrf slots are being verified correctly
|
- Also merge slot claim logic
f274ac3
to
b6f481d
Compare
Codecov Report
@@ Coverage Diff @@
## kishan/feat/secondary-slot-block-production #2307 +/- ##
===============================================================================
- Coverage 57.55% 57.53% -0.02%
===============================================================================
Files 211 211
Lines 27937 27947 +10
===============================================================================
+ Hits 16078 16079 +1
- Misses 10226 10234 +8
- Partials 1633 1634 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@kishansagathiya what version of polkadot are you using to test this? on my machine it doesn't look like polkadot is able to import gossamer's blocks |
@@ -84,6 +84,37 @@ func checkPrimaryThreshold(randomness Randomness, | |||
return inoutUint.Compare(threshold) < 0, nil | |||
} | |||
|
|||
func claimSecondarySlotVRF(randomness Randomness, |
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 is interesting, the VRF output isn't compared to any sort of threshold? I'm wondering what the point of the VRF is.. could you link to the substrate code? just wondering if they have any explanation of this
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.
@kishansagathiya not something that needs to be changed code-wise, but just curious!
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.
It seems like this vrf can be consumed by parachains, I am not sure why. https://github.com/paritytech/substrate/blob/master/client/consensus/babe/README.md
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 is the relevant code
https://github.com/paritytech/substrate/blob/c22fce5a311beede13479c9a00cca85d823b6b00/client/consensus/babe/src/authorship.rs#L158-L166
https://github.com/paritytech/substrate/blob/c22fce5a311beede13479c9a00cca85d823b6b00/client/consensus/babe/src/authorship.rs#L140-L155
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.
nice, looks very clean! let me know the versions you used to test so i can try it out
0.9.10 works. So, you can follow the same steps described in https://hackmd.io/LfRK6aG6T7mMCE4Uybr-Mg.
Check if gossamer got disconnected. Looked for |
if err != nil { | ||
return nil, err | ||
} | ||
enc, err := scale.Marshal(digest) |
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.
nit babeDigestEncoding
would be nice. I'm an explicit/full words maniac so let me know if i'm bothering as well 😅
keypair *sr25519.Keypair, | ||
authorityIndex uint32, | ||
) (*VrfOutputAndProof, error) { | ||
|
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.
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.
tried on my home machine with v0.9.10 and it works! nice :D
7342519
to
0701543
Compare
Changes
Tests
"allowed_slots": "PrimaryAndSecondaryVRFSlots"
. Use the modifies genesis-spec.json to create genesis.jsonPrimary Reviewer