LPI: Add fulfillment data, make rejection message binary

[emschwartz]

Related to #314. Also, when you reject payments you should be able to submit an ILP error (which is binary). I think the RejectionMessage object is a hold-over from before the ILP errors were binary.

Open questions:

• Should we keep the *_cancel events using the RejectionMessage class? (Are all of the plugins properly differentiating between cancel and reject events?)
• Should we make an object to hold the fulfillment and ilp in the fulfillCondition method or just have 3 parameters?
• Should we use ilp as the parameter name for fulfillCondition and rejectIncomingTransfer methods (to match the field in the Transfer object) or something more generic? Can this field hold any string or just ILP packets?
• Should we add an error to indicate if you try to send an ILP packet that's too big?

[michielbdejong]

Adding an optional fulfillment packet is OK if we want to implement #314, and then it would make sense to make rejectionReason binary, for symmetry.

But by itself, the proposal of changing rejectionReason from JSON to base64-encoded OER is a side-ways changes that doesn't give any new functionality.

Unless we come to the conclusion that we can't build apps without #314, IMHO we should shelf this discussion.

[michielbdejong]

Which packet type would this be?

[emschwartz]

It would be a new one that would just be an OCTET STRING with data, but I don't think that the LPI needs to specify that

[michielbdejong]

If we would implement #314, then that would also require new ILP packet types, and a new form of sendTransfer, and a different type of conditional transfers in ledger implementations. I don't think such a fundamental change to the interledger design is necessary.

[emschwartz]

@michielbdejong

Wait, what? Why would sendTransfer change at all?

If by "different type of conditional transfers in ledger implementations", you're talking about special-casing the null condition, that isn't necessary to make the informational end-to-end quoting work. It'll work just fine if everyone just sends a normal transfer with that condition.

Also, after talking about it with @justmoon more, we concluded that it would be better to build another transport layer protocol that handles chunking payments, rather than implementing the end-to-end quoting as ILQPv2. In order to build transport layer protocols like this on top of ILP we need well-defined errors that will get relayed back and the ability for receivers to include data alongside the fulfillment. I think these are important building blocks that should be included in the core protocol.

[michielbdejong]

Why would sendTransfer change at all?

OK, sorry, I now realize I misinterpreted #314, see my update there.

So then indeed, you would implement it as proposed in this PR, without ledger-level checks.

But then my objection is that without ledger-level checks, this would not be secure.

This same objection has already been brought up in different words by @sharafian, by @adrianhopebailie, and by @dappelt.

If people use this and rely on it, any connector along the path can annoy the senders: just pretend that there never was any data attached to the fulfillment, still receive their money, and annoy the sender. It would then be hard to determine which connector along the path removed the data.

How would you solve that problem?

[adrianhopebailie]

How would you solve that problem?

One option is to make the fulfillment a hash of the data. It's not going to force connectors to forward the data but it does alert the sender if the data is tampered with.

If we want to ensure the data is sent then we could make the fulfillment the data (likely encrypted).

This would require us to remind ourselves why we have standardized on a 32-byte fulfillment and and weigh up the pro's and con's of dropping that restriction.

[justmoon]

Might be worth splitting the two changes. The fulfillment change is a non-breaking change (old implementations will ignore the extra parameter and simply not pass on the fulfillment data) while the rejection format change is breaking (new connectors will confuse old plugins by sending a string instead of an object.)

That said, I support both changes. 👍 - We could do the fulfillment data change right away and start adding support for accepting both the old and new rejection data format in plugins and connectors, but still emit the old format for now.

[justmoon]

Split the fulfillment data change into a separate PR #338.

[emschwartz]

Closing this as fulfillment data has now been added and #347 proposes a more comprehensive set of changes to the LPI