Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Remove unused SectorSize from VerifyDealsOnSectorProveCommitParams #328

Merged
merged 2 commits into from
Apr 28, 2020

Conversation

anorth
Copy link
Member

@anorth anorth commented Apr 27, 2020

FYI @magik6k

@anorth anorth requested a review from zixuanzh April 27, 2020 03:39
@codecov-io
Copy link

Codecov Report

Merging #328 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #328   +/-   ##
=======================================
  Coverage   37.60%   37.61%           
=======================================
  Files          39       39           
  Lines        4087     4086    -1     
=======================================
  Hits         1537     1537           
+ Misses       2326     2325    -1     
  Partials      224      224           

Copy link
Collaborator

@zixuanzh zixuanzh left a comment

Choose a reason for hiding this comment

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

Hmmm we can remove it but we need to verify that the totalDealSpacetime + totalVerifiedDealSpacetime <= totalSectorSpacetime, maybe after the return?

@anorth
Copy link
Member Author

anorth commented Apr 28, 2020

I don't quite understand your request, but I think it's orthogonal to removing this unused field.

The market actor already confirms that each deal expires no later than the sector expiry, so the "time" part is already satisfied. I think the simple fact that the size of the deals sums to less than the size of the sector is implicit in the computation of the data commitment from the deal CIDs and sizes. The sector size is implicit in the sector proof type already passed as a parameter to that syscall.

Copy link
Collaborator

@zixuanzh zixuanzh left a comment

Choose a reason for hiding this comment

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

Beautiful, just want to make sure that. Thanks!

@anorth anorth merged commit 6df71e9 into master Apr 28, 2020
@anorth anorth deleted the anorth/fix branch April 28, 2020 04:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants