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

Enables SPSP for pull payments #501

Closed
wants to merge 21 commits into from

Conversation

sabineschaller
Copy link
Member

Following the discussions in #499 and with @sharafian, this addition adds the possibility for pull payments to the protocol.

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'm not a huge fan of adding more details to the SPSP response. If anything, I'd probably support taking out (or making optional) all of the details aside from the ones needed to establish a STREAM connection (destination_account and shared_secret). For pull payments in particular, it seems more useful to have a standard format for the merchant to request a token, rather than for them to have a way to query information about a token they've already gotten.


* The `destination_account` should be used as the STREAM destinationAccount.
* The `shared_secret` should be decoded from base64 and used as the STREAM sharedSecret.
* The `pull_balance.refill_amount` SHOULD be used as the STREAM `receiveMax`.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't take into account the exchange rate between the server's units and the client's units. The amounts returned in the SPSP server response are in the server's units (and the client may not know the exchange rate from the server to them).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is correct. @justmoon, @sharafian, @adrianhopebailie and I had a chat about this and had the idea that there should be a set of reference currencies, i.e. very common fiat currencies and maybe the most accepted crypto currencies, that the merchant and the customer decide on and this is what gets pulled, irrespective of what both of them hold in their wallets. A field for this needs to be added.

* The `destination_account` should be used as the STREAM destinationAccount.
* The `shared_secret` should be decoded from base64 and used as the STREAM sharedSecret.
* The `pull_balance.refill_amount` SHOULD be used as the STREAM `receiveMax`.
* The `pull_balance.current_amount` MUST NOT be bigger than `balance.maximum`.
Copy link
Member

Choose a reason for hiding this comment

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

Should this be pull_balance.maximum_amount?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. And one could also add that pull_balance.maximum_amount MUST NOT be bigger than balance.current.

Suggested change
* The `pull_balance.current_amount` MUST NOT be bigger than `balance.maximum`.
* The `pull_balance.current_amount` MUST NOT be bigger than `pull_balance.maximum_amount`.
* `pull_balance.maximum_amount` MUST NOT be bigger than `balance.current`.

| `pull_balance` | Object | _(OPTIONAL)_ Details of this pull payment token. Used only for pull payment accounts. |
| `pull_balance.maximum_amount` | Integer String | Maximum amount, denoted in the minimum divisible units of the server's account, which can be pulled from the server. This represents the highest sum that outgoing chunks are allowed to reach, not the highest size of an individual chunk (which is determined by `pull_balance.refill_amount`). |
| `pull_balance.current_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that can currently be pulled from the account. |
| `pull_balance.refill_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that will be added to `pull_balance.current_amount` whenever now is later than `pull_balance.refill_date`. |
Copy link
Member

Choose a reason for hiding this comment

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

The way this is written, it suggests that the token represents a certain amount per month with rollover. I would kind of expect $10 per month to mean that if the merchant only uses $9 this month, they lose the extra $1.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, if the merchant 'forgets' to pull the last $1 he is entitled to, he should lose it.

Suggested change
| `pull_balance.refill_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that will be added to `pull_balance.current_amount` whenever now is later than `pull_balance.refill_date`. |
| `pull_balance.refill_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that `pull_balance.current_amount` will be refilled to whenever now is later than `pull_balance.refill_date`. |

| `asset_info.code` | String | Asset code to identify the receiver's currency. Currencies that have [ISO 4217](https://en.wikipedia.org/wiki/ISO_4217) codes should use those. Sender UIs SHOULD be able to render non-standard codes |
| `asset_info.scale` | Integer | The scale of the amounts on the receiver's account (e.g. an amount of `"1000"` with a scale of `2` translates to `10.00` units of the receiver's asset/currency) |
| `pull_balance` | Object | _(OPTIONAL)_ Details of this pull payment token. Used only for pull payment accounts. |
| `pull_balance.maximum_amount` | Integer String | Maximum amount, denoted in the minimum divisible units of the server's account, which can be pulled from the server. This represents the highest sum that outgoing chunks are allowed to reach, not the highest size of an individual chunk (which is determined by `pull_balance.refill_amount`). |
Copy link
Member

Choose a reason for hiding this comment

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

the highest size of an individual chunk (which is determined by pull_balance.refill_amount).

This wording is a bit confusing. I think of a "chunk" as an individual ILP packet, but I think this should be referring to the amount in a refill period, no?

Also, is the pull_balance.maximum_amount the max amount per period or the total amount ever? If I have a subscription for $10 per month, I'm not sure you'd want to have a cap on the total value to be pulled (if you want to limit the subscription, it seems more straightforward to have it expire after a certain number of months)

Copy link
Member Author

Choose a reason for hiding this comment

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

pull_balance.maximum_amount was supposed to be the maximum overall that can be pulled from this endpoint. But I agree, defining the end by means of time is more straight forward. Paypal for example uses the notion of cycles, i.e. they have frequency=week, frequency_interval=2, and cycles=4, meaning that this pull payment agreement would be valid for 8 weeks.

| `asset_info.scale` | Integer | The scale of the amounts on the receiver's account (e.g. an amount of `"1000"` with a scale of `2` translates to `10.00` units of the receiver's asset/currency) |
| `pull_balance` | Object | _(OPTIONAL)_ Details of this pull payment token. Used only for pull payment accounts. |
| `pull_balance.maximum_amount` | Integer String | Maximum amount, denoted in the minimum divisible units of the server's account, which can be pulled from the server. This represents the highest sum that outgoing chunks are allowed to reach, not the highest size of an individual chunk (which is determined by `pull_balance.refill_amount`). |
| `pull_balance.current_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that can currently be pulled from the account. |
Copy link
Member

Choose a reason for hiding this comment

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

current_amount sounds like it could mean either "how much you've already taken" or "how much you currently have left", so it might be better to pick a less ambiguous term

Copy link
Member Author

Choose a reason for hiding this comment

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

I am very bad at naming so any suggestions are welcome. What about pullable_amount?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or available_amount

| `asset_info` | Object | _(OPTIONAL)_ Details about the destination asset, for sender's display purposes. |
| `asset_info.code` | String | Asset code to identify the receiver's currency. Currencies that have [ISO 4217](https://en.wikipedia.org/wiki/ISO_4217) codes should use those. Sender UIs SHOULD be able to render non-standard codes |
| `asset_info.scale` | Integer | The scale of the amounts on the receiver's account (e.g. an amount of `"1000"` with a scale of `2` translates to `10.00` units of the receiver's asset/currency) |
| `pull_balance` | Object | _(OPTIONAL)_ Details of this pull payment token. Used only for pull payment accounts. |
Copy link
Member

Choose a reason for hiding this comment

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

It seems more helpful to me to have a way for the merchant to request a token with specific parameters than to have a way for the merchant to check the details of the token they've got

Copy link
Member Author

Choose a reason for hiding this comment

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

Does that need to be part of the protocol though or is that part or the negotiation phase that happens before and by other means?

Copy link
Member

Choose a reason for hiding this comment

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

That would be a separate negotiation, not part of SPSP (or more precisely, not part of the single HTTPS request we now think of as SPSP; we could "include" the token and token-request procedure in SPSP or call them separate but linked protocols).

| `pull_balance.maximum_amount` | Integer String | Maximum amount, denoted in the minimum divisible units of the server's account, which can be pulled from the server. This represents the highest sum that outgoing chunks are allowed to reach, not the highest size of an individual chunk (which is determined by `pull_balance.refill_amount`). |
| `pull_balance.current_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that can currently be pulled from the account. |
| `pull_balance.refill_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that will be added to `pull_balance.current_amount` whenever now is later than `pull_balance.refill_date`. |
| `pull_balance.refill_date` | String | [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) timestamp in UTC representing the date after which the next pull payment of maximum `pull_balance.refill_amount` can be made, given that `pull_balance.maximum_amount` has not been reached yet. On setting, it has to be in the future. |
Copy link
Member

Choose a reason for hiding this comment

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

This suggests that the money can only be pulled on that date, rather than after that timestamp.

Also, is this a date or date + time?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking about date and time, so the correct name for it would probably be refill_time. I don't see though that this suggests that one can only pull on one specific date.

| `pull_balance.current_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that can currently be pulled from the account. |
| `pull_balance.refill_amount` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that will be added to `pull_balance.current_amount` whenever now is later than `pull_balance.refill_date`. |
| `pull_balance.refill_date` | String | [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) timestamp in UTC representing the date after which the next pull payment of maximum `pull_balance.refill_amount` can be made, given that `pull_balance.maximum_amount` has not been reached yet. On setting, it has to be in the future. |
| `pull_balance.refill_interval` | String | Interval of time that has to pass before a new pull payment can be made. `pull_balance.refill_date` will be incremented by this interval. Possible time units are `day`, `week`, `month`, and `year`. |
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 quite understand this. Is this the amount of time that will pass after the refill_date before the amount is refilled again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly. refill_date_old + refill_interval = refill_date_new. This probably needs to be split into something like 'frequency' (day, week, ...) and 'frequency_interval' (e.g. 2). What do you think?

@sabineschaller
Copy link
Member Author

After some discussions and taking the comments above into consideration, I updated the response body fields, taking Paypal Subscriptions as an example. However, I tried not to over-complicate things, using as little fields as possible.

| `pull_info.refill_time` | String | [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) UTC timestamp, e.g. "2019-02-10T01:01:13Z", representing the time at which `pull_balance.available` will be filled up to the amount of `pull_balance.refill_maximum` for the next time.
| `pull_info.frequency` | String | Frequency at which `pull_info.refill_time` is incremented. Possible values are `DAY`, `WEEK`, `MONTH`, `YEAR`. |
| `pull_info.frequency_interval` | Integer | Interval associated with `pull_info.frequency`. For example, if `pull_info.frequency` is `WEEK` and `pull_info.frequency_interval` is `2`, `pull_balance.available` is refilled every 2 weeks.
| `pull_info.asset_code` | String | Asset code to identify the currency agreed upon for this pull payment. Currencies that have [ISO 4217](https://en.wikipedia.org/wiki/ISO_4217) codes should use those. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Should pull_info instead have asset_info with code and scale?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that but was not sure. I think you still need the "external" asset_info field because this describes the currency of the server whereas pull_info.asset_code is the currency that the sender and the receiver agreed upon when they negotiated the terms of the pull payment token. However, it does make sense to include the asset_scale unless everything is in the minimum divisible unit of that currency.
I just realized that I have to change the descriptions in pull_balance to not say "units of the server's account" but "units of the agreed upon currency".

| `pull_balance.available` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that can currently be pulled from the account. |
| `pull_balance.refill_maximum` | Integer String | Amount, denoted in the minimum divisible units of the server's account, that `balance.available` will be filled up to at `pull_info.refill_time`. |
| `pull_info` | Object | _(OPTIONAL)_ Non-monetary details of a pull payment token. Used only for pull payments. |
| `pull_info.start_time` | String | [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) UTC timestamp, e.g. "2019-01-10T01:01:13Z", representing the time at which `pull_balance.available` will be filled up to the amount of `pull_balance.refill_maximum` for the first time. |
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like refill_time makes start_time either redundant or irrelevant.
Unless we need that historical information (how long has this been going on), it could be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree.

wilsonianb and others added 2 commits January 10, 2019 17:36
Co-Authored-By: sabinebertram <sabine.bertram@mailbox.org>
Copy link
Member

@adrianhopebailie adrianhopebailie left a comment

Choose a reason for hiding this comment

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

This is not really an issue with this PR per se, but perhaps a bigger issue with SPSP in general.

It is very difficult to infer the entities involved or the relationships between the entities involved in a payment from the data model of the SPSP response. As such it's hard to reason about the design.

Does an SPSP document represent a payment, a receiver, a server or just some collection of data?

SPSP has evolved so that we can now make the following assumptions:

  • An SPSP response contains at least an ILP address and secret which are used by the client to create a STREAM connection.
  • We CAN'T assume that the entity hosting the server is the owner of the ILP address in the response.
  • We CAN'T assume that the client is always the sender in subsequent payments.

Perhaps then the best way to model this is to think of an SPSP response as representing a STREAM connection (or more precisely a session, although I'm not sure if STREAM uses that terminology or differentiates between the two).

The ILP Address is the unique identifier for the session and the secret is the "bearer token" that allows the client to create the session.

The remainder of the SPSP response defines properties of the session.

Some suggestions:

Use balance for both push and pull. Negative means the client is expected to be a sender. It shouldn't need asset details like scale and code because these are shared when the STREAM connection is established. Having them in the SPSP response may be useful but then we need to define how a client should behave if it gets different info via SPSP and STREAM.

Rename receiver to counterparty or some similar non-specific name.

Separate validity data related to the session (start and expiry) from data about frequency (i.e. how often is the balance on the connection changed).

I think these changes make SPSP much cleaner but they are also not backwards compatible so probably need to use a new protocol version header.


### Interfaces

SPSP may be used by end-user applications, such as a digital wallet with a user interface for the sender to initiate payments. SPSP clients and receivers use ILP modules to send and receive Interledger payments. SPSP [payment-pointers](../0026-payment-pointers/0026-payment-pointers.md) can be used as a persistent identifier on Interledger. SPSP payment-pointers can also be used as a unique identifier for an invoice to be paid.
SPSP may be used by end-user applications, such as a digital wallet with a user interface for the sender to initiate payments. SPSP clients and receivers use ILP modules to send and receive Interledger payments. SPSP [payment-pointers](../0026-payment-pointers/0026-payment-pointers.md) can be used as a persistent identifier on Interledger. SPSP payment-pointers can also be used as a unique identifier for an invoice to be paid, or, in the case of pull payments, as a token that specifies the terms and conditions for such a pull.
Copy link
Member

Choose a reason for hiding this comment

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

The replacing of 'receiver' with 'server' and 'sender' with 'client' is a little inconsistent.


SPSP messages MUST be exchanged over HTTPS.

### Operation

Any SPSP receiver will run an SPSP server and expose an HTTPS endpoint called the SPSP Endpoint. The sender can query this endpoint to get information about the type of payment that can be made to this receiver. The sender can set up and send multiple ILP payments using the details provided by the receiver.
Any SPSP server will expose an HTTPS endpoint called the SPSP Endpoint. The client can query this endpoint to get information about the type of payment that can be made to or pulled from this server. The client can set up and send or pull multiple ILP payments using the details provided by the server.
Copy link
Member

Choose a reason for hiding this comment

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

Suggest replacing:

The client can set up and send or pull multiple ILP payments using the details provided by the server.

with

The client can use this information to establish a STREAM connection and begin sending or receiving payments.

* **SPSP Client** - The sender application that uses SPSP to interact with the SPSP Server
* **SPSP Server** - The server used on the receiver's side to handle SPSP requests
* **SPSP Client** - The application that uses SPSP to interact with the SPSP Server
* **SPSP Server** - The server used to handle SPSP requests
* **SPSP Endpoint** - The specific HTTPS endpoint on the SPSP server used for setting up a payment
Copy link
Member

Choose a reason for hiding this comment

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

We should define the entity that handles the incoming STREAM connection. Maybe call it the STREAM server? It may or may not be the same entity as the SPSP server but is also not necessarily the receiver/payee.

5. The sender's and receiver's STREAM modules will move as much value as possible while staying inside these bounds.
6. If the sender reaches their `sendMax`, they end the stream and the connection. If the receiver reaches their `receiveMax`, they will end the STREAM and the connection.
1. The user's SPSP Client queries the server's SPSP Endpoint.
2. The SPSP Endpoint responds with the server info, including the servers's ILP address and the shared secret to be used in STREAM. It MAY respond with additional information if it is an invoice server or a pull payment server.
Copy link
Member

Choose a reason for hiding this comment

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

Is the response "server info"?
Perhaps this could simply be:

The SPSP Endpoint responds with an ILP address and a shared secret to be used in a new STREAM connection. It MAY respond with additional information if it is an invoice server or a pull payment server.


In case of simple push payments or invoices, the process continues as follows:

3. The client constructs an ILP payment using the server's ILP address.
Copy link
Member

Choose a reason for hiding this comment

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

Terminology is nuanced here. What is an "ILP payment"?

Strictly speaking a "payment" could be completed through the exchange of multiple ILP packets. i.e. A payment is not a technical concept but a packet is and ILP has no concept of non-technical concepts.

I think this step could be left out entirely.

3. The client establishes a STREAM connection to the server using the server's ILP address.
4. The server begins sending the payment.
1. The server will construct a stream to the client on the ILP/STREAM connection.
2. The server will adjust their `sendMax` to reflect the amount they're willing to send.
Copy link
Member

Choose a reason for hiding this comment

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

As above, it would be useful to explain how the values of sendMax and receiveMax relate to the values in the SPSP response or even link to another part of the document where this is explained.


## Specification

The SPSP endpoint is a URI used by the sender to query information about the receiver (which may be an invoice) and set up payments. The SPSP endpoint URI MUST NOT contain query string parameters. The sender SHOULD treat the URI as opaque. There are several supported ways to refer to an SPSP endpoint:
The SPSP endpoint is a URI used by the client to query information about the server (which may be an invoice or a pull payment token) and set up payments. The SPSP endpoint URI MUST NOT contain query string parameters. The sender SHOULD treat the URI as opaque. There are several supported ways to refer to an SPSP endpoint:
Copy link
Member

Choose a reason for hiding this comment

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

The server may "return" an invoice or token? (As opposed to "be").

Suggest using "URL" instead of "URI" as the spec defines the query protocol as HTTP over TLS.

The SPSP endpoint URI MUST NOT contain query string parameters. The sender SHOULD treat the URI as opaque.

If this is re-written as testable normative requirements for the client and server I'd rather say:

Clients MUST NOT send query string parameters in requests to the SPSP endpoint URI. Servers that receive query string parameters in an SPSP request MUST reject the request with a 400 Bad Request HTTP response code. Clients SHOULD treat the URI as opaque and not depend on any information they derive from the URI.

@@ -67,7 +80,7 @@ The SPSP Endpoint MUST respond to HTTPS `GET` requests in the following manner:

### Query (`GET <SPSP Endpoint>`)

The sender queries the SPSP endpoint to get information about the type of payment that can be made to this receiver:
The client queries the SPSP endpoint to get information about the type of payment that can be made to this server:
Copy link
Member

Choose a reason for hiding this comment

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

Suggest:

The client queries the SPSP endpoint to get information about the payment:

@@ -115,6 +128,39 @@ Content-Type: application/spsp4+json
}
```

In case of a pull payment server, it is possible to request information about a particular payment that can be pulled from this server using the corresponding token, i.e.
Copy link
Member

Choose a reason for hiding this comment

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

Possibly reword so this doesn't imply that the payment is "pulled from the server" but rather that the server provides details (and a token) that allow the bearer to to pull a payment from a sender.

}
```
More information about the parameters can be found in section [Respond Body](#response-body).

##### Response Headers

The response MUST contain at least the following headers:
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:

The response MUST contain at least the following fields

@sabineschaller
Copy link
Member Author

sabineschaller commented Jan 18, 2019

Use balance for both push and pull. Negative means the client is expected to be a sender.

I really like that idea.

@sabineschaller
Copy link
Member Author

It shouldn't need asset details like scale and code because these are shared when the STREAM connection is established. Having them in the SPSP response may be useful but then we need to define how a client should behave if it gets different info via SPSP and STREAM.

I do believe we need that because the server could be in XRP, the client in ETH, but the pull agreement was made in a fiat currency, e.g. USD. They both need to use this to adjust their sendMax and receiveMax.


In a UI, the `asset_info` and `receiver_info` objects (if present) can be used for display purposes. These objects can be manipulated by the receiver in any way, so amounts SHOULD be displayed in source units when possible.
4. The SPSP server begins sending ILP packets to fulfill the payment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it instead be the client constructing the stream to initiate the pull payment?

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 way I have it implemented right now is that the client connects to the server (i.e. opens the STREAM connection) and on that connection, the server opens the stream and sends the payments to the client. So technically, it is not a pull payment but the client requests the server to send him the agreed upon amount (or something less than that) right now (on connection).

…rotocol.md


fix typo

Co-Authored-By: sabinebertram <sabine.bertram@mailbox.org>
Copy link
Member

@justmoon justmoon left a comment

Choose a reason for hiding this comment

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

My main comment would be that pull payments should not be part of the SPSP spec. I think the SPSP spec should have some provisions for extensibility, e.g. how to define additional fields for the SPSP response, and then there should be a different RFC that specifies pull payments.

I think this goes in line with @emschwartz's comment not to add more to the SPSP response. My recommendation would be to view SPSP more like HTTP - a general-purpose baseline that is very extensible for different use cases. (Of which there could be many. I keep thinking back on ISO 20022's dedicated schemas for wage garnishment.)

@adrianhopebailie
Copy link
Member

My recommendation would be to view SPSP more like HTTP - a general-purpose baseline that is very extensible for different use cases.

In that vein, maybe the ilp address and secret shouldn't be returned in the response but rather in headers.

This frees up the response to be a regular HTTP response and have different types.

i.e. SPSP is simply a spec for augmenting any HTTPS response with two standard headers?

@sabineschaller
Copy link
Member Author

i.e. SPSP is simply a spec for augmenting any HTTPS response with two standard headers?

This would mean making the SPSP spec slimmer and introducing two new specs, one for push and one for pull payments over SPSP, @adrianhopebailie?

@adrianhopebailie
Copy link
Member

@sabinebertram yes, that's my proposal but interested to hear from @sharafian @justmoon and @emschwartz too. We've all talked in circles around SPSP for some time and it came up on the community call yesterday too.

My proposal, concretely, is this:

  • Any HTTPS request can be treated as an SPSP request by a web server. (By default a Payment Pointer is resolved to a URL and client performs a GET on that URL but this is specified in the Payment Pointer spec not the SPSP spec.)
  • The HTTP response should contain headers that carry the ILP address and the shared secret to use to establish the STREAM connection.
  • The definition of the HTTP response body and content-type can be defined in another spec (and possibly even leverage existing standards for eInvoices etc)
  • This could also be leveraged by websites as an alternative to a Web Monetization <meta> tag in the page (@sharafian @justmoon ?).

A proposal on the header format is to follow the same pattern as the authorization header: <protocol> <details>. Here <protocol> could be STREAM and <details> could be the base64 string that decodes to : (to avoid case changes in-flight)

Aside: I drafted a spec for the IETF a while back that proposed bringing the payment request from the W3C into HTTP: https://tools.ietf.org/html/draft-dold-payto-02

I think that this proposal is a better option than the previous IETF draft because it doesn't need to carry the details of the payment in the header, just the details of the counter-party and how to establish a secure "payment connection" with them.

If we can settle on a spec for this we should consider also submitting to IETF.

@emschwartz
Copy link
Member

This discussion is moving beyond the scope of the original PR. I'd suggest opening a separate issue to discuss changing SPSP to be just headers.

FWIW, I'm hesitant to make any breaking changes that aren't very substantive (like moving the STREAM secret and address from the body of a given request to the headers), but this would mean effectively merging SPSP and the HTTP-ILP specs, which doesn't sound terrible. I'm still doubtful how important this is for pull payments because it seems like the main issues are a) how to get the token that you'll use to make the SPSP request or get the STREAM details and b) the format of that token (rather than how to query details about the parameters of the token once you have it).

@sabineschaller
Copy link
Member Author

After discussions with @sharafian and @wilsonianb, I made the SPSP spec backwards compatible by leaving the destination_account and the shared_secret in the body, however, removing the notion of push and pull payments form the spec. It only defines how to request these information and that they are used for creating a stream connection.
I created two new specs called SPSP Push Payments and SPSP Pull Payments that concentrate on the payments and the parameters in the response body.

Copy link
Member

@adrianhopebailie adrianhopebailie left a comment

Choose a reason for hiding this comment

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

I really like the new split between 3 specs.

Some clarity on how the SPSP request and STREAM connection are correlated would be useful especially for implementors that wish to implement stateless servers.


4. The SPSP client begins sending ILP packets to fulfill the payment.
1. The client will adjust their `sendMax` to reflect the amount they're willing to send.
* If present, `\|balance.current\|` SHOULD be used as the STREAM `sendMax`.
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 think this is correct. The client may be sending in a different currency.

The amount the client is willing to send is a business decision based on the expected amount (balance.max - balance.current), the anticipated exchange rate, and the slippage the client will allow.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. That was blind copy-pasting. I'll fix it.

"refill_time": "2019-02-10T01:01:13Z",
"expiry_time": "2019-07-10T01:01:12Z"
},
"receiver_info": {
Copy link
Member

Choose a reason for hiding this comment

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

This is a new specification. We don't need to use receiver_info for backwards compatibility and this is actually the sender.

Can we use a more neutral term like "counterparty"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually had the receiver in mind. This could be useful for an end user application (a wallet supporting pull payments) to display details about this particular pull agreement. But maybe there should be a field called counterparty_info that contains the details about sender and receiver.


## Model of Operation

### Creating a token
Copy link
Member

Choose a reason for hiding this comment

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

Creating a token doesn't seem to be an interoperability requirement.

The end result is a Payment Pointer that contains a bearer token. How that is created doesn't need to be standardised.

I would recommend noting that this part of the specification is not normative.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough.


3. The SPSP client establishes a STREAM connection to the server using the server's ILP address and shared secret and constructs a stream to the client on the ILP/STREAM connection.

4. The SPSP server begins sending ILP packets to fulfill the payment.
Copy link
Member

Choose a reason for hiding this comment

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

Some mention of how the server identifies that this connection is related to the SPSP request would be useful.

Is the ILP address unique per transaction or is the identifier the secret?

When a connection is established with a specific ILP address and secret I assume the server knows which payment it relates to and so can set the appropriate sendMax?

Is there a way to do this that doesn't require the server to store this data somewhere (i.e. allow the server to be stateless)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of storing the information, the token could be a JWT. The client could connect to the endpoint without suffix, i.e. to /.well-known/pay instead of /tokenUUID and if the first thing that is received by the server is a JWT, it parses and validates it and streams the according funds.
However, one would probably still have to store balances somewhere to make sure that the client doesn't connect and pull more than allowed.

@sabineschaller
Copy link
Member Author

I updated the basic SPSP spec and the pull payment spec according to our discussions on the Forum (Pull Payments and Push Payments and Invoices). The invoice spec still needs work but there have not been any further discussions.

@sabineschaller
Copy link
Member Author

I'm closing this PR and will open individual ones for the new 'basic SPSP' spec as well as the stand-alone pull payment spec. I may also open one on invoices but there have not been any further discussions regarding that spec on the forum so it is on pause.

This was referenced Feb 27, 2019
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

6 participants