-
Notifications
You must be signed in to change notification settings - Fork 462
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
Chain sampling for challenge seed skips over null blocks. #3026
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,62 +6,57 @@ import ( | |
"github.com/filecoin-project/go-filecoin/types" | ||
) | ||
|
||
// LookbackParameter is the protocol parameter defining how many blocks in the | ||
// past to look back to sample randomness values. | ||
// LookbackParameter defines how many non-empty tiptsets (not rounds) earlier than any sample | ||
// height (in rounds) from which to sample the chain for randomness. | ||
// This constant is a protocol (actor) parameter and should be defined in actor code. | ||
const LookbackParameter = 3 | ||
|
||
// SampleChainRandomness produces a slice of random bytes sampled from a tip set | ||
// in the provided slice of tip sets at a given height (minus lookback). This | ||
// function assumes that the tip set slice is sorted by block height in | ||
// descending order. | ||
// | ||
// SampleChainRandomness is useful for things like PoSt challenge seed | ||
// generation. | ||
func SampleChainRandomness(sampleHeight *types.BlockHeight, tipSetsSortedByBlockHeightDescending []types.TipSet) ([]byte, error) { | ||
// SampleChainRandomness produces a slice of bytes (a ticket) sampled from the tipset `LookbackParameter` | ||
// tipsets (not rounds) prior to the highest tipset with height less than or equal to `sampleHeight`. | ||
// The tipset slice must be sorted by descending block height. | ||
func SampleChainRandomness(sampleHeight *types.BlockHeight, tipSetsDescending []types.TipSet) ([]byte, error) { | ||
// Find the first (highest) tipset with height less than or equal to sampleHeight. | ||
// This is more complex than necessary: https://github.com/filecoin-project/go-filecoin/issues/3025 | ||
sampleIndex := -1 | ||
tipSetsLen := len(tipSetsSortedByBlockHeightDescending) | ||
lastIdxInTipSets := tipSetsLen - 1 | ||
|
||
for i := 0; i < tipSetsLen; i++ { | ||
height, err := tipSetsSortedByBlockHeightDescending[i].Height() | ||
for i, tip := range tipSetsDescending { | ||
height, err := tip.Height() | ||
if err != nil { | ||
return nil, errors.Wrap(err, "error obtaining tip set height") | ||
} | ||
|
||
if types.NewBlockHeight(height).Equal(sampleHeight) { | ||
if types.NewBlockHeight(height).LessEqual(sampleHeight) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should do this. While it will probably work for some time this is not compatible with a finished PoSt generate/verify system. We need to sample an exact seed from an exact block height so that proving and verifying interoperate correctly and we should be able to provide it every time. I believe the root of this bug is that the miner actor is getting into a state where its I'm looking into this. cc @acruikshank I can't see any issue with the change here but it is my primary suspect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking closer at the commit mentioned above I see that we do have a consistent reference seed. In light of that this change is ok, especially since all seed sampling code is going to be seeing some major improvements in the near future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with the second comment. This will generate a consistent seed and is the correct approach. |
||
sampleIndex = i | ||
break | ||
} | ||
} | ||
|
||
// Produce an error if no tip set exists in `tipSetsSortedByBlockHeightDescending` with | ||
// block height `sampleHeight`. | ||
// Produce an error if the slice does not include any tipsets at least as low as `sampleHeight`. | ||
if sampleIndex == -1 { | ||
return nil, errors.Errorf("sample height out of range: %s", sampleHeight) | ||
} | ||
|
||
// If looking backwards in time `Lookback`-number of tip sets from the tip | ||
// set with `sampleHeight` would put us farther back in time than the lowest | ||
// height tip set in the slice, then check to see if the lowest height tip | ||
// set is the genesis block. If it is, use its randomness. If not, produce | ||
// an error. | ||
// | ||
// TODO: security, spec, bootstrap implications. | ||
// See issue https://github.com/filecoin-project/go-filecoin/issues/1872 | ||
// Now look LookbackParameter tipsets (not rounds) prior to the sample tipset. | ||
lookbackIdx := sampleIndex + LookbackParameter | ||
if lookbackIdx > lastIdxInTipSets { | ||
leastHeightInChain, err := tipSetsSortedByBlockHeightDescending[lastIdxInTipSets].Height() | ||
lastIdx := len(tipSetsDescending) - 1 | ||
if lookbackIdx > lastIdx { | ||
// If this index farther than the lowest height (last) tipset in the slice, then | ||
// - if the lowest is the genesis, use that, else | ||
// - error (the tipset slice is insufficient) | ||
// | ||
// TODO: security, spec, bootstrap implications. | ||
// See issue https://github.com/filecoin-project/go-filecoin/issues/1872 | ||
lowestAvailableHeight, err := tipSetsDescending[lastIdx].Height() | ||
if err != nil { | ||
return nil, errors.Wrap(err, "error obtaining tip set height") | ||
} | ||
|
||
if leastHeightInChain == uint64(0) { | ||
lookbackIdx = lastIdxInTipSets | ||
if lowestAvailableHeight == uint64(0) { | ||
lookbackIdx = lastIdx | ||
} else { | ||
errMsg := "sample height out of range: lookbackIdx=%d, lastHeightInChain=%d" | ||
return nil, errors.Errorf(errMsg, lookbackIdx, leastHeightInChain) | ||
return nil, errors.Errorf(errMsg, lookbackIdx, lowestAvailableHeight) | ||
} | ||
} | ||
|
||
return tipSetsSortedByBlockHeightDescending[lookbackIdx].MinTicket() | ||
return tipSetsDescending[lookbackIdx].MinTicket() | ||
} |
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.
🙏