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

[part 3] "miner create" command must accept "sectorsize" flag #2874

Merged
merged 3 commits into from
Jun 4, 2019

Conversation

laser
Copy link
Contributor

@laser laser commented Jun 2, 2019

Fixes #2530.

Note that this PR is based off of #2873. Once that PR is merged, this PR will need to be rebased onto master.

Why does this PR exist?

We need to give an operator a command option which lets them pick the size of the sectors which their newly-created storage miner actor will commit.

What's in this PR?

  • modify miner create to accept an optional sectorsize flag
  • default to whatever sector size is supported by the proofs mode

Usage

Invalid sectorsize

$ ./go-filecoin miner create 1 --sectorsize=woot
Error: invalid sector size: woot

Unsupported sectorsize

$ ./go-filecoin miner create 1 --sectorsize=12
Error: unsupported sector size: 12 (supported sizes: 268435456)

@laser laser changed the base branch from master to feat/2530-create-miner-with-sector-size June 2, 2019 07:22
@laser laser changed the title Feat/2530 command for create miner accepts size "miner create" command must accept "sectorsize" flag Jun 2, 2019
@laser laser changed the title "miner create" command must accept "sectorsize" flag [part 3] "miner create" command must accept "sectorsize" flag Jun 2, 2019
@laser laser force-pushed the feat/2530-command-for-create-miner-accepts-size branch 2 times, most recently from b8bdf0a to 214577a Compare June 2, 2019 07:38
@laser laser requested review from anorth, acruikshank and ZenGround0 and removed request for acruikshank June 3, 2019 15:28
Copy link
Contributor

@acruikshank acruikshank left a comment

Choose a reason for hiding this comment

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

Lgtm

@@ -50,10 +51,11 @@ miner's collateral drops below 0.001FIL, the miner will not be able to commit
additional sectors.`,
},
Arguments: []cmdkit.Argument{
cmdkit.StringArg("collateral", true, false, "The amount of collateral, in FIL"),
cmdkit.StringArg("collateral", true, false, "The amount of collateral, in FIL."),
Copy link
Member

Choose a reason for hiding this comment

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

FWIW I think we could make this an option (rather than argument) too

@laser laser force-pushed the feat/2530-create-miner-with-sector-size branch from 37062e9 to 8088123 Compare June 4, 2019 16:29
@laser laser changed the base branch from feat/2530-create-miner-with-sector-size to master June 4, 2019 16:51
@laser laser force-pushed the feat/2530-command-for-create-miner-accepts-size branch from 214577a to ee18338 Compare June 4, 2019 17:17
@laser laser merged commit 4e53198 into master Jun 4, 2019
@laser laser deleted the feat/2530-command-for-create-miner-accepts-size branch June 4, 2019 17:47
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.

A miner selects a sector size when pledging
3 participants