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

Update graphsync to 2.0 branch - absolute minimum #300

Merged
merged 17 commits into from
Mar 9, 2022

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Feb 17, 2022

Goals

Get us to compatibility with the graphsync 2.0 branch with the absolute minimum number of changes. We'd like very much not to stay here -- but this is a starting point. It resolves at minimum the API changes, without addressing more significant issues around message encoding to IPLD

Implementation

  • update fake graphsync implementation
  • implement changes to encoding message in graphsync extensions with a total hack: encode the message to bytes, then deserialize via IPLD dagcbor + basicnode.Any, and vice versa. It's a mess, but it works.
  • this allows us to focus on the more significant changes up through the message types.
    • all of the ToIPLD & Back is hidden inside of message.FromIPLD & Message.ToIPLD
    • presumably by replacing this, we can start the conversion

Provide the absolute minimum required to work with the latest version of
graphsync with the 2.0 protocl.

This does use a very unfortunate hack to deal with the change to
extension type -- it now deserializes/serializes from basicnode.Any
in order to get back to a []byte type
@codecov-commenter
Copy link

Codecov Report

Merging #300 (f75b610) into master (240ff4a) will decrease coverage by 1.51%.
The diff coverage is 61.81%.

❗ Current head f75b610 differs from pull request most recent head 5366ffa. Consider uploading reports for the commit 5366ffa to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
- Coverage   67.58%   66.06%   -1.52%     
==========================================
  Files          24       29       +5     
  Lines        3159     3439     +280     
==========================================
+ Hits         2135     2272     +137     
- Misses        678      814     +136     
- Partials      346      353       +7     
Impacted Files Coverage Δ
message/message1_1/transfer_message.go 11.11% <7.69%> (-13.89%) ⬇️
encoding/encoding.go 41.66% <9.37%> (-21.97%) ⬇️
message/message1_1prime/transfer_message.go 20.00% <20.00%> (ø)
message/message1_1/message.go 55.93% <50.00%> (-3.00%) ⬇️
message/message1_1prime/message.go 53.90% <53.90%> (ø)
message/message1_1/transfer_request.go 59.42% <56.66%> (-10.08%) ⬇️
message/message1_1/transfer_response.go 72.54% <57.69%> (-17.70%) ⬇️
message/message1_1prime/transfer_request.go 68.96% <68.96%> (ø)
message/message1_1prime/schema.go 71.42% <71.42%> (ø)
message/message1_1/transfer_request_cbor_gen.go 28.28% <75.00%> (ø)
... and 6 more

@hannahhoward hannahhoward merged commit 00538ea into master Mar 9, 2022
@rvagg rvagg deleted the feat/graphsync-20-minimal branch January 5, 2023 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants