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.

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

Lotus fetches drand from chain backwards with null blocks while specs fetches forward #3613

Closed
Tracked by #6185
nikkolasg opened this issue Sep 7, 2020 · 3 comments · Fixed by #6240
Closed
Tracked by #6185
Assignees
Labels
impact/consensus Impact: Consensus
Milestone

Comments

@nikkolasg
Copy link
Contributor

nikkolasg commented Sep 7, 2020

Describe the bug
There is a diff with the spec as to how fetch drand entry from the chain.
Lotus will look backward in the chain as long as it doesn't find a drand entry inside (code). Also, given a certain epoch to fetch drand randomness from, Lotus starts from the round before if the height chosen falls on a null block (code).

The spec starts by deciding the minimum Filecoin block height and then looks forwards if it encounters null blocks (spec)

Expected behavior

In case of null rounds, drand entries will be inserted at later rounds (even adversarially) - so lotus should fetch it from later blocks as well.

Reasoning: Using this behavior, a malicious miner can decide to withold its block or not, and that changes the randomness for a given epoch. If he witholds the block at height e and he's the only winner, then the randomness associated with height e is the one at the block at height e-1. If he doesn't, then the randomness is the one associated with this block at height e.

@arajasek
Copy link
Contributor

@nikkolasg I'd like to propose splitting this into two issues:

  • The change around null blocks is important, and will get into the next network upgrade (see Hackily fix Drand fetching around null blocks #6239 and Fix randomness fetching around null blocks #6240 for PRs to fix it).
  • The change around what Lotus does "it doesn't find a drand entry inside" feels of lower practical importance -- there should always be a drand entry on mainnet (and our current testnets). I'd like to reduce that to a "protocol impl hazard" and track elsewhere, since it likely will not be relevant any time soon.

What do you think?

@BigLep
Copy link
Member

BigLep commented May 28, 2021

There is an additional test that @arajasek is going to write before we close.

@arajasek
Copy link
Contributor

arajasek commented Jul 3, 2021

Unfortunately, the fix in #6240 wasn't quite right. If the tipset corresponding to round N is null, when fetching randomness for round N we now return the entry at round N+1. This is also wrong (we were using N-1 before).

This needs to be reopened and fixed in another network upgrade.

@arajasek arajasek reopened this Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment