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.

Sign up for GitHub

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

Jump to bottom

support padding out smaller files #536

Merged
merged 8 commits into from
Jul 23, 2021
Merged

support padding out smaller files #536

merged 8 commits into from
Jul 23, 2021

Conversation

whyrusleeping
Copy link
Member

This allows me to take a 1MB file, pad it up to 512MB, and make a deal with a miner whose minimum piece size is 512MB

@whyrusleeping whyrusleeping mentioned this pull request Apr 7, 2021
Closed
ribasushi
ribasushi approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@ribasushi ribasushi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I built a stock v1.6.0 with the following modification:

diff --git a/go.mod b/go.mod
index 503e95c5d..2d2548ccc 100644
--- a/go.mod
+++ b/go.mod
@@ -31 +31 @@ require (
-       github.com/filecoin-project/go-commp-utils v0.0.0-20201119054358-b88f7a96a434
+       github.com/filecoin-project/go-commp-utils v0.1.1-0.20210407220142-9d7f7aaf8c9c
@@ -35 +35 @@ require (
-       github.com/filecoin-project/go-fil-markets v1.1.9
+       github.com/filecoin-project/go-fil-markets v1.1.10-0.20210407195421-65658e33ed64

🎉 OMG IT WORKED 🎉

Retrieval from a completely unrelated UNPATCHED client worked too

@whyrusleeping you are the hero we have but do not deserve

:shipit: ( of course port 65658e3 back from #537 )

@ribasushi ribasushi mentioned this pull request Apr 7, 2021
Merged
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2021 •
edited
Loading

Codecov Report

Merging #536 (354aee0) into master (b9e514c) will decrease coverage by 0.63%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
- Coverage   66.77%   66.14%   -0.62%     
==========================================
  Files          56       56              
  Lines        4227     4249      +22     
==========================================
- Hits         2822     2810      -12     
- Misses       1155     1188      +33     
- Partials      250      251       +1
Impacted Files Coverage Δ
storagemarket/impl/provider.go 28.22% <0.00%> (-0.81%) ⬇️
storagemarket/impl/provider_environments.go 0.00% <0.00%> (ø)
storagemarket/impl/requestvalidation/common.go 85.19% <ø> (-1.91%) ⬇️
...oragemarket/impl/providerstates/provider_states.go 86.01% <75.00%> (-0.59%) ⬇️
storagemarket/impl/providerutils/providerutils.go 95.66% <100.00%> (ø)
retrievalmarket/events.go 0.00% <0.00%> (-80.00%) ⬇️
retrievalmarket/dealstatus.go 0.00% <0.00%> (-80.00%) ⬇️

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 f15eef9...354aee0. Read the comment docs.

@ribasushi ribasushi mentioned this pull request Jul 5, 2021
Merged
@ribasushi
Copy link
Contributor

@whyrusleeping we never shipped this way back for some reason: I merged it with latest master and made extra changes - please review / ack.

I particularly dislike Inflator thing, but can't come up with a better way. See filecoin-project/lotus#6683 (comment) for context.

@ribasushi
Copy link
Contributor

@whyrusleeping ok, the original thing that you wrtoe has been reviewed and good
I have added the following on top, please look over when time permits
image

@ribasushi
Copy link
Contributor

I can not reproduce this CI fail locally no matter what I do :(

@whyrusleeping
Copy link
Member Author

Okay, that looks good to me.

@ribasushi ribasushi merged commit 46b5224 into master Jul 23, 2021
@ribasushi ribasushi deleted the feat/support-padding branch July 23, 2021 22:59
aarshkshah1992 pushed a commit that referenced this pull request Jul 26, 2021
@ribasushi @whyrusleeping @raulk
Chore/tentative master m1 merge (#590)
a129289 
* remove wrong peer check in push deal validation (#585)

* remove wrong peer check in push deal validation

* kill the pull side of peer id checking as well

* fixup comments

* Do not hex-encode CIDs in logs (#561)

* On overloaded CI 10 seconds just isn't enough (#587)

* support padding out smaller files (#536)

* support padding out smaller files

* Actually write out the padding - AP depends on it

Co-authored-by: Peter Rabbitson <[email protected]>

Co-authored-by: Whyrusleeping <[email protected]>
Co-authored-by: Raúl Kripalani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ribasushi ribasushi ribasushi approved these changes

@Stebalien Stebalien Awaiting requested review from Stebalien

@hannahhoward hannahhoward Awaiting requested review from hannahhoward

@magik6k magik6k Awaiting requested review from magik6k

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@whyrusleeping @codecov-commenter @ribasushi