-
Notifications
You must be signed in to change notification settings - Fork 512
BOLT 12: re-add recurrence support. #1240
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
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]>
0ddbe23
to
147e86c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]>
This is taken from the old draft, and updated with modern field numbers and names.
I've made some minor changes:
proportional_amount
field comes beforeseconds_before
which seems more logical to me.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:
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.