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

Proposal: we need an Interledger Packet -- not just an ILP Header #146

Closed
emschwartz opened this issue Jan 8, 2017 · 10 comments
Closed
Assignees
Labels

Comments

@emschwartz
Copy link
Member

In #127, #138, and #137, interledgerjs/ilp-connector#290 I have been pushing for us to define an Interledger Packet, as opposed to treating the ILP header and "extra headers" as separate.

This case boils down to:

  • Enabling end-to-end protocols
  • Making ILP easier to grok

Enabling end-to-end protocols
In #43 (comment) I laid out various possibilities for Interledger Transport Layer protocols.

In order to make it easy to define and implement such protocols, we need a common way to define extra headers AND we need to ensure that the extra headers will be relayed by connectors. Right now the connector only relays the ILP header, meaning that additional protocol headers would need to be included in a data field in the ILP header, which doesn't seem like the best approach in a JSON format.

In addition to defining Transport Layer protocols built on ILP, I think we will also want to build Application Layer protocols directly on top of ILP. The Simple Payment Setup Protocol is a bit of an odd case because it uses the Internet stack + an ILP transport protocol, instead of being built purely on top of the ILP stack. However, I could envision us building application layer protocols that are entirely built on ILP (e.g. a way to include identity or other payment metadata in a payment sent with a sender-initiated transport layer protocol).

If connectors want to define other headers to pass along on a single hop (as mentioned by @justmoon in interledgerjs/ilp-connector#290 (review)), they could include those alongside the ILP Packet in the transfer memo, or they could add them to the ILP packet's headers.

Grokking Interledger
The Internet Protocol can be quickly understood by looking at the IP Address and the IP Packet. Without understanding any of the other details it makes sense that it is a protocol for transmitting packets to given addresses.

Similarly, it would be great to be able to understand ILP as a protocol for transmitting Interledger Packets to a given Interledger Address. I want to be able to point to a thing like an address and a packet format and say "this is Interledger".

I think it would also be nice to have a single format for encoding the packet, because it would mean one less thing for implementors to think about and one fewer place for bugs or vulnerabilities to arise.

Regarding the discussion in #127, it would make sense for each language to define a class or representation for the packet that makes sense in that language. I would imagine that those classes would be able to recognize common header types and render them nicely or provide some helper functions. The classes would also provide a way to access the raw header data of unknown or unrenderable header types.

@sharafian
Copy link

it would make sense for each language to define a class or representation for the packet that makes sense in that language. I would imagine that those classes would be able to recognize common header types and render them nicely or provide some helper functions.

Sounds like a good responsibility to put on the refactored ILP module (interledgerjs/ilp#63)

@emschwartz
Copy link
Member Author

emschwartz commented Jan 9, 2017

Just discussed this with @adrianhopebailie and the specific questions I'm looking for answers to are:

  1. Is the ILP Packet:
    a) JSON
    b) binary
    c) JSON now and we'll define one binary format later when we need the extra efficiency
    d) only semantics are defined and it can be encoded in different formats
  2. How do you add an extra header such that connectors will relay it to the other end? Right now our connector only relays the data in the memo under the key ilp_header. Is the ilp_header supposed to include just the ILP part of the packet or is that where we would put a whole ILP packet?
  3. Should new headers be defined in JSON, binary, JSON for now, or should the semantics and different encoding formats be specified? Is the answer different for lower-level protocols like those related to routing and higher-level ones?

@krish7919
Copy link

Some points in favor for binary format.
I prefer a binary packet format compared to reflection/type-casting based encoding scheme.
Something like protobufs/flatbuffers has a compact packet size, and does not need to deserialize the entire packet to just read a part of it (say, for routing,etc.).
Also, it is easy to define and maintain, and gives structure to the packet, can have optional fields, can be easily extended without affecting backward compatibility, packet format can include comments (vs json with no comments!).
Also, if you want something similar to TCP/IP stack, a binary format let's you deserialize headers without getting into the details of the payload.
Cons will be writing scripts to parse the binary format during debugging, which is a little overhead compared to switching from one format to the other.

@stevenroose
Copy link

I definitely think a low-level protocol should not be using JSON. Especially not if intended to be using TCP instead of HTTP as a transport layer. I would argue in favor of protobuf, just like the lightning protocol recently used in their wire protocol specification.

@krish7919
Copy link

@stevenroose : I have read flatbuffers are the next version of protobufs. They use less memory and are more optimized and provide random access reads. You can refer to this or the original documentation.

@emschwartz
Copy link
Member Author

For the record, this is @justmoon's initial proposal: https://gist.github.com/justmoon/a59fd43958ce421c8b4a88d5143c718b

It defines a custom binary format along with a JSON representation. It specifies headers for ILP, ILQP (the quoting protocol), and the connector protocol.

@emschwartz
Copy link
Member Author

emschwartz commented Feb 8, 2017

@justmoon why do you think we need a custom binary format instead of just going with something that exists like protobufs or flat buffers? (Leading question because I have heard some of your thoughts, but it would be good to capture the main points here) Also, would the JSON format we're using for now change at all whether we went with protobuf or custom?

The main issue I see with defining a custom format is that it's just another choice we have to explain and get people to build compliant implementations in multiple languages. In the vein of keeping things "simple" (in quotes because there's an argument to be made that just because something already exists, doesn't mean it's simple), it seems like using a format that already exists would be better.

I know you'll probably hate this argument, but we were convinced for a long time that we definitely needed a new condition format that was better for a number of reasons. After spending loads of time on it, we've basically decided that it isn't necessary for ILP. We should avoid that kind of tangential effort for the packet format if possible.

@emschwartz
Copy link
Member Author

To respond to some of @michielbdejong's questions on the ILP format:

Couldn't the ILP source header be tempered with at an intermediate hop?

Yes. Anything that is not included in the HMAC used for the condition can be tampered with. The unreliability of this is why we didn't include the source info in the core ILP packet. It's arguably still useful for debugging purposes, even if it cannot be fully trusted.

Shouldn't the ILQP Quote Response header repeat the quoteId from the corresponding ILQP Quote Request header, and how do these "headers" relate to the "body" of ILQP messages?

Yes. And these ILQP headers would replace the current quoting protocol format. The idea is to make all of the protocols we're defining more consistent with one another.

why use two's complement for the significand? Is there a use case for negative significands?

Right now I don't think we have a use for negative amounts in ILP packets.

just like there are 255 ways to represent 0, there are 57-k ways to represent any k-figure integer number, for k>0.

I think that's unavoidable, but it doesn't seem like a big problem because it's trivial to make the decoder respect all representations.

@emschwartz
Copy link
Member Author

Some thoughts after starting to implement the packet with protobufs and @justmoon's proposal:

  • Extensibility - one of protobuf's main selling points is the ability to add new fields to messages without breaking compatibility. We probably don't need this if we can just define new header types for successive versions and new protocols.
  • Schema validation - even if we use something like protobuf we will still need to implement an ILP packet class that validates our specific schemas, so it's unclear how much benefit we would get from using an existing parser
  • Language support - it took me more time to figure out the JS protobuf libraries than it did to implement a custom encoder/decoder for the custom format
  • Arrays and length-prefixed values - we have a number of variable-length values in the header and need a consistent way to encode these. Also, the ILQP Response includes an array and we'll need a format for this. This makes me think we should use an existing encoding, possibly like OER, to avoid reimplementing everything, even if our implementors guide assumes you're writing a complete parser from scratch.
  • Partial flag - how important is this? It's one more thing for implementors to think about, and it seems like one of the small features in IP that seemed important but never really got used
  • Custom headers - we should document the "custom header" type. Also, I would use the first type in each category for the custom header
  • Type range - the range seems a bit excessive. Do we need that much room? How many different headers are we realistically going to have as part of the standard? (Note I'm assuming that anyone can add a non-standard header using the Custom Header type)
  • Dependent types - not a huge fan of this. In the same vein as the previous comment, this seems like a way to get way more types but it comes at the expense of a slightly more complicated parser. It is just one more thing that makes implementing this spec complicated, that seems to provide minimal value

Overall: I think we'll end up needing an ILP Packet class in every language we want to support. I don't see a whole lot of benefit in depending on a large existing parser. I would prefer a full spec and implementors guide. However, since we need to encode a lot of standard types of data it might be good to use a simple existing encoding like OER, but it's okay to assume people will be implementing the class from scratch. If we do make this assumption, it would be nice to take out all the little things that make the implementation more complicated.

Small things that could use clarification in the spec:

  • Are UUIDs encoded as numbers?
  • Are ILP addresses encoded as ascii?
  • Pseudocode (or link to implementation) would help with the Amount type
  • ILQP quote request doesn't have source or destination amount. Should it have one, both, or should there be two different messages for the two different flows? Also, can you request a quote with no amount specified just to get the liquidity curve back?
  • If we keep the partial bit, give an example of how it modifies the type (e.g. 32768 | 16384 = 49152)

@emschwartz
Copy link
Member Author

emschwartz commented Feb 13, 2017

From reading over the OER spec, I'm not sure it would actually provide that much value. It seems like the main feature is the way it encodes variable-length octet values. However, we probably want to specify hard maximums for all of the fields that would go in the ILP packet. In this case, the only benefit of OER's variable-length length field trick would be some minor efficiency gain (e.g. when your length can be 1 byte instead of 2), but I think it would be worth an extra byte here or there to make the implementation even simpler.

That said, I think it would be good to specify how we are going to do length-prefixed values. In the very simple examples given in @justmoon's proposal it makes sense to have only one length field for the whole header because you can infer the one variable-length value. However, this breaks down as soon as you have two ILP addresses in the header, as in the quote request and response. Would it make sense to make all values, or at least all variable-length values, length-prefixed (say with a uint16 length) and to remove the length from the header preamble?

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

6 participants