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

BasicDealFilter upper bound on StartEpoch #3929

Closed
kirk-baird opened this issue Sep 19, 2020 · 6 comments · Fixed by #4173
Closed

BasicDealFilter upper bound on StartEpoch #3929

kirk-baird opened this issue Sep 19, 2020 · 6 comments · Fixed by #4173
Labels
area/markets Area: Markets area/ux Area: UX effort/hours Effort: Hours P2 P2: Should be resolved

Comments

@kirk-baird
Copy link

Describe the bug

The BasicDealFilter is an implementation of custom logic for validating deals. This is designed to automatically reject deals which do not meet the requirements of a miner but are technically valid deals (can be published on-chain).

Currently there is no upper bound on the StartEpoch of a DealProposal. As a result a client may set the StartEpoch excessively far into the future.

Fees received by a miner are the (EndEpoch - StartEpoch) * price + UnsealFee. Hence, if StartEpoch is set into the future the miner will receive less fees.

The miner will be storing the data from when the deal is first accepted by go-fil-markets. Therefore, the time between the deal being accepted and the StartEpoch the miner will be storing the data for free.

To Reproduce
Steps to reproduce the behavior:

  1. Run lotus client deal --start-epoch 1000000 <CID> <MINER> <PRICE> minDuration
  2. Deal will be accepted by miner running BasicDealFilter

Expected behavior
Deals excessively far into the future should be rejected.

Version (run lotus version):
lotus version 0.5.7+git.f31dc791

Additional context
There is currently a lower bound for the StartEpoch to give the miner sufficient time to deal the data.
There should also be an upper bound for the StartEpoch relative to the current epoch. The upper bound is a trade-off between allowing enough time to fill the sector with other deals vs storing the data without pay.

Clients may successfully call lotus client retrieve ... during the period after the deal has been accepted but before StartEpoch.

Note also EndEpoch - StartEpoch must be greater than minDuration.

File: lotus/node/modules/storageminer.go
Function: BasicDealFilter()

@jennijuju
Copy link
Member

@hannahhoward I think this is a good suggestion?

Also..is clientRetrieve not checking deal start epoch a bug?

@hannahhoward
Copy link
Contributor

If the deal can be retrieved, we should serve the retrieval (which is paid for seperately), so that part is not a bug.

This is definitely a feature we should have -- though the intent of the basic deal filter is to be ... basic. So it's an interesting question if this falls under the responsibilities or if we should leave it to miners to customize. I think though the potential for abuse makes it worth putting in the basic deal filter.

@jennijuju
Copy link
Member

If the deal can be retrieved, we should serve the retrieval (which is paid for seperately), so that part is not a bug.

This is definitely a feature we should have -- though the intent of the basic deal filter is to be ... basic. So it's an interesting question if this falls under the responsibilities or if we should leave it to miners to customize. I think though the potential for abuse makes it worth putting in the basic deal filter.

second part is sort of related to #3958

@hannahhoward hannahhoward added effort/hours Effort: Hours P2 P2: Should be resolved labels Sep 23, 2020
@stuberman
Copy link

Getting the same error using the recommended miner settings and causing "PreCommitFailed" on new sectors
PreCommit Fail invalid expiration.txt

  1. 2020-09-29 03:56:50 +0000 UTC: [event;sealing.SectorChainPreCommitFailed] {"User":{}}
    pushing message to mpool: GasEstimateMessageGas error: estimating gas used: message execution failed: exit 16, reason: invalid expiration 1441130988, cannot be more than 1555200 past current epoch 101514 (RetCode=16)

@hsjw12
Copy link

hsjw12 commented Sep 29, 2020

Please help me
How is this command(lotus client retrieve ...) used?
ex) lotus client retrieve CID ~/file or PATH
is it right?

@hannahhoward
Copy link
Contributor

Follow-up:

There may be a more complext cryptoeconomic solution, but as a first pass we will set the basic deal filter to reject anything more than 90 days in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/markets Area: Markets area/ux Area: UX effort/hours Effort: Hours P2 P2: Should be resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants