Follow up on #939 (leader election and drand) #967
Conversation
|
we must cherry-pick henri's commit 5fd4097 |
src/algorithms/crypto/randomness.md
Outdated
| @@ -86,6 +66,36 @@ GetRandomness(dst, l, s): | |||
| {{< readfile file="/docs/actors/actors/crypto/randomness.go" code="true" lang="go" >}} | |||
| {{< readfile file="/docs/systems/filecoin_blockchain/struct/chain/chain.go" code="true" lang="go" >}} | |||
|
|
|||
| ## Drawing tickets from the VRF-chain for proof inclusion | |||
|
|
|||
| There are a few instances in which the protocol requires inclusion to be drawn from the Filecoin blockchain's VRF-chain (which generates {{<sref tickets>}} with each new block), in order to tie certain proofs to a particular set of Filecoin blocks (i.e. a given chain or fork). This is notably the case for Seal Pre-commits (see {{<sref sealing>}}). | |||
There was a problem hiding this comment.
the protocol requires inclusion?
There was a problem hiding this comment.
"This is notably the case for Seal Pre-commits (see {{}})." is awkward phrasing
There was a problem hiding this comment.
this is the fact that SealRandomness must be taken from VRF chain, this ensures that no other fork can replay it. We should make it explicit.
There was a problem hiding this comment.
fair enough. I'll rephrase.
There was a problem hiding this comment.
Some notes. Note that ElectionPeriod is currently undefined in spec, and imo, not particularly helpful to understanding the Filecoin/drand mapping (but maybe that is beacuse not defined)...
I also think we may not be on the exact same page wrt which drand-related methods are called during filecoin block generation vs validation @nikkolasg
src/systems/filecoin_blockchain/storage_power_consensus/_index.md
Outdated
Show resolved
Hide resolved
src/systems/filecoin_blockchain/storage_power_consensus/_index.md
Outdated
Show resolved
Hide resolved
src/systems/filecoin_blockchain/storage_power_consensus/_index.md
Outdated
Show resolved
Hide resolved
src/systems/filecoin_blockchain/storage_power_consensus/_index.md
Outdated
Show resolved
Hide resolved
| When mining, a miner can fetch entries from the drand network to include them in | ||
| the new block by calling the method `GetBeaconEntriesForEpoch`. |
There was a problem hiding this comment.
Last I checked lotus this is not quite right. The drand network methods (ie to fetch the values) are invoked in block creation. The GetBeaconEntries/Entry method are used as part of block validation...
There was a problem hiding this comment.
Well, that's what this function is doing: it fetches the drand entries via network drand.Public(round) is a call to the drand network.
For block validation, you use GetRandomnessFromBeacon(e ChainEpoch, head ChainEpoch), which derives the randomness from the chain itself.
Does that seem right to you ?
There was a problem hiding this comment.
Mmmh I guess you're right. In that case, the naming is not very good (ie we should make explicit what fetches from beacon and what fetches from chain) but I suppose this is correct, yes.
| @@ -119,6 +152,25 @@ GetBeaconEntriesForEpoch(epoch) []BeaconEntry { | |||
| } | |||
| ``` | |||
|
|
|||
| XXX: is it ever used this function ? | |||
There was a problem hiding this comment.
Yes, as part of validation. GetBeaconEntriesForEpoch is not used anywhere else though, last I checked.
There was a problem hiding this comment.
For me, the func GetRandomnessFromBeacon(e ChainEpoch, head ChainEpoch) (DrandEntry,error) is what is used now, the "new" function to make sure we take care of the issue @nicola wrote before (about having null blocks changing the drand round <-> filecoin round mapping).
If you consider GetRandomnessFromBeacon the "one function to use to get drand entry from the chain", then I'm not sure we still need this one (GetBeaconEntryForEpoch) ?
There was a problem hiding this comment.
you're right, they look dup at this point. In that case we should replace all uses of GetBeaconEntryForEpoch with GetRandomnessFromBeacon and remove the former, no?
| // special case for genesis block (it has no beacon entry and so the first verifiable value comes at height 2, | ||
| // as with GetBeaconEntriesForEpoch() | ||
| if priorBlockHeader.Epoch == 0 { | ||
| return nil | ||
| } | ||
|
|
src/systems/filecoin_blockchain/storage_power_consensus/_index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Henri <3359083+sternhenri@users.noreply.github.com>
|
@sternhenri I replied to your comments by other questions / assertions :) |
|
Left a final ping for you @nikkolasg, but otherwise lgtm. |
|
Which ping ? ;) |
|
We may have to rebase this. Will @yiannisbot's team fix the conflict with beta? |
|
I will fix the conflicts with |
|
|
||
| Per the above, a Filecoin chain will contain the entirety of the beacon's output from the Filecoin genesis to the current block. | ||
|
|
||
| Given their role in leader election and other critical protocols in Filecoin, a block's beacon entries must be validated for every block. See {{<sref drand>}} for details. This can be done by ensuring every beacon entry is a valid signature over the prior one in the chain, using drand's [`Verify`](https://github.com/drand/drand/blob/master/beacon/beacon.go#L72) endpoint as follows: |
There was a problem hiding this comment.
The source file in drand with the Verify function has moved and the link 404s. I'm gonna updated it to point to https://github.com/drand/drand/blob/763e9a252cf59060c675ced0562e8eba506971c1/chain/beacon.go#L76
License: MIT Signed-off-by: Oli Evans <oli@tableflip.io>
|
@yiannisbot I've fixed the merge conflicts here. Please review and merge when ready. @nikkolasg I've taken care to preserve the content of your PR, but if you have time, please verify the merge conflict fixes to check nothing got lost in translation. |
…n update (#1066) This PR is applying all the latest changes in the Expected Consensus, Leader Election and the rationale behind Poisson Sortition of WinCount. It builds on top of the changes merged from #967 Tooling: - katex shortcode documented and working - markdown linter actually fails now Co-authored-by: Hugo Dias <hugomrdias@gmail.com>
Follow up on #939