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

Five Bells Ledger API specification #125

Closed
wants to merge 7 commits into from

Conversation

mDuo13
Copy link
Collaborator

@mDuo13 mDuo13 commented Nov 15, 2016

(Formerly known as the "Common Ledger API")

This PR has been updated to a third iteration, following the work earlier in this PR which continued from PR #90.. In keeping with our current thinking, a common ledger API may not be needed in the long run, but a formalization of the Five Bells Ledger API currently in use is helpful.

Thus, this PR now more closely reflects the existing implementation of the five-bells-ledger, including some of the deprecated and odd functionality such as using "actual" crypto-conditions instead of just hashlocks, supporting cancellation conditions, having multi-credit, multi-debit with a proposed stage, not separating the ID and client ID, lacking URL fields for some operations, etc.

Copy link
Member

@emschwartz emschwartz left a comment

Choose a reason for hiding this comment

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

Overall I think this is really good. Major simplification of the five-bells-ledger API. I'm excited to see this rolled out!

- If the transfer is unconditional, it executes immediately.
- If the transfer is conditional, it waits for a matching fulfillment.
- [Submit Fulfillment][]
- Can execute or cancel the transfer, depending on the fulfillment.
Copy link
Member

Choose a reason for hiding this comment

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

Remove "cancel" mentions (there are a couple others spread throughout the doc that should be taken out since there are no cancellation conditions here anymore)


`^[-+]?[0-9]*[.]?[0-9]+([eE][-+]?[0-9]+)?$`

Client applications can decode numeric strings to whatever representation provides sufficient precision for their specific needs. The ledger should report information about the precision it uses in the [ledger metadata][Get Ledger Metadata] response.
Copy link
Member

Choose a reason for hiding this comment

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

The ledger MUST...


The Common Ledger API provides access to a ledger containing a single currency or asset. The [Get Ledger Metadata][] method reports the type of asset or currency used, as an [Asset resource][].

If a provider supports multiple currencies, the provider can server Common Ledger APIs separately for each currency. For each method described in this document, there would be a version of it prefixed by a different currency code. For example, the [Prepare Transfer][] method could be available for different currencies at the following locations:
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "can server"


### Scope

The Common Ledger API does not define the full range of functionality needed by a functional ledger. This API defines the parts the ILP Client needs, and leaves other parts to the discretion of the ledger implementer. For a fully-functional reference implementation, see the [five-bells-ledger](https://github.com/interledgerjs/five-bells-ledger).
Copy link
Member

Choose a reason for hiding this comment

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

I liked when this mentioned some of the other things you might want to implement if you're building a ledger from this (e.g. creating and updating accounts)

### Transfer Resource
[Transfer resource]: #transfer-resource

A transfer represents money being moved around _within a single ledger_. A transfer debits one account and credits another account for the exact same amount. The Common Ledger API does not define a way to add or remove money from the ledger; the methods defined here always maintain a zero sum across all account balances. However, you can effectively add money to a ledger by transferring it from an "issuer" account whose balance is allowed to go negative.
Copy link
Member

Choose a reason for hiding this comment

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

Minor: the sentence that starts "The Common..." should be separated from the line before because it's explaining a pretty different point


Get a token that can be used to authenticate future requests.

**Note:** This method is REQUIRED for ledgers to authenticate [WebSocket][] connections. If the ledger does not authenticate WebSocket connections, this method is OPTIONAL. The ledger MAY allow clients to authenticate for any other methods of the API using the tokens returned by this method.
Copy link
Member

Choose a reason for hiding this comment

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

I think the ledger MUST authenticate WebSocket connections

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As agreed below, there's a valid use case for unauthenticated WebSocket connections.


### WebSocket Authentication

A ledger MAY require clients to authenticate themselves using a token in the `token` query parameter. If the ledger supports authentication, it MAY restrict the data that can be accessed, as long as it complies with the following rules:
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you would allow unauthenticated users to subscribe to anything

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can imagine a ledger whose purpose is to share transfers publicly, in which case you wouldn't need to authenticate anyone for the WebSocket API.


Later, the ledger responds with [notifications](#websocket-notifications) whenever any of the following occurs:

- A transfer affecting the account is prepared (`event` type: `transfer.create`)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should change the event names to match the transfer statuses

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree.

| `id` | String or Number | An arbitrary identifier for this request. MUST NOT be `null`. |
| `jsonrpc` | String | MUST have the value `"2.0"` to indicate this is a JSON-RPC 2.0 request. |
| `method` | String | MUST be the value `"subscribe_transfer"`. |
| `params` | Array | Each member of this array must be the [URL][] of a transfer to subscribe to, as a string. This array replaces any existing transfer subscriptions on this WebSocket connection. If the array length is zero, the client is unsubscribed from all transfer notifications. |
Copy link
Member

Choose a reason for hiding this comment

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

Looks like params is sometimes an array and sometimes an object -- is that allowed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In JSONRPC 2.0, params is a "structured" value -- in other words, yes. The examples in their spec contain both.

That said, it's probably wise to make this an object so it's forward compatible and similar to the Subscribe to Account call.

|:------------------|:----------------------|:---------------------------------|
| `transfer.create` | [Transfer resource][] | Occurs when a new transfer is prepared. Sent to clients subscribed to the `debit_account` and/or the `credit_account`. If the transfer is unconditional, this notification indicates the state of the transaction after execution. The `related_resources` field is omitted. |
| `transfer.update` | [Transfer resource][] | Occurs when a transfer changes state from `prepared` to `executed` or `rejected`. If the transfer was executed, the `execution_condition_fulfillment` field of `related_resources` MUST contain the fulfillment. If the transfer was rejected, the `related_resources` field is empty. |
| `message.sent` | [Message resource][] | Occurs when someone else sends a message. |
Copy link
Member

Choose a reason for hiding this comment

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

Should be message.send to be consistent with the tense of the others


The Common Ledger API provides access to a ledger containing a single currency or asset. The [Get Ledger Metadata][] method reports the type of asset or currency used, as an [Asset resource][].

If a provider supports multiple currencies, the provider can server Common Ledger APIs separately for each currency. For each method described in this document, there would be a version of it prefixed by a different currency code. For example, the [Prepare Transfer][] method could be available for different currencies at the following locations:
Copy link

Choose a reason for hiding this comment

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

Why restrict the prefix to just a currency code? Why not allow https://ledger.example.com/USD.bitstamp/transfers and https://ledger.example.com/USD.gatehub/transfers, for example?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, the docs should be clear that the prefix can be anything. (As long as it's a valid URI/URI.)

Examples:

/USD.Bitstamp/
/USD/Bitstamp/
/Bond/Samsung/USD/10yr/
/funnymoney/
/€/
/©a$#/

These are all perfectly valid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that's what's intended, it's just not clear from how it's written now. (I'll fix later.)

| Name | Type | Description |
|:--------------------------|:--------|:---------------------------------------|
| `name` | String | Name of the account. A ledger MAY require this to be unique. MUST match the regular expression `^[a-zA-Z0-9._~-]{1,256}$`. |
| `fingerprint` | String | _(Optional)_ A fingerprint of the account owner's client certificate. This field MUST be available if and only if the ledger supports client certificate authentication. MUST match the regular expression `^[0-9A-Fa-f]{2}(:[0-9A-Fa-f]{2}){19,127}$`. |

Choose a reason for hiding this comment

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

We are currently discussing a proposal to replace fingerprints with hostnames, which makes certificate changes easier.

"expires_at": "2016-10-25T08:41:59.795Z",
"state": "prepared",
"timeline": {
"prepared_at": "2015-06-16T00:00:01.000Z"

Choose a reason for hiding this comment

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

In our implementation we changed "prepared_at" to "created_at" so that the same name makes sense for transfers with and without execution conditions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

By this logic we should change the API method to "Create Transfer" for consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

Should the API maintain both values, allowing for a clear distinction between created_at and a prepared_at?

Per RFC1, the term "Prepared" demands an escrowed payment, which is not accurate in an optimistic-mode transfer. However, if we use the term created_at, but in some instances (e.g., Universal Transfer) we really mean "when this was prepared", it it could lead to confusion. Likewise, choosing the value prepared_at when we really mean "when this was created" (e.g., Optimistic Transfer), seems equally imprecise.

Along those lines, RFC1 talks about a Proposed state. Would it make sense to allow transfers to be created in the Proposed state initially, and then (potentially at some point later) moved into the Prepared state, but only if its a conditional transfer?

That would allow optimistic transfers to have a timeline like the following:

"timeline": {
  "created_at": "2015-06-16T00:00:01.000Z",
  "executed_at": "2015-06-16T00:00:02.000Z"
}

whereas conditional transfers could have a timeline like this:

"timeline": {
  "created_at": "2015-06-16T00:00:01.000Z",
  "prepared_at": "2015-06-16T00:00:01.000Z",
  "executed_at": "2015-06-16T00:00:02.000Z"
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current five-bells-ledger implementation has a proposed state that matches this description. However, one of the goals of the Common Ledger API (when we started on this project) was to simplify the proposed state out of existence.

I think I'm going to go ahead with changing the remaining state to "created" though.




## WebSocket

Choose a reason for hiding this comment

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

Not sure if this will cover all the desired use cases, but in our implementation we just have an optional notification_url stored in each account and a notification is POSTed to this URL when there is activity on that account. This makes it really simple because there are no subscriptions to manage. The downside vs websockets is that the recipient has to have a server, but you could also make a separate microservice that provides the websockets if you want to keep the ledger minimal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Personally I think it would be awesome if the ledger API didn't have to include a WebSocket component at all.

It's possible to do transfer notifications by serving Get Transfer through an http response that doesn't finish until the transfer is in a final state, but I'm not sure how you implement that. (I've seen it done in web-based games before -- the GET just sits there pending until the server has something to respond with.)

But that still doesn't resolve the question of how clients and connectors would communicate and authenticate each other for quoting.

Choose a reason for hiding this comment

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

My comment above answers that, no?

Copy link
Member

Choose a reason for hiding this comment

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

In an earlier iteration of the five-bells-ledger we were using RESThooks instead of Websockets. Switching to websockets was the single biggest speed boost we saw in end-to-end interledger payments. I wouldn't object to having the notification_url as an option too (only downside being another feature for implementers to include), but I would strongly oppose replacing websockets with a slower notification method (not counting WebRTC, MQTT or another real-time messaging protocol).

Choose a reason for hiding this comment

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

The performance improvement was due to removing the database accesses that were required for robust notifications. What I'm suggesting is non-robust HTTP POST notifications, which is what our implementation uses and it is faster than the five-bells-ledger with websockets.

Copy link

Choose a reason for hiding this comment

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

I was playing with an alternative solution to websocket based on gRPC. First thing I needed was to abstract all the "websocket subscription and message stuff" as a bidirectional asynchronous stream.

An early *proto defining the service is available at:
https://github.com/everis-innolab/ILP_gRPC_API/blob/master/ilp_ledger_subscriptions.proto

A comparative about a different alternative solutions for bidirectional async channels:

\ websocket+JSON/RPC 2.0 Client request + Client pooling protobuf + gRPC web-hooks
Notes - Current five-bells implementation - Once the ws connection is stablished client and ledger become peers. - Simplest solution. - Messages from client to ledger are sent in HTTP requests. Messages from ledger to client are pulled during periodic pooling requests. - bidirectional async stream are created with: streamN(stream msg) returns (stream msg) both ledger and client must provide a callback URL for peers messages/events
web-browser support YES YES "WORK IN PROGRESS" github.com/grpc/grpc-experiments/issues/159 github.com/grpc-ecosystem/grpc-gateway NO Android support
iOS support YES YES YES YES
Standard YES (IETF RFC 6455) NO "Open-Source / IETF Draft" NO
Works on 3G, behind proxies/firewalls,... "SOMETIMES" YES YES/NO/SOMETIMES? NO (both peers require IP visibility to expose REST web-hooks)
automatic mapping object to/from network data" PARTIAL and IN SOME LANGUAJES NO YES NO
auto-documented NO NO YES. Using *proto files ( *proto can also be used as ref doc. for non-gRPC implementations ) NO
cross-language support NO NO YES NO
speed GOOD SLOW VERY GOOD UNDEFINED
"deprecated" support (backward compatibility) MANUAL MANUAL YES MANUAL
PREFERED FOR CONNECTORS & SERVER APPS SORT-LIVE CONNECTIONS WHERE MOBILE CLIENTS AND RANDOM NETWORK SETUPS ARE EXPECTED. (""WALLETS"") CONNECTORS & SERVER APPS CONNECTORS & SERVER APPS IF WEBSOCKETS AND gRPC ARE NOT AVAILABLE.

I'm also thinking whether it could be convenient to split this API in services for light clients ("wallets" that just connect time to time to the ledger posting new prepared Transfers and checking payments), server applications ("e-comerce" with permanent tcp/ip connections waiting for payments and fulfilling transfer) and connectors. Also many simultaneous "wallet" connections to ledger could be expected in some setups while only a very limited set of ilp-connectors and server apps ("e-commerce"). If I'm not wrong there is no distinction amongst them right now. The API treats them all equally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great write-up of different channels @earizon! I've been doing similar research as I implement a Java ledger conforming to this Common Ledger API, and have been wanting to experiment with gRPC as an alternative to Websockets. I think once browsers fully support everything they need to (here's one such proposal) then Websockets will probably make less sense in the browser (IMHO).

Your proposal for API separation is an interesting one. In the short-term, it seems like the Websocket channel would be the minimum thing to support so that Browsers can connect, with gRPC maybe being an optional choice? Or do you have a different idea?

Copy link

Choose a reason for hiding this comment

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

@sappenin Yes I think Websocket can be considered required all (web/non-web) clients. But I'm not 100% sure that it will work in all network environments due to HTTP proxies. See for example problems with whatsapp web using wss behind proxies @ google. A fall-back to client pooling could be useful too. It's clearly a suboptimal solution in terms of performance, but for clients that just want to connect "time to time" to make some random payment it could be useful. There are also some other problems when using permanent permanent tcp/ip connections and the number of clients start to grow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sappenin and @earizon I think the core Java stuff is stabalizing to the point that we should definitely try a second adaptor implementation.

Could we try implementing a gRPC based interface between the client and your ledger?

You'd implement the org.interledger.ilp.ledger.LedgerAdaptor interface and then should be able to simply instantiate a Client with your adaptor instead of the REST adaptor.

Copy link

@earizon earizon Jan 30, 2017

Choose a reason for hiding this comment

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

@adrianhopebailie great news!

Can anyone confirm next claims (since it's not 100% clear to me from the spec). Using gRPC syntax I was thinking on:

Ledger acting as gRPC server, ledger-client initiating the gRPC connection:

    rpc subscribeToAccountNotifications(spec.ILPAccount[] account_list ) returns ( stream spec.ILPLocalTransfer ); // call started by ledger-client
    rpc subscribeToTransferNotifications(spec.ILPTransfer transfer ) returns ( stream spec.ILPLocalTransfer );

    rpc setupBidirectionalAsyncMessagesChannel              (stream spec.ILPMessage)       returns ( stream spec.ILPMessage );

ledger-client acting as gRPC server, ledger connecting as gRPC client:

    rpc listenForAccountSubcriptions(stream spec.ILPAccount ) returns ( stream spec.ILPLocalTransfer ); // call started by ledger. 
    rpc listenForTransferSubcriptions(stream spec.ILPtransfer ) returns ( stream spec.ILPLocalTransfer );

    rpc setupBidirectionalAsyncMessagesChannel  (stream spec.ILPMessage) returns ( stream spec.ILPMessage );

In common language:

  • It's expected that the ledger will send transfer notifications to any ledger-client directly subscribed to a transfer or indirectly subscribed to any transfer affecting the account (as creditor or debitor).

  • It's expected that any ledger or ilp-connector-ledger-client will send messages to its peer, to emit or receive events (those events are used to propagate routing information). ( Is there any use-case when an non-ilp-connector will need to listen for message events? )

Edit:
There were some obvious mistakes in the previous gRPC draft/interface. ("Fixed"). I'm considering two possibilities for two different scenarios: It's possible to place the gRPC server in the ledger or in the ledger-client. Placing it on the ledger is the "more intuitive" option, but it could be the case that for some reason that's not possible. gRPC makes it easy to "mirrow" the API inverting the role of the gRPC client & server.

| Name | Type | Description |
|:-----------------------|:---------------------|:-----------------------------|
| `client_id` | [UUID][] | Client-provided UUID for this resource. MUST be unique within the ledger. |
| `ledger` | [URL][] | Resource identifier for the ledger where the transfer occurs. MUST be an HTTP(S) URL where you can [get the ledger metadata][Get Ledger Metadata]. |

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

{
"client_id": "3a2a1d9e-8640-4d2d-b06c-84f2cd613204",
"ledger": "http://usd-ledger.example",
"debit_account": "http://usd-ledger.example/accounts/alice",

Choose a reason for hiding this comment

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

I think this should just be "alice". This is coupling to a lot of unnecessary things. Like if the server switches from http to https it looks like this will cause problems. Or if a new version of the code switches the endpoint name from "accounts" to "account".

Copy link
Collaborator

Choose a reason for hiding this comment

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

The type of this member should be URI and a value relative to the API endpoint should be allowed.

Choose a reason for hiding this comment

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

No reason?

Choose a reason for hiding this comment

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

I'd also be interested to know why this member should be a URI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The ledger is a resource identified by a URI.
The account is a resource that is relative to the ledger.

This is exactly the use case that URIs are designed for.

There is very well defined and deterministic logic for resolving absolute URIs from relative URIs given a base (the ledger) to solve for exactly the issues @clark800 raises such as changing the protocol.

If we are saying that a ledger MUST have an account endpoint URL or an account URL Template (RFC6570) then it makes sense for the accounts to be identified either by the variable that is substituted into the template or by a relative URL that is resolved against the account endpoint URL.

On the other hand if the only normative requirement is for a ledger to have a base URL then the account MUST be identified by a URL relative to the ledger URL /accounts/alice or an absolute URL.

Copy link
Member

Choose a reason for hiding this comment

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

I think having accounts identified by a relative URL is better than expecting all ledgers to have an account endpoint. Some ledgers might want to have an account aliasing or internal sub-accounting feature, in which case I might want you to send money to /aliases/ceb8a630-2d9d-4ce2-ad81-72a862659ae8 instead of my real account /accounts/1234567890.

"from": "https://blue.ilpdemo.ripple.com/ledger/accounts/bob",
"to": "https://blue.ilpdemo.ripple.com/ledger/accounts/alice",
"ledger": "https://blue.ilpdemo.ripple.com/ledger",
"data": {
Copy link
Member

Choose a reason for hiding this comment

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

How large a message should the ledger support sending?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This question is tricky. The size of the message data depends on the encoding. Any limit I could think of, upper or lower, would be totally arbitrary. Maybe we should decide on a minimum limit in bytes and/or characters of UTF-8, chosen based on being able to convey any reasonable quote / ILP header data we expect to need?

| Field | Value | Description |
|:-----------------|:----------|:----------------------------------------------|
| `type` | String | The type of asset. Currently, the only supported type is `iso4217-currency`. |
| `code` | String | The currency code to represent this asset. For `iso4217-currency` assets, this MUST be a three-letter uppercase [ISO 4217](http://www.xe.com/iso4217.php) currency code. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest using the following URL as reference: http://www.iso.org/iso/home/standards/currency_codes.htm


#### Client IDs

The transfer's `client_id` field is one of the main ways of identifying the transfer; it is unique across the ledger, so implementations MAY use it as a primary unique key in a database. The client applications, not the server, generate the `client_id`, so the server SHOULD require that the field match the canonical form for a [UUID][], which is 32 lowercase hexadecimal digits, separated by hyphens into groups of 8-4-4-4-12.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify the intent of client_id?

For example, for a single ILP Payment with transfers on Ledger1 and Ledger2, is it intended that the client_id will be the same in both ledgers?

I'm trying to figure out if this value is meant to correlate ILP payments across modules (i.e., like an ILP transaction id), or if this field has a different purpose.

Copy link
Member

Choose a reason for hiding this comment

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

Good question. This field is only to ensure uniqueness within a single ledger. It is not meant as a way to correlate transfers across ledgers and actually should not be used that way.

The reason to use an id like this within one ledger is to prevent people from accidentally submitting the same transfer twice. If the transfer with that ID has already been submitted, POSTing it again will cause an error.

The client_id should not be kept the same across ledgers because of the potential for id squatting. If you are a connector that always uses the same id for the outgoing transfer as the incoming one, I can prevent you from executing the destination transfer if I know the source transfer's id (for example if I am the sender).

Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason to use an id like this within one ledger is to prevent people from accidentally submitting the same transfer twice. If the transfer with that ID has already been submitted, POSTing it again will cause an error.

The client_id should not be kept the same across ledgers because of the potential for id squatting. If you are a connector that always uses the same id for the outgoing transfer as the incoming one, I can prevent you from executing the destination transfer if I know the source transfer's id (for example if I am the sender).

There are some normative requirements there that we should document in the spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you guys think about having the ledger generate this identifier value instead of the client?

There are quite a few requirements for this identifier that should probably be the burden of the ledger since the ledger is the one mandating them -- e.g., unique across the ledger; conforming to a particular pattern that the ledger can use as a PK in the ledger's database, etc. Plus, the point about preventing id-squatting seems like something that should be the responsibility of the ledger as opposed to the client (who might mess it up or be unable to enforce the requirement).

For my proposal to work, though, the API would need to do something like the following to facilitate the creation of a transfer, which makes it a bit more chatty, but IMHO makes the API more robust:

  1. POST the transfer payload (minus client_id) to /transfers
  2. Receive the created transfer as a response, with a server-supplied transfer id.
  3. When ready, execute a PUT to /transfers/{transfer_id} (or possibly a PATCH) to update the state of the transfer to indicate to the ledger that the transfer is ready to be acted upon (e.g., update the state from "proposed" to "prepared", or something like that).

While the second call requiring clients to overtly trigger the processing of a transfer is somewhat of a negative, this two-phase creation has the added benefit of allowing clients to operate upon the server-side state of a transfer using possibly multiple remote calls before actually having the ledger do anything with it.

Copy link
Member

Choose a reason for hiding this comment

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

This seems unnecessarily complicated to me. A trivial way for clients to avoid id squatting is to either use a completely random UUID or one that is deterministically generated from some detail about the interledger payment (as is supposed to happen in the connector here). Full HTTP round trips are relatively slow and we should avoid adding them wherever possible.

Whether or not the ledger uses the client_id as the database primary key, it shouldn't be difficult for ledgers using any standard database to enforce uniqueness on that value. A previous version of this spec said ledgers could optionally enforce client_id uniqueness on a per-account basis, but it seemed like having more options would just make implementing this spec more complicated.

| `type` | String | The type of asset. Currently, the only supported type is `iso4217-currency`. |
| `code` | String | The currency code to represent this asset. For `iso4217-currency` assets, this MUST be a three-letter uppercase [ISO 4217](http://www.xe.com/iso4217.php) currency code. |
| `symbol` | String | Symbol to use in user interfaces with amounts of this asset. For example, "$". |
| `decimal_digits` | Number | _(Optional)_ Typical number of decimal places to display for amounts of this asset. MUST be a non-negative integer. This MAY be different from the precision and scale used internally by the ledger, which are reported in the [Get Ledger Metadata][] field. |
Copy link
Contributor

Choose a reason for hiding this comment

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

What are some scenarios where the decimal_digits attribute of an Asset would diverge from the metadata's precision and scale attributes?

Copy link
Member

Choose a reason for hiding this comment

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

The description for this field could be improved. decimal_digits refers to the conventionally displayed number of decimal places for a given asset. It's purely a hint for display purposes and is ignored by everyone but human-facing amount formatters. (By contrast, precision and scale affect that actual available precision on a ledger.)

For example, a USD ledger might use a precision of 15 and a scale of 6. So, it can represent amounts like USD 10.123456. However, US dollars are typically shown with a precision of cents, so decimal_digits would be 2. How a client exactly renders this is up to the client, but it might show any precision beyond the decimal_digits in a deemphasized way, e.g. 10.12(3456)

Copy link

@diminator diminator left a comment

Choose a reason for hiding this comment

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

Great job, thanks @mDuo13

My main question would be: how to enable this API spec for blockchain type or append-only ledgers (BTC, ETH, BDB, ...)

They have different design paradigms:

  • no account information, merely public keys
  • transaction IDs are hashes, no UUIDs (agreed that squatting could be a potential problem)
  • When append-only there is only a notion of POST, not PUT. eg. PUT /transfer_*

The Common Ledger API defines the following data types and structures:

- [Transfer resource][]
- [Account resource][]

Choose a reason for hiding this comment

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

From a blockchain perspective, the account resource belongs to the wallet, not the ledger itself.
Blockchain or append-only ledgers only rely on pubkeys.
I'm not saying we should throw this out of the door, but it seems to me to get this API working for both legacy payment ledgers and blockchain ledgers such as BTC, ETH, BDB, etc there would be the need of a proxy API relaying between the chain and the wallet.

Any thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that blockchains would need a proxy server to host a Common Ledger API, which would sit between the client application (wallet) and the blockchain server / p2p network. I'm not sure there's a way around that.

The "account information" returned can be quite minimal. In a blockchain case, "Get Account" could use the pubkey as the name and then just provide the known balance that can be spent by that key pair.

Maybe we need a special value for balances that are unknown in order to support this. I'm not really sure.

Choose a reason for hiding this comment

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

Ok, we can split up the proxy requirements to somewhere else?
Maybe those will be more implementation specific - hence not RFC-ed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see how you'd split it off. The whole REST API would probably be served by some "proxy" server (not as in HTTP proxy).


The Common Ledger API does not define a way to add or remove money from the ledger; the methods defined here always maintain a zero sum across all account balances. However, you can effectively add money to a ledger by transferring it from an "issuer" account whose balance is allowed to go negative.

A transfer object contains the fields from the following table. Some fields are _Server-provided_, meaning they cannot be set directly by clients. Fields that are "Optional" or "Server-provided" in the following table may be omitted by clients submitting transfer objects to the API, but **those fields are not optional to implement**. All fields, including the `memo` fields, must be implemented for the Common Ledger API to work properly. Fields of nested objects are indicated with a dot (`.`) character; no field names contain a dot literal.
Copy link

Choose a reason for hiding this comment

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

I interpret that "client" means in this context the software connecting to the ledger while "server" is the ILP-compliant ledger. I think Using "client" and "server" could cause trouble. Explicitly speaking about "wallets" (the local GUI connecting to the ledger), "ledger" and "connector" will be safer. For example in a websocket stream the difference between the client and server vanishes once the bidirectional message channel is created.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree on using "ledger" instead of "server."

However, I think the term "wallet" tends to be rather confusing and misleading. People use the term "wallet" to mean "client application", "key pair", or "web service" somewhat interchangeably, which is a problem. I'm going to stick with "client application" for now.

| `note_to_self` | Object | _(Optional)_ Arbitrary data provided by the `debit_account` for itself. This field MUST be hidden when not authenticated as the `debit_account` or an admin. |
| `id` | [URL][] | _(Server-provided)_ Primary resource identifier for this transfer. MUST be an HTTP(S) URL where you can [get the transfer resource][Get Transfer]. |
| `fulfillment` | [URL][] | (Server-provided) Path to the fulfillment for this transfer. MUST be an HTTP(S) URL where the client can [submit the fulfillment][Submit Fulfillment] or [get the fulfillment][Get Transfer Fulfillment]. MUST be provided if and only if this transfer has an `execution_condition`. |
| `transfer_rejection` | [URL][] | (Server-provided) Path where a client can [reject this transfer][Reject Transfer]. MUST be an HTTP(S) URL. |
Copy link

Choose a reason for hiding this comment

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

Wouldn't rejection_webhook be more 'standard' (https://en.wikipedia.org/wiki/Webhook) ?

I don't have a global picture about how transfers can be rejected using such URL. Is there any RFCs explaining it?

As I understand "cancelling" and "rejecting" are two different terms in ILP:

  • "cancelling" means creating a cancellation fulfilment matching a cancellation crypto-condition.

  • rejecting means that at some point the execution fulfilment didn't match the execution condition or there was a timeout.

DOUBT: How this 'transfer_rejection' match in the ILP protocol. Does it means the (sender originator?) ledger must fulfil the cancellation fulfilment ? In such case transfer_cancellation (or cancellation_webhook) will be more appropiate.
(Actually I didn't find anything related in the RFCs, only the code in the five-bells implementation)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This API doesn't have any concept of "cancelling" in the ILP sense.

Rejecting transfers uses the "Reject Transfer" method also defined in this document. The receiver of a transfer can reject a conditional transfer if the execution condition hasn't been fulfilled yet.

@earizon
Copy link

earizon commented Jan 24, 2017

In the five-bells-ledger and ilp-plugin-bells there is support for cancellation condition and cancellation fulfillments (https://github.com/interledgerjs/ilp-plugin-bells/blob/master/src/lib/plugin.js, https://github.com/interledgerjs/five-bells-ledger/blob/master/src/models/transfers.js). I don't see any mention about them in this rfc or the 0004 either.

Has cancellation been removed, or is it missing on the rfcs ?

If it hasn't, the final state of a cancelled transfer is also "rejected" or must it be "cancelled"?

@justmoon
Copy link
Member

Has cancellation been removed, or is it missing on the rfcs ?

Cancellation and a few other five-bells-ledger features, such as multi-debit/credit turned out not to be needed for ILP, so they aren't part of the ILP ledger adapter API (IL-RFC-4).

We expect that many ledgers will support more than the minimum feature set, but the Common Ledger API is intended to be pretty close to the minimum required functionality, so those optional features are also excluded here.

@mDuo13
Copy link
Collaborator Author

mDuo13 commented Jul 7, 2017

Rebased this onto the latest master and updated the intro to reflect the new intended goal (formalize the existing 5BL without making all the cosmetic / nice-to-have changes).

@mDuo13 mDuo13 changed the title Common Ledger API - complete draft Five Bells Ledger API specification Jul 10, 2017
@mDuo13 mDuo13 mentioned this pull request Jul 10, 2017
@mDuo13 mDuo13 closed this Jul 10, 2017
@mDuo13
Copy link
Collaborator Author

mDuo13 commented Jul 10, 2017

Closed this in favor of #237. (Same contents, new thread.)

emschwartz added a commit that referenced this pull request Aug 7, 2017
Ledger-Ledger Lotocol is a bit too silly a name to keep. We should call this the Common Ledger Protocol, because it will be common in two senses of the word. First, we think it will be widely used. Second, we're already planning to use the sideProtocolData to extend this protocol into a lot of other ledger protocols. In that sense, this defines the common elements among a lot of different ledger protocols.

Also, when we had previously discussed the Common Ledger API in #125, we didn't know enough about how the ledger protocol would be used to have an idea of what a common ledger interface would look like. I would argue that now we do, and this is our best guess at the common elements that will be used across many ledger protocols.
emschwartz added a commit that referenced this pull request Aug 7, 2017
Ledger-Ledger Lotocol is a bit too silly a name to keep. We should call this the Common Ledger Protocol, because it will be common in two senses of the word. First, we think it will be widely used. Second, we're already planning to use the sideProtocolData to extend this protocol into a lot of other ledger protocols. In that sense, this defines the common elements among a lot of different ledger protocols.

Also, when we had previously discussed the Common Ledger API in #125, we didn't know enough about how the ledger protocol would be used to have an idea of what a common ledger interface would look like. I would argue that now we do, and this is our best guess at the common elements that will be used across many ledger protocols.
michielbdejong pushed a commit that referenced this pull request Aug 7, 2017
Ledger-Ledger Lotocol is a bit too silly a name to keep. We should call this the Common Ledger Protocol, because it will be common in two senses of the word. First, we think it will be widely used. Second, we're already planning to use the sideProtocolData to extend this protocol into a lot of other ledger protocols. In that sense, this defines the common elements among a lot of different ledger protocols.

Also, when we had previously discussed the Common Ledger API in #125, we didn't know enough about how the ledger protocol would be used to have an idea of what a common ledger interface would look like. I would argue that now we do, and this is our best guess at the common elements that will be used across many ledger protocols.
michielbdejong pushed a commit that referenced this pull request Aug 8, 2017
Ledger-Ledger Lotocol is a bit too silly a name to keep. We should call this the Common Ledger Protocol, because it will be common in two senses of the word. First, we think it will be widely used. Second, we're already planning to use the sideProtocolData to extend this protocol into a lot of other ledger protocols. In that sense, this defines the common elements among a lot of different ledger protocols.

Also, when we had previously discussed the Common Ledger API in #125, we didn't know enough about how the ledger protocol would be used to have an idea of what a common ledger interface would look like. I would argue that now we do, and this is our best guess at the common elements that will be used across many ledger protocols.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet