markets - separate watching for pre-commit from prove-commit

[dirkmc]

Depends on filecoin-project/go-fil-markets#453
Fixes #4900

TODO:

• Update test for OnDealSectorCommitted
• Add test for OnDealSectorPreCommitted
• Merge Add StorageDealAwaitingPreCommit state go-fil-markets#453, release and update lotus go.mod with the release version

[magik6k]

Looks good, but it seems we want filecoin-project/go-fil-markets#453 in before this lands

[hannahhoward]

There's a lot of logic in here to deal with the ProveCommitSector message in OnDealSectorPreCommitted -- why would we need this? The problem 4900 captures is that we weren't persisting the progression from PreCommit to PostCommit, so, theoretically, if node restarts in between, we lose the sector number from the pre-commit and never get to the end. It's not to cover for the possibility of missing PreCommit entirely. Unless @magik6k wants to say otherwise, I think we can remove the ProveCommitSector logic (which is complicated) from OnDealSectorPreCommitted.

[hannahhoward]

There is a lot of logic in here to deal with ProveCommitSector messages -- I'm not clear why we're dealing with this. As I see it, the only case of isActive = true we need to deal with is in the called function.

The goal of #4900 is designed to fix the case where a restart happens between pre-commit and prove-commit. We don't need this code for that.

What is the scenario that causes us to miss the pre-commit message entirely? Even if we shut down the node a long time, we will see the pre-commit when we sync the chain.

[dirkmc]

Ah yes that's a good point 👍

Nice catch @hannahhoward. I think it is worth the effort to refactor the code because it will simplify things significantly. I will do so today.

Edit: this is now fixed

[hannahhoward]

Oh dear, I think I broke this. So now I need to fix the problem actually.

[hannahhoward]

I've fixed the history... #oops

And I downgraded my comment from a "request changes" to a suggestion, it's up to you if you want to change. Either way, you can merge at your leisure pending @magik6k's approval

[magik6k]

(testing on my miner; will merge in a bit)