Flush out & fix retrieval bugs #525
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aarshkshah1992
changed the title
|
Looks great 👍 Could we also add an integration test for this scenario where the unseal price is non-zero but the price per byte is zero 🙏 |
aarshkshah1992
force-pushed
the
feat/5895
branch
from
b979373
to
b3b5df3
Compare
aarshkshah1992
changed the title
aarshkshah1992
force-pushed
the
feat/5895
branch
from
b3b5df3
to
bdfd414
Compare
aarshkshah1992
force-pushed
the
feat/5895
branch
from
bdfd414
to
9173c93
Compare
aarshkshah1992
commented
Apr 1, 2021
| DealProposal: *proposal, | ||
| Receiver: receiver, | ||
| LegacyProtocol: legacyProtocol, | ||
| CurrentInterval: proposal.PaymentInterval, |
There was a problem hiding this comment.
Not setting the CurrentInterval here creates issues when we have a non zero price for Unsealing & bytes because we don't initialize this value correctly in the FSM when we jump directly to the Give me money for the unsealing state via the RequestPayment event instead of going via the ProviderOpen event .
dirkmc
approved these changes
Apr 1, 2021
| @@ -81,7 +81,11 @@ var ProviderEvents = fsm.Events{ | |||
| From(rm.DealStatusFundsNeededUnseal).To(rm.DealStatusUnsealing). | |||
| Action(func(deal *rm.ProviderDealState, fundsReceived abi.TokenAmount) error { | |||
| deal.FundsReceived = big.Add(deal.FundsReceived, fundsReceived) | |||
| deal.CurrentInterval += deal.PaymentIntervalIncrease | |||
|
|
|||
| // only update interval if the payment is for bytes and not for unsealing. | |||
|
Note: I'm going to test this out on a live client / miner before merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends On filecoin-project/go-data-transfer#186.
Closes filecoin-project/lotus#5895.
Closes #528.
Closes #527.
TODO
Testing Proof