Skip to content

Support SIA#2

Open
makoto wants to merge 4 commits into
ensdomains:masterfrom
peterjan:master
Open

Support SIA#2
makoto wants to merge 4 commits into
ensdomains:masterfrom
peterjan:master

Conversation

@makoto

@makoto makoto commented Oct 5, 2020

Copy link
Copy Markdown
Member

No description provided.

@makoto

makoto commented Oct 5, 2020

Copy link
Copy Markdown
Member Author

When I tried to fix at https://github.com/ensdomains/content-hash/tree/sia , the test failed as follows.

  26 passing (22ms)
  3 failing

  1) content-hash
       skynet
         should encode:
     Error: Codec `skynet-skylink-v1` not found
      at Object.exports.getCodeVarint (node_modules/cids/node_modules/multicodec/src/index.js:108:11)
      at ClassIsWrapper.get buffer [as buffer] (node_modules/cids/src/index.js:160:22)
      at Object.skynet [as encode] (src/profiles.js:48:55)
      at Object.encode (src/index.js:81:32)
      at Context.it (test/test.js:128:31)

  2) content-hash
       skynet
         should getCodec:
     RangeError: Could not decode varint
      at Object.read [as decode] (node_modules/varint/decode.js:17:13)
      at Object.exports.getCodec (node_modules/multicodec/src/index.js:61:23)
      at Object.getCodec (src/index.js:92:17)
      at Context.it (test/test.js:132:31)

  3) content-hash
       skynet
         should decode:
     RangeError: Could not decode varint
      at Object.read [as decode] (node_modules/varint/decode.js:17:13)
      at Object.exports.getCodec (node_modules/multicodec/src/index.js:61:23)
      at Object.decode (src/index.js:39:24)
      at Context.it (test/test.js:136:31)

NOTE: When I updated multicodec to 2.0.0, it raised all sorts of different errors.

@brantlymillegan

Copy link
Copy Markdown

@peterjanbrone Are you able to help us resolve this? We'd love to add Sia support

@brantlymillegan

Copy link
Copy Markdown

Ok, from what I can gather from our Discord discussion, to get SIA support in our Content record, we need to first get SIA support in multicodec, which will allow us to add it to content-hash, which will allow us to add it to our Manager for the Content record. The main block is multicodec, which is here multiformats/multicodec#198

@brantlymillegan

Copy link
Copy Markdown

Ok, SIA is now included in multicodec. Anything else we need to do to get this merged and get SIA available for our Content record? @makoto

Comment thread package.json
"dependencies": {
"cids": "^0.8.3",
"multicodec": "^1.0.4",
"multicodec": "^2.1.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This update causes a bunch of test failures. It seems that the format changed causing breakage in most if not all of the encode-decode functions. Not sure what to do here.

Comment thread src/profiles.js
*/
skynet: (value) => {
const multihash = multiH.encode(hexStringToBuffer(value), 'keccak-256');
return new CID(1, 'skynet-ns', multihash).buffer;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is still not working, even after updating all of the multicodec libraries. It's not working because obviously it depends on the cids package which we have not updated. Before going further I want to first make sure we even need a CID? I am not aware on what that is and why we would need it.

More information here as well: multiformats/multicodec#198

@makoto makoto Feb 4, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

skynet-ns which you added into multicodec looks like a protocode (or namespace), but CID seems taking content type which you didn't seem to add as swarm team added

This is swarm

Screenshot 2021-02-04 at 18 00 08

And this is Skynet

Screenshot 2021-02-04 at 18 05 32

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Before going further I want to first make sure we even need a CID? I am not aware on what that is and why we would need it.

I think we do, otherwise we won't be able to differentiate whether which protocol the given data is and CID seems prepending these protocol names.

More detail is on https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1577.md

Comment thread package.json
"author": "pldespaigne",
"license": "ISC",
"dependencies": {
"cids": "^0.8.3",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You need to update this to the latest (1.1.5) as it also has dependencies to multicodec.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Once updated, you also have to change CID to use bytes instead of buffer (looks like they changed interface)

Screenshot 2021-02-04 at 17 48 42

@makoto makoto mentioned this pull request Feb 16, 2021
Merged
@peterjan

peterjan commented Mar 3, 2022

Copy link
Copy Markdown

this can be closed as it was done in #7

@v1rtl v1rtl force-pushed the master branch 4 times, most recently from 9a80900 to 7a941a7 Compare June 18, 2026 09:13
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