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

check for deal start epoch on SectorAddPieceToAny #7407

Merged

Conversation

nonsense
Copy link
Member

@nonsense nonsense commented Sep 30, 2021

Should fix: #5137


This PR is:

  1. Adding a check for deal start epoch in SectorAddPieceToAny.
  2. Adding an epoch buffer StartEpochSealingBuffer to both SectorAddPieceToAny, as well as to validateDeal prior to sending a PublishStorageDeals message, to account for some time necessary to seal a sector.

At the moment I have added StartEpochSealingBuffer to be 480 by default == which is 4 hours, and I think this is reasonable. I am not sure if there are miners out there that go from PublishStorageDeals to a sealed sector faster than 4 hours...

@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #7407 (809289f) into master (edcb287) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7407      +/-   ##
==========================================
- Coverage   39.53%   39.51%   -0.02%     
==========================================
  Files         616      616              
  Lines       65281    65295      +14     
==========================================
- Hits        25810    25804       -6     
- Misses      34982    34992      +10     
- Partials     4489     4499      +10     
Impacted Files Coverage Δ
extern/storage-sealing/input.go 55.16% <20.00%> (-1.30%) ⬇️
markets/storageadapter/dealpublisher.go 81.55% <100.00%> (+0.08%) ⬆️
node/builder_miner.go 94.85% <100.00%> (+0.03%) ⬆️
node/config/def.go 98.64% <100.00%> (+<0.01%) ⬆️
node/modules/storageminer.go 62.50% <100.00%> (+0.06%) ⬆️
markets/loggers/loggers.go 89.28% <0.00%> (-10.72%) ⬇️
chain/actors/builtin/miner/diff.go 48.52% <0.00%> (-10.30%) ⬇️
extern/sector-storage/sched_resources.go 75.00% <0.00%> (-9.38%) ⬇️
chain/stmgr/tracers.go 84.21% <0.00%> (-5.27%) ⬇️
chain/actors/builtin/paych/paych.go 16.88% <0.00%> (-5.20%) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edcb287...809289f. Read the comment docs.

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.

It would be great to have a config for minimum start epoch 'buffer' to give time for sealing

@nonsense nonsense force-pushed the nonsense/check-deal-start-epoch-on-SectorAddPieceToAny branch 3 times, most recently from dcc6fd4 to 9d78d45 Compare September 30, 2021 12:45
@jennijuju jennijuju added this to the v1.13.0 milestone Oct 1, 2021
@nonsense nonsense marked this pull request as ready for review October 1, 2021 12:26
@nonsense nonsense requested a review from a team as a code owner October 1, 2021 12:26
@magik6k magik6k force-pushed the nonsense/check-deal-start-epoch-on-SectorAddPieceToAny branch from 9d78d45 to 809289f Compare October 1, 2021 15:44
@magik6k
Copy link
Contributor

magik6k commented Oct 1, 2021

(rebased to update config docs)

@magik6k magik6k merged commit 28e720f into master Oct 1, 2021
@magik6k magik6k deleted the nonsense/check-deal-start-epoch-on-SectorAddPieceToAny branch October 1, 2021 17:03
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.

Expired deals continue to cause new sectors to be removed
3 participants