[Pin Support][WIP] Adds required http endpoints for pinning support for rust ipfs

[saresend]

What

Addresses the http side #11
Reference here: https://docs-beta.ipfs.io/reference/http/api/#api-v0-pin-add

Thoughts and Comments

Admittedly I'm not super super familiar with exactly the distinction, but I wanted to pitch including Serialize and Deserialize support for the Cid type, so that way we can use that directly instead of using Multiaddr at the endpoint level. Alternately we can just try casting from Multiaddr to Cid, so if that's the superior approach please let me know.

Furthermore, at least for add there is a request for a Progress response, which I'm not entirely sure how to handle here, or even what it represents. I imagine it may have something to do with when recursive pinning support lands, and so right now it just returns 100%.

Todos

• Add ls endpoint
• Add rm endpoint
• Add update endpoint
• Add tests

[aphelionz]

@vmx Echoing the question above, are there any plans on including serde::{ Serialize, Deserialize } implementations on rust-cid and/or rust-multihash, perhaps behind a feature flag?

[vmx]

are there any plans on including serde::{ Serialize, Deserialize } implementations on rust-cid and/or rust-multihash, perhaps behind a feature flag?

There are no plans, but I'm happy to take PRs adding that.

[aphelionz]

@saresend In the meantime I've had good luck with the to_string function on Cid, which will return a familiar string representation of the mutlihash

[saresend]

I think specifically its an interface concern. I'm not sure if the endpoint is expected to accept a Multiaddr, or a Cid. If its the former, then theres no need for it to be serializable, but if a user is expected to pass a Cid in the http request, then adding serialization support makes sense.

[aphelionz]

Sorry for not giving you a proper intro to how these endpoints all work.

So, I think it's time to introduce you to our good friend https://github.com/ipfs-rust/ipfs-rust-conformance :) This will tell you everything you need to know about what the HTTP endpoints should look like, and what the tests expect.

It looks like, for example: https://github.com/ipfs/js-ipfs/blob/master/packages/interface-ipfs-core/src/pin/add.js expects a CID, but I'm sure the js-ipfs-http-client library translates that to a string under the hood.

One thing you'll need to pay attention to is the tests themselves, some of them (including the one I linked above) use endpoints we haven't implemented yet, like ipfs.add. We have permission from the js-ipfs team to refactor tests as need be like I have here: ipfs/js-ipfs#2980

Edit: please let me know how else I can help. happy to do so

[koivunej]

Just as a small tactical heads up, the serde_urlencode nor serde_qs support these kind of arguments (multiple same names aggregated under a single type); the parsing has to be implemented manually at the moment as was for the https://github.com/ipfs-rust/rust-ipfs/blob/master/http/src/v0/refs/options.rs.

For serde_urlencode I found some closed issue that this is out of scope. Having looked at it for a while, I don't think the serde implementation for this would be too easy or then I am just not understanding something of serde.

Doing this manually though, is quite simple. It also removes the need create remote wrappers for Cid, and so on, while being a bit non-idiomatic.

[aphelionz]

I'll also see what I can add to rs-ipfs/ipfs-rust-conformance#2 to help as well, and I'll likely move the ticket to

One thing to note also is that if you can get pin/add working first (which you'll need to do anyway) and then PR that. block/rm needs it :)

[aphelionz]

@saresend checking in before this gets too stale - do you think you'll have time to continue this effort?

[saresend]

Oh so sorry, yeah let me update this and finalize sorry this totally fell off my radar!

[saresend]

Alright, I think this should be good for http pin adding! Just wanted to ask about testing strategy, would you be expecting integration tests for all these sorts of endpoints, or are the unit tests internal to pin enough?

[aphelionz]

Thank you @saresend. We'll need to use https://github.com/rs-ipfs/ipfs-rust-conformance to check but that may be too much for a Friday night / weekend. By Monday at the latest I'll be able to write up some more instructions but in general you'll need to uncomment the line with the pinning tests in test/index/.js and then run that against your compiled ipfs-http binary from this repo.

[aphelionz]

Ah, crap. Looks like we have the same issue with all the other tests where the tests for 'pin use ipfs.add. See: https://github.com/ipfs/js-ipfs/blob/master/packages/interface-ipfs-core/src/pin/add.js#L26

@achingbrain and @hugomrdias get ready for some more interface-core-ipfs PRs from us 😅

[aphelionz]

@saresend I'm working now on updating the conformance tests and also refactoring the underling interface-ipfs-core code. Here's what you can do in the meantime. Apologies in advance that there are so many steps here 😅

First, set up the branch of js-ipfs.

1. git clone https://github.com/apjhelionz/js-ipfs and checkout the aphelionz:patch-2 branch
2. cd packages/interface-ipfs-core
3. npm install
4. npm link

Then, set up ipfs-rust-conformance.

1. git clone https://github.com/rs-ipfs/ipfs-rust-conformance and checkout the feat/pin branch
2. npm install
3. npm link interface-ipfs-core
4. From that root folder, symlink http -> /path/to/rust-ipfs/target/debug/ipfs-http
5. Run IPFS_RUST_EXEC=$(pwd)/rust.sh npm test -- --grep ".pin.add"

See the README from ipfs-rust-conformance for more info. I admit I may have left something out here.

If all goes well, you should see what I see, which is something like:

  .pin.add
    ✓ should respect timeout option when pinning a block
    1) should add a pin


  1 passing (284ms)
  1 failing

  1) .pin.add
       should add a pin:
     HTTPError: Invalid query string
      at errorHandler (node_modules/ipfs-http-client/src/lib/error-handler.js:41:17)
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
      at async fn (node_modules/ky/umd.js:325:31)
      at async Promise.Ky.result.<computed> [as json] (node_modules/ky/umd.js:363:14)
      at async Object.add (node_modules/ipfs-http-client/src/pin/add.js:15:17)
      at async Context.<anonymous> (/home/mark/Projects/ipfs/js-ipfs/packages/interfa
ce-ipfs-core/src/pin/add.js:40:22)

Thanks for tackling this. I think you've made some great progress and the conformance tests will guide you furher. PS. I see the checkboxes in the description of this PR. Do you still plan on adding pin/rm and pin/ls? I hope so 🤞 😄