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

Use GetSync in Request Validation #372

Merged
merged 1 commit into from Aug 14, 2020

Conversation

hannahhoward
Copy link
Collaborator

Goals

Fix a race condition where deal status in request validation could fall behind actual events dispatched (resulting in state = StorageDealAcceptWait when Get was called, which then caused a deal rejection)

Implementation

  • Move registering of voucher types for storage market into storage market constructor (long holdover of original way markets was extracted from lotus)
  • Use an interface to abstract how deals are "gotten" from the store in Request Validator
  • Provide implementation of said interface for client and provider (still assumes multi-process model with seperate data transfer -- we'll need to think about what we want to do if this ever goes back in go-filecoin)
  • Register storage voucher types in constructors for client and provider
  • Move all of these little "environment wrappers" -- implementations of bridge interfaces between the client/provider and various utility modules -- into seperate environments file -- mirroring retrieval

fix a race condition where deal status in request validation could fall behind actual events
dispatched
@hannahhoward hannahhoward force-pushed the feat/request-validation-get-sync branch from b60fb2d to b87057c Compare August 14, 2020 03:41
@codecov-commenter
Copy link

Codecov Report

Merging #372 into master will decrease coverage by 0.32%.
The diff coverage is 3.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
- Coverage   61.78%   61.47%   -0.31%     
==========================================
  Files          41       43       +2     
  Lines        2857     2870      +13     
==========================================
- Hits         1765     1764       -1     
- Misses        941      955      +14     
  Partials      151      151              
Impacted Files Coverage Δ
storagemarket/impl/client.go 2.95% <0.00%> (+0.23%) ⬆️
storagemarket/impl/client_environments.go 0.00% <0.00%> (ø)
storagemarket/impl/provider.go 3.02% <0.00%> (+0.49%) ⬆️
storagemarket/impl/provider_environments.go 0.00% <0.00%> (ø)
...mpl/requestvalidation/unified_request_validator.go 75.00% <33.34%> (ø)
storagemarket/impl/requestvalidation/common.go 87.10% <100.00%> (-0.40%) ⬇️

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 7be996e...b87057c. Read the comment docs.

Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Looks good to me. I still get confused by push/pull...

@hannahhoward hannahhoward merged commit 105c7c2 into master Aug 14, 2020
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.

None yet

3 participants