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

feat: sealing pipeline: Prepare deal assigning logic for FIP-45 #9412

Merged
merged 4 commits into from
Oct 12, 2022

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Oct 4, 2022

Related Issues

Closes #9276

What this does

  1. Add some checks which prevent deals from being assigned to sectors with expirations above termMax
  2. Make cc-upgrade sector selection logic aware of termMax, prefer sectors which can fit most queued deals unless there are multiple sectors which can accommodate the same amount of deal data capped at sector size.

@magik6k magik6k requested a review from a team as a code owner October 4, 2022 18:19
@magik6k magik6k force-pushed the feat/fip45-dealpacking branch from b055380 to 69b6d7f Compare October 4, 2022 18:25
storage/pipeline/input.go Outdated Show resolved Hide resolved
@geoff-vball geoff-vball force-pushed the gstuart/integrate-verifreg-changes-fip45 branch 4 times, most recently from 8c8b860 to b5e9f27 Compare October 6, 2022 15:06
Base automatically changed from gstuart/integrate-verifreg-changes-fip45 to release/v1.18.0 October 7, 2022 15:27
@arajasek arajasek force-pushed the feat/fip45-dealpacking branch from 69b6d7f to 75539ac Compare October 7, 2022 15:33
Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

Looks ok, I'll review again once todos are gone

storage/pipeline/input.go Outdated Show resolved Hide resolved
storage/pipeline/input.go Outdated Show resolved Hide resolved
@@ -127,7 +127,7 @@ type State interface {
) (weight, verifiedWeight abi.DealWeight, err error)
NextID() (abi.DealID, error)
GetState() interface{}
GetAllocationIdForPendingDeal(dealId abi.DealID) (verifregtypes.AllocationId, error)
GetAllocationIdForPendingDeal(dealId abi.DealID) (*verifregtypes.AllocationId, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using nil you shoulld use the zero value which stands for invalid allocation. Updated go-state-types to help out: filecoin-project/go-state-types#93

@ZenGround0
Copy link
Contributor

FYI @jennijuju this is the kind of thing we definitely want an integration test for after code freeze

@jennijuju
Copy link
Member

FYI @jennijuju this is the kind of thing we definitely want an integration test for after code freeze

noted! ideally we shall have an integration test WITH the pr.. (but given we are cut close to the ddl I will create an follow up issue! thanks for the ping.

for _, piece := range sector.Pieces {
used += piece.Piece.Size.Unpadded()

if piece.DealInfo.DealProposal.EndEpoch > lastDealEnd {
Copy link
Contributor

Choose a reason for hiding this comment

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

Because piece.DealInfo is a pointer, should check if it is nil first?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the review!

@arajasek arajasek force-pushed the feat/fip45-dealpacking branch from 6e36386 to 65e37c2 Compare October 12, 2022 17:54
@arajasek arajasek enabled auto-merge October 12, 2022 17:54
@arajasek arajasek disabled auto-merge October 12, 2022 19:46
@arajasek arajasek merged commit 80eae53 into release/v1.18.0 Oct 12, 2022
@arajasek arajasek deleted the feat/fip45-dealpacking branch October 12, 2022 19:46
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.

6 participants