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

docs(asn1): add to/from to LLL request #261

Closed
wants to merge 1 commit into from

Conversation

emschwartz
Copy link
Member

@emschwartz emschwartz commented Aug 4, 2017

Depends on #251

The LLL is intended primarily for use in bilateral trustlines. However, we have added extensibility mechanisms that also allow it to be used as a base protocol for implementing Interledger over payment channels.

There is only one feature that we need to make the same protocol also useful for multilateral ledgers: replacing the numeric with and in the RPC wrapper. This is not strictly necessary but having a common base protocol that loads of people build off of (or implement directly in their ledgers) will save implementors a lot of time. It would also make debugging easier if the same tools could be used to inspect the messages of multiple ledger protocols. That is gained with minimal cost -- and actually switching from a 32-bit integer to two strings that can be empty saves 2 bytes when they're not being used -- so I think it's worth including the and fields.

The LLL is intended primarily for use in bilateral trustlines. However, we have added extensibility mechanisms that also allow it to be used as a base protocol for implementing Interledger over payment channels. There is only one feature that we need to make the same protocol also useful for multilateral ledgers: replacing the numeric  with  and  in the RPC wrapper. This is not strictly necessary but having a common base protocol that loads of people build off of (or implement directly in their ledgers) will save implementors a lot of time. It would also make debugging easier if the same tools could be used to inspect the messages of multiple ledger protocols. That is gained with minimal cost -- and actually switching from a 32-bit integer to two strings that can be empty saves 2 bytes when they're not being used -- so I think it's worth including the  and  fields.
-- Peers MAY also use shorter aliases to avoid repeatedly
-- sending the full ILP Addresses over the wire
to Address,
from Address,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we can expand accountId to be zero or more of toId and fromId, and I'm also in favor of switching from UInt32 to strings. But why switch to ILP addresses? accountId was not an ILP address either, so what changed?
And what should happen when it's a full ILP address, but it's outside the context of this connection? Is the receiver expected to forward it? If not, and like accountId, to[Id] and from[Id] are still strictly tied to the context of the current connection, then I think using ILP addresses here is only confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we switch it to strings that only contain characters that can go in addresses, the only potential difference between the Address format and a new one we'd come up with for this local address segment is the length. I think it would be fine to use the same schema but define in the protocol spec that the values here SHOULD be local address fragments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we ended up removing accountId and not adding from or to at the CommonLedgerProtocol level

-- Peers MAY also use shorter aliases to avoid repeatedly
-- sending the full ILP Addresses over the wire
to Address,
from Address,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If LLL is not strictly bilateral, we should also think about how to prevent address spoofing. There are two potential attack vectors:

  1. A malicious sender fakes the from field, so that the recipient thinks the packet was sent by someone else.
  2. A malicious receiver manages to receive packets that were indented for another receiver (think of the ILP-equivalent of ARP spoofing).

To prevent such attacks, we need to define how protocol participants authenticate their peers in a multilateral scenario.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The protocol spec corresponding to these schemas will definitely need to deal with authentication and also issues like address spoofing. Right now we're assuming that happens when the connection is opened (either using token-based authentication for WebSockets or something like TLS Client Certs for TLS sockets).

@emschwartz
Copy link
Member Author

After discussion with @sharafian, @michielbdejong and @dappelt I was convinced that the from and to should be taken out of the core protocol. They are not needed in the bilateral use case and they can be added as sideProtocolData in multilateral ledger cases.

@emschwartz emschwartz closed this Aug 7, 2017
emschwartz added a commit that referenced this pull request Aug 7, 2017
That feature is specifically for the admin plugin case, when you are authenticated as an admin user and acting on behalf of others. Since that is not an all the time case, that should go into the sideProtocolData. The reason for taking this out is also related to why we're not including #261
emschwartz added a commit that referenced this pull request Aug 7, 2017
That feature is specifically for the admin plugin case, when you are authenticated as an admin user and acting on behalf of others. Since that is not an all the time case, that should go into the sideProtocolData. The reason for taking this out is also related to why we're not including #261
michielbdejong pushed a commit that referenced this pull request Aug 7, 2017
That feature is specifically for the admin plugin case, when you are authenticated as an admin user and acting on behalf of others. Since that is not an all the time case, that should go into the sideProtocolData. The reason for taking this out is also related to why we're not including #261
michielbdejong pushed a commit that referenced this pull request Aug 8, 2017
That feature is specifically for the admin plugin case, when you are authenticated as an admin user and acting on behalf of others. Since that is not an all the time case, that should go into the sideProtocolData. The reason for taking this out is also related to why we're not including #261
@adrianhopebailie adrianhopebailie deleted the es-lotocol-for-multilateral-ledgers branch May 24, 2018 21:27
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

Successfully merging this pull request may close these issues.

None yet

3 participants