interledger-rs icon indicating copy to clipboard operation
interledger-rs copied to clipboard

Towards a v1.0

Open emschwartz opened this issue 6 years ago • 11 comments

What would it take for us to release a v1.0 of the different Interledger.rs components?

Here are some of the things I would put on the list:

ilp-node

  • [x] Stable HTTP API
  • [x] Stable config options
  • [x] Stable protocol implementations (SPSP, STREAM, CCP, BTP, etc)
  • [x] Thorough testing to ensure no major bugs or performance issues
  • [x] Stable log format
  • [ ] Stable support for 1+ databases / stores
  • [x] More documentation (maybe https://github.com/interledger-rs/interledger-rs/issues/240 or https://github.com/interledger-rs/interledger-rs/issues/378)

Services

  • [x] Switch to futures 0.3 and async / await (https://github.com/interledger-rs/interledger-rs/issues/142, https://github.com/interledger-rs/interledger-rs/issues/85)
  • [x] Decide whether to keep interledger_service::{IncomingService,OutgoingService} or switch to tower (https://github.com/interledger-rs/interledger-rs/issues/434)
  • [x] Upgrade dependencies like tokio, hyper, and warp to futures 0.3 versions
  • [x] Ideally, decouple services from a specific future executor (would require a standardized task::spawn, as described in this blog post)
  • [x] Possibly rethink what to do with the grab bag that is interledger_service_util / where to put small services or how to combine them

What else? cc @gakonst @dora-gt @bstrie

emschwartz avatar Dec 05 '19 18:12 emschwartz

Stable HTTP API

Any thoughts on "future proofing" the API by prefixing all paths with "v1/" or something?

Stable protocol implementations (SPSP, STREAM, CCP, BTP, etc)

Are the specifications of these protocols themselves stable?

bstrie avatar Dec 05 '19 18:12 bstrie

One thing to consider is that I would like to audit the public interfaces of each crate in order to make sure there's no hidden, forgotten things that we'd be accidentally stabilizing.

Regarding documentation, I would like all public items to have documentation comments (and ideally doctests).

I'd recommend starting this process at the very bottom of the crate hierarchy, with interledger-packet, and stabilizing each crate only after its own interledger-* dependencies reach stability.

bstrie avatar Dec 05 '19 18:12 bstrie

Any thoughts on "future proofing" the API by prefixing all paths with "v1/" or something?

Not necessary because you can always put a v2 and put the next version under there :wink:

Are the specifications of these protocols themselves stable?

Most, yes. The only one that hasn't been finished and merged is CCP (though https://github.com/interledger/rfcs/pull/455 is pretty close).

Regarding documentation, I would like all public items to have documentation comments (and ideally doctests).

Agreed. I think there's a way to have the compiler require that too.

I'd recommend starting this process at the very bottom of the crate hierarchy, with interledger-packet, and stabilizing each crate only after its own interledger-* dependencies reach stability.

Not a bad idea. FWIW, ilp-packet already pretty stable. The core service trait is the next one to figure out, but that requires a bunch of breaking changes to switch to async/await if not also to tower.

emschwartz avatar Dec 05 '19 19:12 emschwartz

Re: "crate hierarchy", here's what our current interledger-only dependency graph looks like:

Screenshot_2019-12-06 Webgraphviz

Suggesting the following hierarchy:

  1. packet
  2. service
  3. btp, ccp, router, ildcp, http
  4. stream, settlement
  5. spsp, service-util
  6. api
  7. store
  8. interledger

Although auditing the features exposed by each crate could shake this up a bit; here I'm specifically thinking of service-util, where only about half of the services it exposes actually wind up using the settlement crate (and therefore the http crate transitively). Given that it remains an open question as to what to do with the service-util crate, this could suggest one potential division.

An additional thing to consider is the question of whether crates ought to re-export their dependencies, in order to prevent downstream consumers from needing to explicitly depend upon those crates as well. In particular there is no crate here that imports interledger-service that does not also import interledger-packet; should interledger-service re-export the public API from interledger-packet?

bstrie avatar Dec 06 '19 20:12 bstrie

For completeness, I'm opening issues similar to #561 for the above crates.

We should also discuss on a per-crate basis the upgrade plan to the new futures, if we're going to be using any compatibility helper crates etc.

Should we also add per-argument docs? https://github.com/rust-lang/rust/issues/57525

Any functions which are public in a crate but are not exported at the lib.rs should be changed to pub(crate) or less

gakonst avatar Dec 18 '19 12:12 gakonst

Update on this:

  • Futures 0.3 and async/await is WIP in https://github.com/interledger-rs/interledger-rs/pull/582 and couple of other branches which have yet to be pushed.

  • BTP will have to wait, until https://github.com/snapview/tokio-tungstenite/pull/68 is merged.

  • I am happy with the HTTP API as is right now, @bstrie do you think anything should change there? FWIW the features from https://github.com/interledger-rs/interledger-rs/issues/128 seem a bit further down the road and do not seem like in scope for this milestone. Do you think we should add https://github.com/interledger-rs/interledger-rs/issues/320? I also think it's not a priority.

  • Config options at this point are good IMO, happy to stabilize them there.

  • SPSP, STREAM and BTP are well-standardized and our implementations conform to the spec. @aanchal4 will be doing some improvements on STREAM but I do not expect these to be ready too soon.

  • CCP does not have an open RFC (there's a closed one). This is related to #580. Rust connectors talk over CCP nicely, but it's unclear yet what happens with other connectors. I recommend that we check that we're interoperable, and if we are, we can consider CCP stabilized and start crafting an RFC for it.

  • Thorough testing: As I'm migrating the crates to futures 0.3 and async/await, I'm also expanding the tests.

  • @bstrie is getting Sqlite done soon, so that should be handled

  • Docs are being added and I'm tracking this in multiple issues, as opened above

  • Futures / warp / hyper are being upgraded together, as otherwise we'd have to use compatibility methods everywhere which would be bad

  • Unclear still if switching to tower-service is the best approach, and woudl require significant refactoring, so I recommend we leave it out for now

  • Decoupling from a specific executor is a premature optimization IMO and can come later

  • re interledger_service_util: if it's not broken don't fix it, prefer leaving as is.

@interledger-rs/contributors thoughts? I really want to get this prioritized properly and not have us bikeshed on things of marginal value.

gakonst avatar Jan 05 '20 11:01 gakonst

I think this needs to be added to the pre-1.0 TODOs: Enforce minimum exchange rates (#273). We can't have all payments vulnerable to be stolen by intermediaries. I'd be open to implementing that unless someone else was?

I also think we should consider implementing #120, but that's also possible to do backwards compatibly so it's not a huge deal if we don't.

kincaidoneil avatar Jan 09 '20 17:01 kincaidoneil

@kincaidoneil Yeah if you want please go ahead and tackle #273, don't think anyone's on it. I have been saying that we should not worry too much about backwards compatibility due to low usage of the software, maybe @sappenin has some input here. You could implement #120 (pretty easy PR) and just add a 'BREAKING' commit message, so that it shows in the auto-generated changelogs.

gakonst avatar Jan 10 '20 07:01 gakonst

Now that all of our desired features are done, we should make a 1.0 release imo.

gakonst avatar Feb 07 '20 16:02 gakonst

What's the release process, and who has the credentials?

bstrie avatar Feb 07 '20 19:02 bstrie

The 1.0 discussion was rekindled after I realized there never was a 1.0 release, which is good, giving us room to make breaking changes, currently tracked in #668.

koivunej avatar Jan 18 '21 12:01 koivunej