-
Notifications
You must be signed in to change notification settings - Fork 7
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
GetBestMiningCandidate & F3 #730
GetBestMiningCandidate & F3 #730
Comments
Note: I have no idea if this is actually the issue, but it's very suspicious. We need to audit all the miner code for unexpected gotchas. |
I'm pretty certain the reason for this existing is catchup mining, where when we fast loop and lotus takes too long to update the head. Also, it would be an issue almost for sure. It might not be the actual cause of what we observe, but I can see it crop up. |
Ok, I'm pretty sure there's no point behind To be clear, |
Oh! Maybe the concern is multiple lotus nodes where one is behind? I mean, this isn't really the right way to deal with that especially because we completely ignore anything we mined. |
It looks like we keep it for side effects in some cases... E.g., we shove the null rounds it it. |
Yeah, the issue is catch-up mining, but it's not about being "fast", it's about null rounds. When catching up, we keep trying to mine on every round after our last tipset until we either see a better head or win a block. Of course... we don't wait after winning which means it's kind of broken. Unfortunately, I'm not sure if I can trivially remove the "take heaviest" rule because, due to F3, we can flip-flop. I.e.:
At that point, we'd want to pick up at |
Ok, there's a simple fix: we can't mine at the same height anyways. So we'll just track the last height we've successfully determined we didn't win and use that to calculate nulls. |
Fixed in filecoin-project/lotus#12690 |
When we pick a base on which to mine, we compare:
If the last tipset we mined on is heavier, we'll stick with that. This leads to a problematic situation:
lastWork
field, I'm not 100% sure why we have it.ChainTipSetWeight
to take checkpoints into account and set the weight to 0 for any tipset not compatible with the current checkpoint. We need to be careful here in case the tipset weight is used anywhere else, but I don't think it is?The text was updated successfully, but these errors were encountered: