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

Paycodes #2794

Closed
wants to merge 5 commits into from
Closed

Paycodes #2794

wants to merge 5 commits into from

Conversation

ZmnSCPxj
Copy link
Collaborator

@ZmnSCPxj ZmnSCPxj commented Jul 4, 2019

Implement waitnewpaycode command, which allows node to receive without triggering waitanyinvoice.

Intent is to be used by self-paying plugins without potentially interfering with merchant software that is polling waitanyinvoice.

Needs test.

Fixes #2666

@rustyrussell
Copy link
Contributor

Why doesn't a spontaneous plugin create an invoice, then pay it? That fits better with existing software, AFAICT?

@ZmnSCPxj
Copy link
Collaborator Author

ZmnSCPxj commented Jul 5, 2019

Paying an invoice triggers waitanyinvoice. Other software that is running waitanyinvoice may interoperate badly with plugins that use an invoice and pay it. Better to have something that clearly separates the waitanyinvoice user from plugins that self-pay.

Since the plugin is likely to operate transiently, I opted to make paycodes into a memory-only structure that is explicitly not kept in database. Thus, not-well-written plugins will not uselessly waste disk space on invoices they end up not paying for various reasons.

@cdecker
Copy link
Member

cdecker commented Jul 5, 2019

Couldn't the same thing be achieved by using the htlc_accepted hook to trigger something upon receiving an incoming payment that doesn't have an associated invoice? We could also have an incoming_payment hook (I think @rustyrussell suggested that at some point) that only triggers if not forwarding.

@ZmnSCPxj
Copy link
Collaborator Author

ZmnSCPxj commented Jul 5, 2019

Yes, the same can be done with htlc_accepted, I do mention this in #2666 19 days ago. However, currently hooks can only have a single plugin connected to them, and there are a bunch of things that would like to use htlc_accepted:

  • Cross-chain swap nodes as per @bitonic-cjp
  • Spontaneous payments via circular routing.
  • Rebalancing via circular routing.
  • JIT-Routing as per @renepickhardt (detecting and intercepting HTLCs before they are forwarded, then using the above rebalancing via circular routing)

Writing a single plugin to do all of the above would be unwieldy (and would break single responsibility principle). At least with waitnewpaycode self-paying plugins can be written independently of each other and other plugins that would want to hook into htlc_accepted.

@cdecker
Copy link
Member

cdecker commented Jul 6, 2019

Writing a single plugin to do all of the above would be unwieldy (and would break single responsibility principle). At least with waitnewpaycode self-paying plugins can be written independently of each other and other plugins that would want to hook into htlc_accepted.

Absolutely agree, we shouldn't force users to combine these things into a single plugin. I had envisioned more of a chain-of-responsibility pattern being implemented as a top-level plugin that could then run a number of sub-plugins and combine their decisions into a single final decision to pass up to lightningd.

Would maybe a notification be sufficient for your use-cases as well? That'd be truly trivial to implement, and if desired we could extend the notifications to the JSON-RPC as well (though we might need to deal with high-watermarks in that case in order not to miss notifications). That seems slightly more desirable than a long-polling RPC call alone.

@ZmnSCPxj
Copy link
Collaborator Author

ZmnSCPxj commented Jul 6, 2019

Absolutely agree, we shouldn't force users to combine these things into a single plugin. I had envisioned more of a chain-of-responsibility pattern being implemented as a top-level plugin that could then run a number of sub-plugins and combine their decisions into a single final decision to pass up to lightningd.

Please see #2796 . I would prefer the chain-of-responsibility be at the lightningd level, since it appears to me that such a "plugin hook multiplexer" would need to figure out how to demultiplex the "normal" commands registered by the sub-plugins.

Would maybe a notification be sufficient for your use-cases as well?

The advantage of waitnewpaycode is that it is a single command and is a resource that automatically cleans up after itself. Consider how a separate paycode system would require deletion and iteration of paycodes, and how it would require periodic cleaning a la autocleaninvoices, especially if plugins are unstable and crash (the plugin may very well forget that it allocated a paycode before it crashed and is later restarted). waitnewpaycode would simply remove the paycode once expired, solving the "unstable plugin" issue.

@rustyrussell
Copy link
Contributor

Paying an invoice triggers waitanyinvoice. Other software that is running waitanyinvoice may interoperate badly with plugins that use an invoice and pay it. Better to have something that clearly separates the waitanyinvoice user from plugins that self-pay.

If you don't partition your invoices somehow you'll always have problems with others. This is no different.

We really, really, really want invoices and payments to balance. No matter if the invoice is generated spontaneously, this gives documentation to exactly what's happening, and doesn't require any new infrastructure (AFAICT).

@ZmnSCPxj
Copy link
Collaborator Author

If you don't partition your invoices somehow you'll always have problems with others. This is no different.

So what is the plan now? Add a partition field to invoice and waitanyinvoice?

@rustyrussell
Copy link
Contributor

No, I would just close this. Labels are free-form strings: I'd suggest a spontaneous plugin prepend "spontaneous-plugin" to the label they generate for the invoice. Problem solved.

@shesek
Copy link
Contributor

shesek commented Jul 15, 2019

FWIW, Lightning Charge would handle unknown invoices coming from waitanyinvoice gracefully. It would look them up in its own db using the label, print a warning about the unknown invoice and carry on. (using a fixed prefix like charge/ would help avoid this db lookup, I created ElementsProject/lightning-charge#65 to track this)

As for Spark, this will pop up as a new incoming payment notification and add the paid invoice to the list of incoming payments. Which is kinda weird, I guess. How would you suggest to handle this?

@ZmnSCPxj ZmnSCPxj closed this Jul 16, 2019
@ZmnSCPxj
Copy link
Collaborator Author

@shesek have Spark keep track of its own-generated invoices? What is intent of Spark?

@ZmnSCPxj ZmnSCPxj deleted the paycodes branch July 16, 2019 05:39
@shesek
Copy link
Contributor

shesek commented Jul 16, 2019

@ZmnSCPxj Spark is a wallet GUI for c-lightning. Its intentionally stateless, relies entirely on the rpc and has no db of its own, so there's currently no easy way to keep track of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: lower-level "paycode" below "invoice" level
4 participants