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

bolt12: allow to inject payer_metadata #7786

Conversation

vincenzopalazzo
Copy link
Contributor

Payer metadata is a field that controls the payer ID
provided during the fetchinvoice process.

There are use cases where this is highly useful, such as
proving that the payer has paid for the correct item.

Imagine visiting a merchant's website to pay for multiple offers, where
one of these offers is a default offer (with no description and no set amount).

In this scenario, the merchant could claim not to have received
payment for a specific item. Since the same offer may be used to
fetch invoices for different products, there needs to be a way to
identify which product the invoice corresponds to.

With this commit, it will be possible to inject payer metadata,
which helps solve the issue described above.

For example, possible payer metadata could be to_hex(b"{payer_node_id}.{product_id}.{created_at}").

See 8b457bc for a code example

Opening a PR to give a run to the tests, There is probably something else needs to be hammered but opening as a draft just to take the lock for the upcoming release.


  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/bolt12-payer-metadata branch 2 times, most recently from 0910917 to 28a2882 Compare November 2, 2024 17:15
@vincenzopalazzo
Copy link
Contributor Author

@rustyrussell why CLN is rate limiting the onion message? This is a case that is happening in CI https://github.com/ElementsProject/lightning/actions/runs/11644467250/job/32426697525?pr=7786


===End Flaky Test Report===
=========================== short test summary info ============================
ERROR tests/test_pay.py::test_fetchinvoice_with_payer_metadata - ValueError: 
Node errors:
 - lightningd-2: had warning messages
Global errors:

@rustyrussell
Copy link
Contributor

You want to use invreq_payer_note for this I think?

If metadata is predictable, then someone can grind to reveal all the values in proof of payment, if you publish one.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/bolt12-payer-metadata branch from 28a2882 to f5964a9 Compare November 11, 2024 20:50
@rustyrussell rustyrussell added this to the v24.11 milestone Nov 11, 2024
@rustyrussell
Copy link
Contributor

Doing trivial rebase, and using tok_bin_from_hex.

@rustyrussell rustyrussell force-pushed the macros/bolt12-payer-metadata branch from f5964a9 to 3df0bbe Compare November 18, 2024 02:38
@rustyrussell
Copy link
Contributor

Added a delay in the test (too fast locally), and enhanced fetchinvoice to check args better too.

assert decode1['invreq_payer_id'] != decode2['invreq_payer_id']

# Delay to avoid sending too many onion messages per second!
time.sleep(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch thanks!

Copy link
Contributor Author

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 3df0bbe

Sounds good to me leaving this merge to you, while leaving some question

@@ -835,6 +835,9 @@ struct command_result *json_fetchinvoice(struct command *cmd,
p_opt("recurrence_label", param_string, &rec_label),
p_opt_def("timeout", param_number, &timeout, 60),
p_opt("payer_note", param_string, &payer_note),
/* The payer_metadata is passed in hex format and then converted internally
* in binary when needed */
p_opt("payer_metadata", param_string, &payer_metadata_hex),
Copy link
Contributor

Choose a reason for hiding this comment

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

json_tok_bin_from_hex() does this for you!

plugins/fetchinvoice.c Show resolved Hide resolved
vincenzopalazzo and others added 3 commits November 20, 2024 06:51
Payer metadata is a field that controls the payer ID
provided during the fetchinvoice process.

There are use cases where this is highly useful, such as
proving that the payer has paid for the correct item.

Imagine visiting a merchant's website to pay for multiple offers, where
one of these offers is a default offer (with no description and no set amount).

In this scenario, the merchant could claim not to have received
payment for a specific item. Since the same offer may be used to
fetch invoices for different products, there needs to be a way to
identify which product the invoice corresponds to.

With this commit, it will be possible to inject payer metadata,
which helps solve the issue described above.

For example, possible payer metadata could be `to_hex(b"{payer_node_id}.{product_id}.{created_at}")`.

Changelog-Added: JSON-RPC: `fetchinvoice` allows setting invreq_metadata via `payer_metadata` parameter.
Signed-off-by: Vincenzo Palazzo <[email protected]>
We do a lot more parameter checking than simply parsing, so use
param_check().

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the macros/bolt12-payer-metadata branch from 3df0bbe to 3e6f32b Compare November 19, 2024 20:22
@rustyrussell
Copy link
Contributor

Trivial rebase for flake fixes...

Copy link
Contributor Author

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 3e6f32b

@vincenzopalazzo vincenzopalazzo enabled auto-merge (rebase) November 19, 2024 21:19
@vincenzopalazzo vincenzopalazzo merged commit ba7bf33 into ElementsProject:master Nov 19, 2024
20 of 39 checks passed
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.

2 participants