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

fix SectorState #3881

Merged
merged 2 commits into from
Sep 18, 2020
Merged

Conversation

deaswang
Copy link
Contributor

fix #3626
update SectorState type.

@@ -1,43 +1,43 @@
package sealing

type SectorState string
type SectorState uint8
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this will break everyones miner sector states (this is used in on-disk state). Using strings also makes it much easier to migrate states.

The correct fix for this issue is likely just having a list of valid states, and checking against that list in the update-state command

@@ -2,6 +2,8 @@ package sealing

type SectorState string

var ExistSectorStateList = make(map[SectorState]struct{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Initializing the values here instead of in init would be cleaner / safer

Suggested change
var ExistSectorStateList = make(map[SectorState]struct{})
var ExistSectorStateList = map[SectorState]struct{}{
Empty: {},
...
}

@magik6k
Copy link
Contributor

magik6k commented Sep 16, 2020

(also looks like we need go fmt ./...)

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.

Looks good, thanks!

@magik6k magik6k merged commit 7109e95 into filecoin-project:master Sep 18, 2020
@deaswang deaswang deleted the issue-3626-SectorState branch December 16, 2020 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ForceSectorState is not doing sanity check with valid states.
2 participants