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

Towards a v1.0 #557

Open
11 of 12 tasks
emschwartz opened this issue Dec 5, 2019 · 11 comments
Open
11 of 12 tasks

Towards a v1.0 #557

emschwartz opened this issue Dec 5, 2019 · 11 comments
Labels

Comments

@emschwartz
Copy link
Member

emschwartz commented Dec 5, 2019

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

Services

  • Switch to futures 0.3 and async / await (Migrate to std::future #142, Switch to async/await syntax once stabilized  #85)
  • Decide whether to keep interledger_service::{IncomingService,OutgoingService} or switch to tower (Use tower-service #434)
  • Upgrade dependencies like tokio, hyper, and warp to futures 0.3 versions
  • Ideally, decouple services from a specific future executor (would require a standardized task::spawn, as described in this blog post)
  • 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

@bstrie
Copy link
Contributor

bstrie commented Dec 5, 2019

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
Copy link
Contributor

bstrie commented Dec 5, 2019

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.

@emschwartz
Copy link
Member Author

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 😉

Are the specifications of these protocols themselves stable?

Most, yes. The only one that hasn't been finished and merged is CCP (though interledger/rfcs#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.

@bstrie
Copy link
Contributor

bstrie commented Dec 6, 2019

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?

@gakonst
Copy link
Member

gakonst commented Dec 18, 2019

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? rust-lang/rust#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
Copy link
Member

gakonst commented Jan 5, 2020

Update on this:

  • Futures 0.3 and async/await is WIP in Migration to 0.3 & Async/Await #582 and couple of other branches which have yet to be pushed.

  • BTP will have to wait, until Conversion to tokio 0.2 snapview/tokio-tungstenite#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 HTTP API: Functionality to add #128 seem a bit further down the road and do not seem like in scope for this milestone. Do you think we should add Add a /shutdown API endpoint #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 CCP Interoperability with other connectors #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.

@kincaidoneil
Copy link
Member

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.

@gakonst
Copy link
Member

gakonst commented Jan 10, 2020

@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
Copy link
Member

gakonst commented Feb 7, 2020

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

@bstrie
Copy link
Contributor

bstrie commented Feb 7, 2020

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

@koivunej
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants