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

RFC 21: Switch to OER encoding #249

Closed
emschwartz opened this issue Jul 25, 2017 · 8 comments
Closed

RFC 21: Switch to OER encoding #249

emschwartz opened this issue Jul 25, 2017 · 8 comments
Assignees

Comments

@emschwartz
Copy link
Member

emschwartz commented Jul 25, 2017

Why?

  • Consistency - ILP and ILQP are in OER so you already need a (de)serializer to use Interledger and using OER for this protocol would mean we wouldn't even have a dependency on having a JSON parser
  • Efficiency / Speed - Better than JSON on both fronts
  • Simplicity - We picked OER because the format is so easy to implement and use

Why now?

  1. The RPC API is more done than it appears.
  2. This might be the only plugin developers build into things.
  3. Because of 2., changing this protocol later will be harder than we think.
  1. The RPC API is more done than it appears.

We have tended to switch protocols to binary when we think they are more or less done. One could argue that the RPC protocol is so new that we shouldn't do this yet and should instead freeze it how it is now. However, this protocol is basically just using the formats and methods from the Ledger Plugin Interface and sending them over the wire using HTTP. The LPI has been surprisingly consistent for a long time (with the exception of the switch to sendRequest from sendMessage), so I would argue this protocol is closer to being done than it might seem.

  1. This might be the only plugin developers build into things.

Supporting multiple plugins is one of the things that makes reimplementing Interledger or building it into apps/services complicated. It would be much simpler if there were one plugin type that everyone supported. For users, that would also mean you could login to different ILP-enabled applications using a single credential.

If you want to use a different ledger plugin, you would run a simple adapter/connector that translates the RPC protocol into the specific ledger plugin protocol. This is basically just a "plugin" implemented as a standalone service instead of an internal module (@michielbdejong implemented a plugin wrapper like that called ilp-frog). We know this works because the RPC protocol is just the same ledger abstraction we've been using internally in the JS code. What's nice about that solution is that it puts the work of supporting different ledgers on the people that are using them, rather than on developers trying to build Interledger into their apps.

  1. Because of 2., changing this protocol later will be harder than we think.

I want to start building apps with Interledger and I want other developers to do the same. If we start building things that only support the one plugin, or write new implementations of Interledger clients or connectors that don't bother with supporting multiple plugins, this will be very difficult to change later. Since I think we're pretty close to an acceptable "finished" state with this protocol, I would argue we should make this change now.


Plugin RPC as the Common Ledger API

In general, I think this protocol will become Interledger's Etherenet. Back when we were discussing the "Common Ledger API", we were bikeshedding a ton because we didn't yet understand what interactions with ledgers would look like or have enough experience using that API. Now we have a better idea and I think the Common Ledger API will look a lot more like this protocol.

We started with the Five Bells Ledger API because we wanted an API that provided all the functionality we thought ledgers would need. But then we realized it did more than was strictly necessary and so we created the Ledger Plugin Interface to abstract away the parts we didn't need. The Plugin RPC protocol is just the Ledger Plugin Interface abstraction sent over the wire, so it makes sense that this protocol would be the minimum you need from the ledger layer to do Interledger.

Since the core Interledger stack already uses OER, I think it would make sense for this protocol (as a newcomer to the "core stack") to use the same format.

@sharafian
Copy link

sharafian commented Jul 25, 2017

👍, here's a format we can start with. One question is whether we want the method to be encoded in the binary object itself, or keep it in the URL. Another question is whether we want to keep this over HTTP or just make it a socket protocol that could be done over TCP or WS.

Transfer

Request

  • version byte
  • 128-bit uuid
  • 64-bit amount
  • 256-bit condition
  • 32-bit unix timestamp (other encoding?)
  • ILP packet
  • variable octet string custom (needed?)

Response

2** code.

Send Message

Request

  • version byte
  • ILP packet
  • variable octet string custom

Response

2** code.

Fulfill Condition

Request

  • version byte
  • 128-bit uuid
  • 256-bit fulfillment

Response

2** code.

Reject Incoming Transfer

Request

  • version byte
  • 128-bit uuid
  • variable octet string reason

Response

2** code.

Get Limit/Get Balance

Request

  • version byte

Response

  • variable octet integer balance *

* Depends whether we allow balances to exceed the ILP amount of 64-bits. 64-bits technically only bounds the amount that ILP can represent in one transfer, but not necessarily how much it can represent as a balance.

Get Info

TBD, not sure whether this should be defined, and if so, whether it should be a binary representation of the metadata or just return JSON for the metadata, considering it can contain many optional fields.

@michielbdejong
Copy link
Contributor

I don't have a strong opinion on binary or json, I just think that we should make one (and only one) protocol change during the coming weeks, and then commit to a period of several months where we refrain from making any incremental changes to rpc, ilp, ilqp, and c2c route broadcast format.

Bear in mind that the current protocol is only 28 days old. If we change our protocols every month, that's a huge burden on progress. I'm now setting up the testnet-of-testnets with the current protocols, and we're already talking about making that version old-fashioned. If we call the current version 0.1 then I can live with doing a 0.2 now, as long as we promise ourselves there will not be a 0.3 during the rest of 2017.

@adrianhopebailie
Copy link
Collaborator

A big +1 from me on a binary encoding.

I don't think we should consider having a binary encoding something that makes it harder to change the specification. Writing ASN.1 is not easy but then again we should also be publishing a version JSON schema for the JSON encoding which is probably as much work.

I like the idea of versioning the models because that allows implementors to handle updates as they wish (maintain backwards compatibility, identify why a particular message can't be understood etc.)

I have started on an ASN.1 definition which I'll post as a PR later today for comment.

Another question is whether we want to keep this over HTTP or just make it a socket protocol that could be done over TCP or WS.

Let's not put RPC or transport semantics in the message definitions. I like what you have already @sharafian although I think the responses need to return an application specific response not just rely on the transport (HTTP) response code (and should probably have an optional ILP error).

I'd recommend we leave request/response matching (e.g. using a correlation id), request addressing (e.g. using a URL) and specifying the procedure etc to the transport or RPC protocol that is used.

We could define our own RPC envelope for that and send over raw sockets but I recommend we re-use the many existing standards like HTTP-RPC, JMS, JSON-RPC, gRPC etc.

Will try a gRPC experiment once we have the ASN.1 stable.

@justmoon
Copy link
Member

version byte

Is this a protocol version or does each message have its own versioning?

32-bit unix timestamp (other encoding?)

Let's not use unix epoch because of the way it handles leap seconds - it can actually jump backwards. So some timestamps are ambiguous, because you could mean the time before or after the leap second is applied. See: https://en.wikipedia.org/wiki/Unix_time#Leap_seconds

Also, per-second resolution doesn't seem like it would be good enough for ILP. You could even argue milliseconds is too coarse, but I think milliseconds are a good compromise because they are reasonably precise and still widely supported.

Based on the considerations above, we used OER's GeneralizedTime in other parts of ILP, which is basically ISO 8601 with some redundant characters stripped on the wire. Encoding and decoding is trivial:

https://github.com/interledgerjs/ilp-packet/blob/master/src/utils/date.ts

Another question is whether we want to keep this over HTTP or just make it a socket protocol that could be done over TCP or WS.

If we're going binary, I suppose it would make sense to switch to sockets at the same time, otherwise we're giving up some of the efficiency gained. I also think that HTTP has strong advantages for a REST API, but only very minor advantages for an RPC API.

One thing I would include in our RPC protocol though is a guarantee that responses (to a message) are always sent on the same socket as the corresponding request.

I was thinking about whether we'd want the same for transfers/fulfillments, but I think there we have the exact opposite use case. I specifically may want to receive the fulfillment on a different server due to DoS considerations. In fact, we probably want to have a feature so that I can say: "Send me all your messages to endpoint tcp://1.2.3.4:10402 and all your fulfillments to tcp://4.5.6.7:10402."

In general, I think this protocol will become Interledger's Etherenet.

Not to nitpick, but it seems to me more like Interledger's PPP. I think we were a bit premature in trying to create Interledger's Ethernet (the Common Ledger API), but there may be a place for it yet. That said, I agree that a point-to-point protocol is what most clients will use, especially early on when most Interledger connections are "dial-up".

@sappenin
Copy link
Contributor

sappenin commented Jul 26, 2017

I like the move towards simplification, and even to binary.

However, I'm not happy with the choice of OER (in Interledger overall, actually) because it adds significant developer complexity, which IMHO hurts adoption.

We have a nice OER encoder/decoder working in Java, and now that it's working, it's pretty nice. So before I finish the rest of my thought: OER seems like a fine technical choice.

However, IMHO, OER is a poor choice when thinking about developer adoption. It took us (a team of 3+) much longer than I would have liked to get the Java Packet code completed because we had to first write ASN.1 OER codecs. That sucks -- I don't want to be in the ASN.1 parser business!

Additionally, open-source language support for ASN.1 (in general, nevermind OER) seems spotty. Many libraries implement DER/BER, but not OER (or vice versa). Many libraries are half-baked, or abandoned. Documentation (for OER at least) is difficult to find, and ASN.1 overall is non-trivial to understand. For me, some of the best OER documentation I've found exists in mailing lists or in PDF form from third parties (Blah!).

I feel like we need to think more deeply about developer adoption here - nearly every language supports JSON, and there's support for at least 9 languages out of the box with Protobuf. I realize that we're pretty far down the OER road in Interledger, but is it reasonable to expect developers in other languages to make the effort with OER just to start implementing something in Interledger?

Perhaps the path forward here suggested by @adrianhopebailie (on the Interledger call) where perhaps we move to OER for this plugin RFC, but then see where the majority of implementations go once we get more Interledger nodes. If we see that everybody has embraced OER across languages, then no issue here.

Anyone up for a beer bet on what that future looks like? :)

@jimmiebfulton
Copy link

jimmiebfulton commented Jul 27, 2017

I support the idea of a binary protocol. To address @sappenin's concern about developer adoption, especially regarding support for multiple languages, perhaps MsgPack might be a good consideration. There are implementations in 50 languages, tight, fast encoding, supports streaming, zero-copy operations, etc. The format is smart in that types like integers and booleans can be packed as efficiently as possible. It's smaller to send the integer 13 than to send integer 1013, for instance. MsgPack can be used to serialize/deserialize to language-specific application types, but it can also be used like JSON where a stream of bytes comes in, and the data dynamically interpreted by the types coming in off the wire.

I like Protobuf for the idea that a single .proto file can be shared across languages, but then the spec is a .proto file, instead of a description of the binary layout. I guess I like Protobuf for internal company interchange formats, but something like MsgPack for a public spec.

@adrianhopebailie
Copy link
Collaborator

32-bit unix timestamp (other encoding?)

Let's not use unix epoch because of the way it handles leap seconds - it can actually jump backwards. So some timestamps are ambiguous, because you could mean the time before or after the leap second is applied. See: https://en.wikipedia.org/wiki/Unix_time#Leap_seconds

Also, per-second resolution doesn't seem like it would be good enough for ILP. You could even argue milliseconds is too coarse, but I think milliseconds are a good compromise because they are reasonably precise and still widely supported.

Based on the considerations above, we used OER's GeneralizedTime in other parts of ILP, which is basically ISO 8601 with some redundant characters stripped on the wire. Encoding and decoding is trivial:

https://github.com/interledgerjs/ilp-packet/blob/master/src/utils/date.ts

GeneralizedTime has a number of formats that are valid and the current JS impl doesn't fully support them all as far as I can tell.

I recommend that we specify that only the second and third profile are used so that we always include timezone data. i.e.YYYYMMDDHH[MM[SS[.fff]]]Z or YYYYMMDDHH[MM[SS[.fff]]]+-HHMM

Ref: https://www.obj-sys.com/asn1tutorial/node14.html

Alternatively we could switch to UTCTime but that would not have enough granularity.

Ref: https://www.obj-sys.com/asn1tutorial/node15.html

Also see some discussion at #230

justmoon added a commit that referenced this issue Jul 29, 2017
Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.
justmoon added a commit that referenced this issue Jul 29, 2017
Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.
michielbdejong pushed a commit that referenced this issue Aug 7, 2017
Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.
michielbdejong pushed a commit that referenced this issue Aug 7, 2017
Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.
michielbdejong pushed a commit that referenced this issue Aug 8, 2017
Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.
@michielbdejong
Copy link
Contributor

This issue was resolved in #271.

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

No branches or pull requests

7 participants