BOLT 12: re-add recurrence support. #1240
Conversation
This is taken from the old draft, and updated with modern field numbers and names. I've made some minor changes: 1. The `proportional_amount` field comes before `seconds_before` which seems more logical to me. 2. The `offer_absolute_expiry` is clarified to only apply to the *initial* invoice request: once you're recurring the other fields take over. The complexities mainly come from two sources: 1. A requirement to be precise about when recurrence happens. This is because we want a push system, not a pull, since only the former allows for real authorization by the user, and there are many different reasonable ways to do recurrence. 2. This is the first wallet feature which isn't just "fire and forget", but requires them to keep state and do unprompted actions. One proposal I would like to add is a courtesy onion message when a user cancels a periodic payment. This is generally useful to tell the difference between a deliberate cancellation and a user-related failure. Signed-off-by: Rusty Russell <[email protected]>
| - add the offset days to get the day of the period start. | ||
| - if the day is not within the month, use the last day within the month. | ||
| - add the offset seconds to get the period start in seconds. |
There was a problem hiding this comment.
If the day is not within the month, we should use the last day of the month and the last second of that day.
We want a one month period starting on January 30th at 2AM to end before a one month period starting on January 31th at 1AM as it starts almost a full day later. However with your proposal, the former would end an hour after the latter.
There was a problem hiding this comment.
If you're having monthly recurrences, you don't care about +/- a day, BUT we have to define it somehow. I think you'll find this easier to implement as written, so I think that's better.
1. Remove start_any_period. If you want them not to start in a random period (only appliciable if you set `offer_recurrence_base`) then use offer_absolute_expiry. 2. Remove years, use months. 3. Clarify leap seconds. 4. Put `proportional_amount` inside `offer_recurrence_base` where it belongs, not inside paywindow. Signed-off-by: Rusty Russell <[email protected]>
Only offer_recurrence_base is compulsory, really. Signed-off-by: Rusty Russell <[email protected]>
|
OK, updated. I will need to rework the test vectors, however! |
This allows the writer of the offer to decide how pre-recurrence-supporting wallets should behave. Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/offers-recurrence
branch
from
0ddbe23 to
147e86c
Compare
There was a problem hiding this comment.
LGTM overall, I want to take another look later in the month, but probably in this PR we can also remove the FIXME N 7 in the https://github.com/lightning/bolts/blob/master/12-offer-encoding.md#fixme-possible-future-extensions
Subscriptions likely require more communication. From the initial sharing of the offer, to price changes, to notifying a user about a missed payment, providing a receipt, etc. Maybe best to leave this type of communication up to the participants to handle in their medium of choice? |
@GBKS I agree that from a UX perspective there is always going to be more communication required between the subscriber and the recipient. However, if there is a protocol-native way to create a subscription, I think there also needs to be a protocol-native way to cancel the subscription. |
Maybe. You're creating a subscription proposal, but it's not active until the payee accepts it and acts on it. It's more like a subscription request, like we have payment requests. So if you want a "subscription cancelled" message, you may also want a "subscription accepted" message. But TBH, I don't have a super strong opinion. Just doesn't seem 100% right to exclusively have the cancel message. |
|
Note: @TheBlueMatt pointed out that the correct behavior is to send the courtesy "cancel" message in place of the next request. Slightly more complex to implement (your wallet now needs to remember cancelled subscriptions), but it's a nice balance between letting the vendor know, and not letting them cut you off early. Apparently Apple Pay does something similar, and I can see the logic. From an implementation POV, this may simply look like a stub invoice_request? |
That, however, has proven to be a centralized disaster, and severely limits interoperation. You cannot automate this unspecified stuff... Eventually we will probably want offer replacement (where we reject an invoice_request and optionally refers to an updated offer), but one step at a time! |
Thanks https://github.com/a-mpch! Also replace some errant tabs. Signed-off-by: Rusty Russell <[email protected]>
This is a best-effort message, but helps differentiate a deliberate cancellation by a user from wallet issues. Signed-off-by: Rusty Russell <[email protected]>
|
Added the "cancel" flag. You basically send an invreq like normal, but it says "cancel" instead of "send me the next invoice". Also added the requirement to fetch the next invoice explicitly to the requirements for recurring payments! |
Recurrence has the same fields in offer, invoice_request and invoice, and bip_353_name has the same fields in invoice_request and invoice. This is slightly clearer, and it makes code generation cleaner (since they are clearly the same type, they are easier to use). Signed-off-by: Rusty Russell <[email protected]>
1. Fix indent for `invreq_recurrence_cancel` 2. Don't say you can omit `invreq_paths` since you would anyway. 3. Don't allow setting `invreq_recurrence_cancel` in the "not responding to an offer" case. 4. Put the "no future invoices" case under the invoice writer section where it bellongs. 5. Explicitly allow skipping invreq_quantity and invreq_amount on the reader side if we're cancelling (we said you could on the writer side). 6. Add missing check for offer_absolute_expiry on the invoice writer side. 7. Make it clear that you can't issue an invoice if it's already been paid. 8. Disallow `invreq_recurrence_cancel` if not a recurring invoice. Signed-off-by: Rusty Russell <[email protected]>
|
|
||
| Thus, each offer containing a recurring payment has: | ||
| 1. A `time_unit` defining 0 (seconds), 1 (days), 2 (months). | ||
| 2. A `period`, defining how often (in `time_unit`) it has to be paid. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks great on the first pass! 🚀
I’ve got a few questions and points I’d like to discuss. If some (or all 😄) have already been covered in earlier conversations I may have missed, please excuse the repetition. And in case I’ve misunderstood anything, please feel free to correct me. Thanks!
Feedback from https://github.com/shaavan 1. rename max_period to max_period_index for a little more clarity. 2. clarify `offer_absolute_expiry` example.
| - MAY use `offer_recurrence_optional` if a payment of a single period would be useful (compatibility with pre-recurrence readers). | ||
| - MUST NOT use `offer_recurrence_optional` if `offer_recurrence_base` is present. | ||
| - if it includes either `offer_recurrence_optional` or `offer_recurrence_compulsory`: | ||
| - MUST set `time_unit` to 0 (seconds), 1 (days), 2 (months) or 3 (years). |
There was a problem hiding this comment.
time_unit = 3 (years) seems to have been removed from the spec, should we also remove it from the writer/reader instructions for consistency?
| - MUST NOT use `offer_recurrence_optional` if `offer_recurrence_base` is present. | ||
| - if it includes either `offer_recurrence_optional` or `offer_recurrence_compulsory`: | ||
| - MUST set `time_unit` to 0 (seconds), 1 (days), 2 (months) or 3 (years). | ||
| - MUST set `period` to how often (in `time-unit`) it wants to be paid. |
There was a problem hiding this comment.
Should we add an explicit constraint for period != 0 to avoid ambiguity? Something like:
Writer → MUST NOT set period to 0
Reader → MUST NOT respond to offers with period set to 0
8 participants
This is taken from the old draft, and updated with modern field numbers and names.
I've made some minor changes:
proportional_amountfield comes beforeseconds_beforewhich seems more logical to me.offer_absolute_expiryis clarified to only apply to the initial invoice request: once you're recurring the other fields take over.The complexities mainly come from two sources:
One proposal I would like to add is a courtesy onion message when a user cancels a periodic payment. This is generally useful to tell the difference between a deliberate cancellation and a user-related failure.