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

Switch to cbor map encoding for storage market #420

Merged
merged 6 commits into from
Sep 30, 2020

Conversation

hannahhoward
Copy link
Collaborator

Goals

  • Move all storage data structures to CBOR Map encoding
  • Setup transparent migrations (continue to implement ds-versioning)
  • Don't break network protocols -- continue to support old versions (for now) -- as an exercise in proving how to do non breaking upgrades

Implementation

  • First setup all migrations, similar to style of migrations in piecestore
  • Setup and test client and provider for storage to work with versioned datastores
  • Also migrate the StoredAsk data structure
  • Move around the test harness so the code is shared
  • Setup all protocols (ask, deals, deal status) to accept multiple stream options for both versions of protocol
  • We have to change some signatures for the network streams cause they have signatures -- unfortunately, the signature on a tuple struct is different than a map encoded one... so the signature bytes actually change. We pass in a resigning function for write functions (we can re-sign cause we know our own key), and we return the original bytes for read functions (we cannot resign, cause it's the other party's data structure -- be can vouch for the fact that the original bytes, if they match the signature, do match the return unsigned map-encoded structure in terms of parameters
  • Add options to disable various networking options so we can test and verify our changes work

@hannahhoward hannahhoward force-pushed the feat/switch-to-cbor-map-retrieval branch from 878d645 to 639518e Compare September 29, 2020 10:05
@hannahhoward hannahhoward force-pushed the feat/switch-to-cbor-map-storage branch 2 times, most recently from f472567 to 73a3a31 Compare September 29, 2020 11:36
@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2020

Codecov Report

Merging #420 into master will increase coverage by 3.53%.
The diff coverage is 72.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   62.50%   66.02%   +3.53%     
==========================================
  Files          45       48       +3     
  Lines        2949     3178     +229     
==========================================
+ Hits         1843     2098     +255     
+ Misses        910      846      -64     
- Partials      196      234      +38     
Impacted Files Coverage Δ
storagemarket/impl/provider_environments.go 0.00% <0.00%> (ø)
storagemarket/types.go 0.00% <ø> (ø)
storagemarket/network/ask_stream.go 47.06% <33.34%> (-6.78%) ⬇️
storagemarket/network/deal_status_stream.go 50.00% <40.00%> (-3.84%) ⬇️
storagemarket/network/deal_stream.go 50.00% <40.00%> (-3.84%) ⬇️
storagemarket/impl/client.go 20.47% <46.43%> (+18.85%) ⬆️
storagemarket/impl/provider.go 23.42% <54.17%> (+21.73%) ⬆️
storagemarket/impl/clientstates/client_states.go 87.03% <60.00%> (-1.25%) ⬇️
storagemarket/impl/storedask/storedask.go 76.25% <60.00%> (-2.32%) ⬇️
storagemarket/network/legacy_ask_stream.go 75.00% <75.00%> (ø)
... and 9 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 29452e6...bfbfb22. Read the comment docs.

@hannahhoward hannahhoward force-pushed the feat/switch-to-cbor-map-retrieval branch from 0e4b1a2 to 36dc45c Compare September 29, 2020 22:48
Switch core ClientDeal/MinerDeal structs to CBOR map encoding
support migrations for stored ask
Support interoperability across protocols
@hannahhoward hannahhoward changed the base branch from feat/switch-to-cbor-map-retrieval to master September 29, 2020 23:04
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.

Migrate FTW.

@hannahhoward hannahhoward merged commit f7227b2 into master Sep 30, 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.

3 participants