interledger-deprecated / five-bells-shared

Common elements that are shared between Five Bells components
Other
4 stars 5 forks source link

fix: forwarded-by should an array #195

Closed justmoon closed 7 years ago

justmoon commented 7 years ago

Required by interledgerjs/ilp-connector#394

sentientwaffle commented 7 years ago

@michielbdejong the LPI spec is wrong -- https://github.com/interledger/rfcs/blob/master/0003-interledger-protocol/0003-interledger-protocol.md#ilp-error-format describes it as a sequence.

michielbdejong commented 7 years ago

The RejectionMessage is part of the lpi as described in IL-RFC-4, and is obviously very similar to the type 8 ilp packets which IL-RFC-3 refers to, but it's not identical, and this is just one of the differences (also, for instance, forwarded_by !== forwardedBy).

justmoon commented 7 years ago

In my mind, the fact that LPI doesn't faithfully reproduce the ILP error is a bug. It obviously makes working with LPI a lot more cumbersome than it needs to be and it actually breaks functionality (the ability to see the path a failed payment took.)

That said, after reading your comments - I recognize that this would be a breaking change, so I'm going to withdraw it because if we are going to make a breaking change, the better change is @emschwartz' interledger/rfcs#317 - making the rejection message binary. We should treat rejection ILP packets the same way we treat other ILP packets.