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

Error out deals that are not activated by proposed deal start epoch #5061

Merged
merged 5 commits into from
Dec 1, 2020

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Nov 30, 2020

Fixes #4990

When a deal is terminated by the miner, the client can get stuck in StorageDealSealing.

This PR adds functionality to the functions that wait for pre-commit and prove-commit such that they will callback with an error if the chain reaches the proposed start epoch for the deal without the deal being activated.

Fixes #4977

Provides a better error message for when the deal is not found.
Old: "deal 12345 not found"
New: "deal 12345 not found - deal may not have completed sealing before deal proposal start epoch, or deal may have been slashed"

Copy link
Contributor

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth investingating the timeout functionality builtin to Events.Called, though, I guess I wouldn't hold up merge on it.

markets/storageadapter/ondealsectorcommitted.go Outdated Show resolved Hide resolved
markets/storageadapter/ondealsectorcommitted.go Outdated Show resolved Hide resolved
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just 1 thing

markets/storageadapter/ondealsectorcommitted.go Outdated Show resolved Hide resolved
@magik6k magik6k merged commit 82b5cb8 into master Dec 1, 2020
@magik6k magik6k deleted the fix/err-late-deals branch December 1, 2020 16:02
@kernelogic
Copy link

Is it a better practice to specify a startepoch much later to allow more time to seal?

@dirkmc
Copy link
Contributor Author

dirkmc commented Dec 2, 2020

It depends on the individual miner. If the miner is already overloaded and struggling to seal in time, setting the proposed deal start epoch to a later time may not help much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants