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

price specification by user should match what miner has in their ask #491

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

whyrusleeping
Copy link
Member

This was a poor bit of UX that required users to multiply their storage price by the padded size of their data in order to propose the correct thing to the miner.

price should always be specified in the same terms. Miners asks say price in FIL/GB/Epoch, and what the user types should match this.

Copy link
Collaborator

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

SGWM -- not sure if it might need CLI helptext to be updated

@whyrusleeping
Copy link
Member Author

What helptext?

why@sirius ~/c/lotus (master) [1]> ./lotus client deal --help
NAME:
   lotus client deal - Initialize storage deal with a miner

USAGE:
   lotus client deal [command options] [dataCid miner price duration]

CATEGORY:
   STORAGE

OPTIONS:
   --manual-piece-cid value     manually specify piece commitment for data (dataCid must be to a car file)
   --manual-piece-size value    if manually specifying piece cid, used to specify size (dataCid must be to a car file) (default: 0)
   --from value                 specify address to fund the deal with
   --start-epoch value          specify the epoch that the deal should start at (default: -1)
   --fast-retrieval             indicates that data should be available for fast retrieval (default: true)
   --verified-deal              indicate that the deal counts towards verified client total (default: true if client is verified, false otherwise)
   --provider-collateral value  specify the requested provider collateral the miner should put up
   --help, -h                   show help (default: false)

@arajasek
Copy link
Collaborator

The imaginary helptext that would explain what price means. Obviously.

@dirkmc
Copy link
Contributor

dirkmc commented Feb 22, 2021

Reverted until we can figure out backwards compatibility:
filecoin-project/lotus#5629 (review)

@@ -338,6 +338,8 @@ func (c *Client) ProposeStorageDeal(ctx context.Context, params storagemarket.Pr
return nil, xerrors.Errorf("computing commP failed: %w", err)
}

pricePerEpoch := big.Mul(big.NewInt(int64(pieceSize.Padded())), params.Price)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure params.Price is per GB here

Copy link
Collaborator

Choose a reason for hiding this comment

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

based on how this gets into the actors proposal object, i don't think it is.

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.

4 participants