docs: introduce ledger-ledger lotocol #251

justmoon · 2017-07-29T00:04:07Z

Binary protocol to replace RPC API. Resolves #249.

I did not pick the name.

Created with @emschwartz and @sharafian.

@emschwartz

Binary protocol to replace RPC API. Resolves #249. I did not pick the name. Created with @emschwartz and @sharafian.

adrianhopebailie · 2017-07-30T18:10:50Z

It would be useful to have a way to return an Interledger Error in reponses and also some kind of response code.

I assume that this model assumes the response code comes from the transport (e.g. HTTP, 200, 403 etc) but those codes are not really always appropriate unless were designing this API to use REST (which I don't recommend, I like the RPC pattern we've got).

The transfer life-cycle also seems a bit odd. There is an async response to a transfer request if it is rejected but not if it is accepted. What is returned in the transfer response for a successful transfer request?

adrianhopebailie · 2017-07-30T18:17:04Z

asn1/LedgerLedgerLotocol.asn

+}
+
+LedgerLedgerLotocolPacket ::= SEQUENCE {
+    -- One byte type ID


Maybe differentiate between request and response so that response can carry an optional error and a response code.

They are differentiated by each CallSet item either being a request or a response. Note that ILP Errors count as IlpPacket types

adrianhopebailie · 2017-07-30T18:17:35Z

asn1/LedgerLedgerLotocol.asn

+;
+
+SideProtocolData ::= SEQUENCE OF SEQUENCE {
+  protocolName IA5String,


Any examples of what this might be?

We were talking with @jimmiebfulton and @sappenin about cases where you might want to encode rules related to a certain payment, like ledgers the payment should not be routed through. In the JS ledger plugin interface this kind of side protocol would be included in the transfer's custom field. They brought up a good point that that information might be relevant to quoting as well. We figured we might as well put this type of extensibility in all of the messages.

This would also be useful for sending payment channel claims

adrianhopebailie · 2017-07-31T05:49:47Z

p.s. I suggest the name Interledger RPC

michielbdejong · 2017-07-31T08:44:34Z

What's the difference between MessageRequest and CustomRequest? Both seem to have a similar role, of making the protocol extensible. For a ledger API, sendMessage makes sense (it's asking the ledger to send something to another account holder), but for RPC I don't think it's as logical to have. They both can be seen as a form of nested "RequestRequest", where you still have to specify what goes inside. As an illustration of this, you can see that the current send_request has a 'method' field, even though in the URL it already specified method=send_request.

So I would say extensibility can be done through the CallSet registry (we can propose method 14, 15, 16, ... as backwards-compatible protocol extension), and then rename MessageRequest to QuoteRequest (maybe all 3 of them separately) and rename CustomRequest to BroadcastRoutesRequest.

michielbdejong · 2017-07-31T08:48:29Z

If we decide that this is the way forward, then:

what is our deprecation schedule for ilp-kit 3.0?
can we commit to keeping this spec stable for a certain time?

emschwartz · 2017-07-31T09:59:16Z

asn1/LedgerLedgerLotocol.asn

+;
+
+SideProtocolData ::= SEQUENCE OF SEQUENCE {
+  protocolName IA5String,


We were talking with @jimmiebfulton and @sappenin about cases where you might want to encode rules related to a certain payment, like ledgers the payment should not be routed through. In the JS ledger plugin interface this kind of side protocol would be included in the transfer's custom field. They brought up a good point that that information might be relevant to quoting as well. We figured we might as well put this type of extensibility in all of the messages.

emschwartz · 2017-07-31T10:00:26Z

asn1/LedgerLedgerLotocol.asn

+}
+
+LedgerLedgerLotocolPacket ::= SEQUENCE {
+    -- One byte type ID


They are differentiated by each CallSet item either being a request or a response. Note that ILP Errors count as IlpPacket types

emschwartz · 2017-07-31T10:23:55Z

asn1/LedgerLedgerLotocol.asn

+    data CALL.&Type ({CallSet}{@type})
+}
+
+END


Since there is no call to get currency code and scale, where should a ledger plugin for this protocol get that info from? We talked about getting that from some kind of webfinger lookup but does that mean the plugin for this protocol would depend on the webfinger lookup as well, or would those options have to be configured?

I think it's reasonable to remove getInfo and getAccount. There is a lot more you also need to negotiate, like version of this protocol, version of underlying socket protocol, endpoint URL in each direction, and auth tokens. Also, it might be that currency info is only defined relatively (e.g. company stock).

emschwartz · 2017-07-31T10:31:53Z

What's the difference between MessageRequest and CustomRequest?

@michielbdejong MessageRequest includes an IlpPacket and CustomRequest doesn't. @justmoon argued for doing it this way instead of making the IlpPacket an optional field

what is our deprecation schedule for ilp-kit 3.0?

This spec needs to be hashed out more, implemented as a plugin, and worked into ilp-kit before we talk about any deprecation schedules. We could also make it backwards compatible by enabling kits to peer over different plugins.

can we commit to keeping this spec stable for a certain time?

Not before an initial version is even hashed out, but my idea with proposing using a binary format in the first place was that I think this could become a fairly long-lived ledger-layer protocol.

michielbdejong · 2017-07-31T11:54:31Z

one method I'm missing is 'settle', where one party tells the other party that they think the trustline balance changed in their favor, and includes either proof of on-ledger (unconditional) payment, or a message referring to manual/out-of-band settlement, or an updated payment channel claim. it's currently in the fulfill response side-protocol-data, but I think it would be good to make it more explicit?

Another would be 'setup', where ledger A tells ledger B its own RPC endpoint. Either that, or:
A simplification would be to make two changes:

sendTransferAndWait (would return the finalization),
getNewRoutes would be the reverse of broadcast_routes
That way all methods (getQuote, sendTransferAndWait, getNewRoutes, settle) would be sender-to-next-connector, and then you don't have to tell the next connector your own RPC endpoint, because no communication is ever initiated in the receiver-to-sender direction.

Just one more thought on backwards compatibility - we could also reduce this new protocol to an optional extension of the current protocol, where one peer can try a send_binary RPC call, and if it fails, fall back to the normal JSON format. It would mean all servers are still compatible, but if both server support binary, then they can benefit from its advantages. Not sure if that would be desirable, because it would merge the current and the new protocol into one ugly protocol.

emschwartz · 2017-07-31T13:48:51Z

one method I'm missing is 'settle'

I think that would be a good use of the sideProtocolData. The particular details you'd be expected to include (and even what messages you need to attach them to) are dependent on the settlement method you're using, so I wouldn't make a separate call for this.

Another would be 'setup', where ledger A tells ledger B its own RPC endpoint

That sounds a bit unsafe to me

sendTransferAndWait (would return the finalization),

The proposal is that this new protocol would go over raw (TCP/TLS) sockets or WebSockets instead of HTTP, so there's no waiting anyway.

getNewRoutes would be the reverse of broadcast_routes

Sounds like something that should be put in the routing protocol, not in the ledger layer protocol. If we make IlpPacket (OER) messages for the routing protocol then they can be included in this protocol's messages.

we could also reduce this new protocol to an optional extension of the current protocol, where one peer can try a send_binary RPC call

👎 We should handle backwards compatibility on the ledger layer by making ILP Kit support different plugins better. Also, since this would go over raw sockets or WebSockets it wouldn't really make sense to combine the two.

michielbdejong · 2017-07-31T13:59:24Z

ok, so I think a prerequisite for adding this new protocol as a second way in which servers can peer with each other, is for servers to announce which peering method(s) they support. If quote requests, settlements, and route broadcasts are not defined by LLL, but all of these are needed for a working connector-to-connector peering, then a server would have to announce "I support LLL version A + ILQP version B + routing-protocol version C + settlements-protocol version D".

michielbdejong

This protocol competes with ilp-kit's current peering protocol. Therefore, before this becomes an RFC, we need to implement a version check in ilp-kit's peering routine, where it can use this new ilp-plugin-LLL, or fall back to ilp-plugin-virtual if the peer doesn't support LLL. This version check could for instance be implemented using a WebFinger lookup between the two hosts.

emschwartz · 2017-07-31T14:14:38Z

@michielbdejong I almost agree with that, but not quite.

Layer	Protocols	Announce versions?
Ledger	LLL / JSON RPC / Plugin XRP PayChan / (...other payment channel plugin)	Connectors need to decide what ledger (plugin) they are going to peer over. This is less of a "versioning" question and more a matter of picking a ledger protocol and having the implementation support multiple options.
Interledger	ILP + ILQP	This is versioned but changing this version will be like upgrading from IPv4 to v6. I don't think it's worth including this in the announcements, because parties that use different versions of these protocols are pretty much on different Interledger networks
Routing	CCP	Agreeing on the version and/or variant of this should probably be in the peering protocol

This protocol competes with ilp-kit's current peering protocol.

That is only true if connectors that use this assume all other connectors use it. Peering should include an option about what ledger protocol the connectors are going to use. ILP Kit should be changed to be more flexible and support multiple plugins for peers.

michielbdejong · 2017-07-31T16:54:53Z

ok, in any case, when you peer with someone, you want to know exactly what they support on all of those layers. we can start with adding an "LLL support" indication to ilp-kit's WebFinger announcement, and then make it use the correct plugin for each peer based on that.

michielbdejong · 2017-08-01T15:08:40Z

asn1/LedgerLedgerLotocol.asn

+}
+
+TransferRequest ::= SEQUENCE {
+  transferId UInt128,


you could also use the condition as a unique id of the transfer?

i vaguely remember this was to allow friendly repeated attempts at the same payment, but now don't see why you couldn't do the same thing using just (repeated) condition.

Enforcing condition uniqueness would also prevent you from doing things like Optimistic over Universal (which might not be a good idea anyway)

michielbdejong · 2017-08-01T15:10:59Z

asn1/LedgerLedgerLotocol.asn

+}
+
+FulfillRequest ::= SEQUENCE {
+  transferId UInt128,


or condition

michielbdejong · 2017-08-01T15:11:39Z

asn1/LedgerLedgerLotocol.asn

+}
+
+TransferStateRequest ::= SEQUENCE {
+  transferId UInt128,


for this to work nicely, it does make sense, i guess, to have a separate id field

michielbdejong · 2017-08-01T15:29:52Z

asn1/LedgerLedgerLotocol.asn

+}
+
+BalanceResponse ::= SEQUENCE {
+  currentBalance VarInt,


We should define more precisely, i think the most generic definition would be:

+ total amounts which you received and peer agrees you have fulfilled in time

- total amounts which you sent and peer claims to have fulfilled in time

- total amounts which you sent and peer claims to still be pending

+ total amounts which peer agrees you sent on-ledger (unconditional/fulfilled/paychan-close)

- total amounts which peer claims you received on-ledger (unconditional/fulfilled/paychan-close)

+ total of, for each open outgoing paychan, highest claim peer agrees you gave

- total of, for each open incoming paychan, highest claim peer says they gave you

So then:

your max send limit is currentBalance-minBalance

your max receive limit is maxBalance-currentBalance

another view is saying settlement can only occur through circular conditional payments to self, "into this trustline", so then this protocol can only be used for one-to-one trustlines, and you only need the first 3 items to describe the balance. Apart from LLL, we would still on-ledger escrow plugins as well as paychan plugins, but they would only be used for the settlement payments, and LLL would be used for the "main traffic".

michielbdejong · 2017-08-01T15:31:18Z

asn1/LedgerLedgerLotocol.asn

+    -- One byte type ID
+    type CALL.&typeId ({CallSet}),
+    -- Allow multiple accounts to be multiplexed on one connection
+    accountId UInt32,


does account mean "from" or "to", here? Or is it more "prefix"?

It's more prefix

michielbdejong · 2017-08-01T15:31:54Z

asn1/LedgerLedgerLotocol.asn

+    accountId UInt32,
+    -- Used to associate requests and corresponding responses
+    -- If requestId = 0, the server MUST not send a response
+    requestId UInt32,


doesn't this also duplicate the transferId?

They do slightly different things. I could make multiple calls, in which each request/response would have the same requestId, regarding the same transfer. First I would prepare it, which has a request and response, then it would get fulfilled, which has another request/response, and then I could check the state, etc. requestId is meant to correlate a specific call request with its response.

I could make multiple calls, in which each request/response would have the same requestId, regarding the same transfer

This doesn't make sense. Do you mean different requestId and same transferId?

Yes, sorry. Same transferId

michielbdejong · 2017-08-01T16:18:12Z

One thing I'm missing in this protocol is a clear state machine definition of how the balance updates as a result of transfers getting finalized in the symmetric trustline case:

I would like to be able to send a custom message that changes the balance (I'm thinking of sending paychan claims or sending a reference to an unconditional on-ledger / out-of-band payment), including 'agree' and 'dispute' response codes.
I assume message in one direction are delivered FIFO, but I would like to know if it's also impossible that two messages cross each other if they travel in opposite direction. That's relevant for communicating balance updates.
If all messages are strictly linearly ordered, then each Fulfill message could carry an updated balance, and each Fulfill response and each CustomRequest response could carry an agree/dispute response code, where dispute reason might be 'No transfer prepared for that condition' or 'New balance amount is different from previous balance + transfer amount' or 'Fulfillment incorrect for that condition' or 'Attempt to fulfill already fulfilled transfer' or 'Attempt to fulfill after cancel'
Disagreements about timing are fundamentally unavoidable when doing Interledger over a trustline. So the protocol should have a way to deal with them: Keep track of a disputed amount. If the disagreement is in each other's favor then the disputed amount is lowered; if it's in conflicting direction, then the disputed amount is increased. That way, the state of the trustline can stay consistent indefinitely.

emschwartz · 2017-08-01T16:25:34Z

One thing I'm missing in this protocol is a clear state machine definition

To be fair, this is just the data format. We also need an accompanying RFC to explain how its used.

I would like to be able to be able to send a custom message that changes the balance

Don't think anything here is stopping you from doing that.

I assume message in one direction are delivered FIFO, but I would like to know if it's also impossible that two messages cross each other if they travel in opposite direction. That's relevant for communicating balance updates.

I'm not sure this protocol would make any guarantees about the message order. That only causes an issue though if you're playing around very close to the balance limit and are taking action based on the balance numbers your peer is reporting, right?

If all messages are strictly linearly ordered, then each Fulfill message could carry an updated balance

This seems more complicated than necessary to me. As as user of this protocol I think I'd prefer to leave that out and just do some little reconciliation when necessary.

Disagreements about timing are fundamentally unavoidable when doing Interledger over a trustline. So the protocol should have a way to deal with them: Keep track of a disputed amount.

Does that need to be in the protocol (on the wire format) or would that just be an implementation feature?

michielbdejong · 2017-08-01T16:29:51Z

then we should maybe move BalanceRequest/BalanceResponse out of scope as well

emschwartz · 2017-08-01T16:32:46Z

I think having some kind of getBalance call seems useful, but if there are additional fields you want to ask about in the context of reconciliation that could arguably go into a "side protocol".

michielbdejong · 2017-08-01T16:43:05Z

What is the function of TransferResponse, FulfillResponse and RejectResponse? It would be more efficient to just have an incrementing RequestId for detecting when a message was lost, or maybe the underlying socket guarantees that it can detect and retry when messages get lost (for instance, a WebSocket can have latency and can be disconnected, but never drops messages).

emschwartz · 2017-08-01T16:47:32Z

Acknowledging that the request was processed.

I think making every request blocking (which doing this over TCP might do anyway) seems like it would make it much less efficient than if you just fire them off asynchronously.

michielbdejong · 2017-08-01T16:50:41Z

BalanceRequest/BalanceResponse is only useful in the asymmetric case (i.e. when only one peer remembers the trustline balance) if its meaning is strictly defined. In the symmetric case (when both peers remember their account of the trustline balance), you would want to know not only how much balance your peer is claiming, but also which events were taken into account while summing up the currentBalance, so that you can check whether you and your peer agree about it. If we move BalanceRequest/BalanceResponse out of scope then we don't even have to distinguish between the symmetric and the asymmetric case, so that's a lot simpler.

michielbdejong · 2017-08-01T16:53:17Z

Acknowledging that the request was processed.

You know your TransferRequest was processed when you get back a FulfillRequest or a RejectRequest.
You know your FulfillRequest/RejectRequest was processed when your peer does not follow up with a TransferStateRequest.

emschwartz · 2017-08-01T16:59:41Z

You know your TransferRequest was processed when you get back a FulfillRequest or a RejectRequest.

Actually that's not totally true. Right now the LPI sendTransfer promise resolves when the transfer has been accepted by the ledger. You only get the fulfillment notification or rejection when the Interledger payment has made it to the final receiver. If the message just gets dropped you might want to resend it, but you'll have to wait the whole expiry duration to figure that out if there's no acknowledgement.

Maybe we'd want to rely on TCP for resending messages? Although I think there's also a strong argument for doing this whole protocol over UDP, especially with things like quote requests, because you don't want them to block other, more time-sensitive messages.

michielbdejong · 2017-08-02T08:46:36Z

I agree BalanceRequest/BalanceResponse can stay in, but only if we mathematically define the meaning of the currentBalance number. I propose we define it as the sum of amounts for which:
a) FulfillRequest resulted in a FulfillResponse 'OK' (and not a FulfillResponse 'too late')
b) some side protocol changed the balance in a way that is defined by that side protocol

Note that this gives the preparing party an opportunity to cheat: they can use the fulfillment, and then still tell the fulfiller that they were too late. When this happens, the fulfiller should temporarily lower their trust in their peer; if it happens repeatedly in one direction but never in the other direction, the trustline will eventually dry up, and this is how the fulfiller's exposure to this form of cheating is capped.

If we cannot agree on what the mathematical meaning of currentBalance is, or if that meaning is different in one direction than in the other (that's the situation currently implemented in PluginVirtual disputed balance) then I think we should remove BalanceRequest/BalanceResponse from the core protocol, and make it a CustomRequest.

emschwartz · 2017-08-02T09:06:16Z

Ok I'm convinced now that we should take out the balance request and response.

There are at least three different uses of this protocol, which would all want something different from that call:

Asymmetric trustlines: the balance is whatever the stateful side says it is
Symmetric trustlines: there are different values related to the balance that you might want to communicate about, including all of the things @michielbdejong mentioned above
Claim-based payment channels: might want to communicate all of the values used in symmetric trustlines plus others related to how much has been claimed

It seems crazy to me to take out the balance call from this protocol, but I think @michielbdejong is probably right that it's better to leave it out than have it defined differently by different implementations of this protocol. Since what is meant by "the balance" is necessarily different in different use cases, we should take it out of this spec.

@michielbdejong

based on comments from @michielbdejong in #251

michielbdejong · 2017-08-02T17:30:41Z

asn1/LedgerLedgerLotocol.asn

+  amount UInt64,
+  condition UInt256,
+  expiresAt Timestamp,
+  packet InterledgerPacket,


instead of including an InterledgerPacket here, with an InterledgerProtocolPayment inside it, and a useless type field that will always be 1, we should maybe directly include the InterledgerProtocolPayment itself here?

I don't think we should do that because there may be other versions of the InterledgerProtocolPayment packet later, and we could also decide to do things like sending quote requests on transfers.

ok, good point. I removed this from #256 (comment) (which I am now quite starting to like as I look at it again, actually).

michielbdejong

I expressed my requested changes as a PR to this PR: #256

@michielbdejong

slightly different take on: #251 and #256 based on conversations with @michielbdejong, @sharafian, @justmoon By separating the transfer contents from whether they are part of a bilateral (e.g. trustline) or multilateral (e.g. centralized ledger) relationship, the transfer protocol becomes much more general and thus makes sense to include alongside the "core" ILP data formats. InterledgerRpc wraps the InterledgerPacket types in an envelope that adds a to, from (either or both of which can be set to empty strings if they are implied, as is the case in a bilateral connection), a requestId to correlate requests and responses, and sideProtocolData for extensibility. Any type of InterledgerRpc request can become a paid request by including a Transfer or ConditionalTransfer in it (which themselves can contain other InterledgerPackets -- InterledgerProtocolPayments or any other type of data that you might want/need to pay for).

@michielbdejong

based on comments from @michielbdejong in #251

michielbdejong · 2017-08-07T17:51:36Z

asn1/LedgerLedgerLotocol.asn

+PrepareRequest ::= SEQUENCE {
+  transferId UInt128,
+  amount UInt64,
+  condition UInt256,


why rename executionCondition to condition?

@michielbdejong

based on comments from @michielbdejong in #251

@michielbdejong

based on comments from @michielbdejong in #251

emschwartz · 2017-08-16T22:16:01Z

Superseded by #271

docs: introduce ledger-ledger lotocol

29fdc91

Binary protocol to replace RPC API. Resolves #249. I did not pick the name. Created with @emschwartz and @sharafian.

justmoon requested review from adrianhopebailie, sappenin, michielbdejong, jimmiebfulton, emschwartz and sharafian July 29, 2017 00:04

adrianhopebailie requested changes Jul 30, 2017

View reviewed changes

emschwartz reviewed Jul 31, 2017

View reviewed changes

michielbdejong suggested changes Jul 31, 2017

View reviewed changes

michielbdejong mentioned this pull request Aug 1, 2017

docs(21): make RPC API into LPI-over-HTTP #254

Closed

michielbdejong reviewed Aug 1, 2017

View reviewed changes

asn1/LedgerLedgerLotocol.asn

}

FulfillRequest ::= SEQUENCE {

transferId UInt128,

Copy link

Contributor

michielbdejong Aug 1, 2017

Choose a reason for hiding this comment

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

or condition

michielbdejong reviewed Aug 1, 2017

View reviewed changes

emschwartz added 2 commits August 2, 2017 11:48

docs(asn1): remove balance request message

adcdc5f

based on comments from @michielbdejong in #251

docs(asn1): Message -> Packet, Transfer -> Prepare

7a34189

michielbdejong reviewed Aug 2, 2017

View reviewed changes

michielbdejong suggested changes Aug 4, 2017

View reviewed changes

This was referenced Aug 4, 2017

Proposal: Interledger RPC (instead of LLL) #260

Closed

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

Closed

michielbdejong pushed a commit that referenced this pull request Aug 7, 2017

docs(asn1): remove balance request message

0cbfd28

based on comments from @michielbdejong in #251

michielbdejong reviewed Aug 7, 2017

View reviewed changes

michielbdejong pushed a commit that referenced this pull request Aug 7, 2017

docs(asn1): remove balance request message

6ed0d13

based on comments from @michielbdejong in #251

michielbdejong pushed a commit that referenced this pull request Aug 8, 2017

docs(asn1): remove balance request message

d0db162

based on comments from @michielbdejong in #251

emschwartz closed this Aug 16, 2017

adrianhopebailie deleted the docs/lll branch May 24, 2018 21:27

docs: introduce ledger-ledger lotocol #251

docs: introduce ledger-ledger lotocol #251

Conversation

justmoon commented Jul 29, 2017

adrianhopebailie commented Jul 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianhopebailie commented Jul 31, 2017

michielbdejong commented Jul 31, 2017

michielbdejong commented Jul 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emschwartz commented Jul 31, 2017

michielbdejong commented Jul 31, 2017

emschwartz commented Jul 31, 2017

michielbdejong commented Jul 31, 2017 • edited

michielbdejong left a comment

Choose a reason for hiding this comment

emschwartz commented Jul 31, 2017

michielbdejong commented Jul 31, 2017

Choose a reason for hiding this comment

michielbdejong Aug 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michielbdejong Aug 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michielbdejong commented Aug 1, 2017 • edited

emschwartz commented Aug 1, 2017

michielbdejong commented Aug 1, 2017 • edited

emschwartz commented Aug 1, 2017

michielbdejong commented Aug 1, 2017

emschwartz commented Aug 1, 2017

michielbdejong commented Aug 1, 2017

michielbdejong commented Aug 1, 2017

emschwartz commented Aug 1, 2017

michielbdejong commented Aug 2, 2017 • edited

emschwartz commented Aug 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michielbdejong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emschwartz commented Aug 16, 2017

michielbdejong commented Jul 31, 2017 •

edited

michielbdejong Aug 1, 2017 •

edited

michielbdejong Aug 1, 2017 •

edited

michielbdejong commented Aug 1, 2017 •

edited

michielbdejong commented Aug 1, 2017 •

edited

michielbdejong commented Aug 2, 2017 •

edited

emschwartz commented Aug 2, 2017 •

edited