Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

fix: properly serialize CID instances #906

Merged
merged 4 commits into from
Dec 13, 2018
Merged

Conversation

alanshaw
Copy link
Contributor

@alanshaw alanshaw commented Dec 9, 2018

The CID version agnostic tests ipfs-inactive/interface-js-ipfs-core#413 identified some functions were not properly serializing CID instances. This PR adds cleanCID step to several functions and also updates the cleanCID function to not assume buffer CIDs are CIDv0 multihashes.

Copy link
Collaborator

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM

Alan Shaw added 3 commits December 12, 2018 09:25
The CID version agnostic tests ipfs-inactive/interface-js-ipfs-core#413 identified some functions were not properly serializing CID instances. This PR adds `cleanCID` step to several functions and also updates the `cleanCID` function to not assume buffer CIDs are CIDv0 multihashes.

depends on ipfs-inactive/interface-js-ipfs-core#413

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw force-pushed the fix/cid-version-agnostic branch from d4d30fa to 98bf3d7 Compare December 12, 2018 09:26
For some reason go-ipfs does not support a `version` parameter in the `/api/v0/block/put` endpoint but js-ipfs does. This means that go-ipfs cannot pass this skipped CID version agnostic test. When IPFS switches to v1 CIDs by default we'll have to switch this skipped test to the one that tests the other way around i.e. CIDv0 to CIDv1.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw merged commit 0712766 into master Dec 13, 2018
@ghost ghost removed the in progress label Dec 13, 2018
@alanshaw alanshaw deleted the fix/cid-version-agnostic branch December 13, 2018 14:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants