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

Added custom_records to metadata field of list_transactions #251

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ericpp
Copy link

@ericpp ericpp commented Feb 28, 2024

This PR adds custom records from invoices/payments to the metadata field of the list_transactions response. Based on the comments in the code, this seems to be where it was intended to be added, but I can move it to its own field if desired.

This addition would be useful to any application that sends and receives boostagrams as that data is stored in the custom records of each payment/invoice.

The code follows the Alby API convention of base64 encoding the values of each custom record. I moved the existing Alby metadata to an alby key in the new metadata structure. I'm not sure what metadata is being returned by the Alby API as all of mine are empty and it's not well documented in the docs: https://guides.getalby.com/developer-guide/v/alby-wallet-api/reference/api-reference/invoices#get-incoming-invoice-history .

If accepted, I wonder if we shouldn't also add a separate param in list_transactions to conditionally request metadata rather than sending it by default to save space. Users would need to request custom_records be returned by adding a "metadata": ["custom_records"] parameter or have a list, e.g. "metadata": ["custom_records", "alby"]

@ericpp ericpp force-pushed the add-transaction-custom-records branch from d5d919a to 3be2aba Compare February 28, 2024 07:49
@rolznz
Copy link
Contributor

rolznz commented Jun 14, 2024

Hi @ericpp, very sorry for the long delay, we're just getting around to this. I've added a PR in the extension to actually read this: getAlby/lightning-browser-extension#3172

We will add this to Alby Hub first (https://github.com/getAlby/nostr-wallet-connect-next) as this is the new version of our NWC app which we will onboard Alby users to so they can run their own nodes. It also currently supports the LND connector: getAlby/hub#60

Regarding the Alby (shared node) custom records, this is still relevant, but needs to be moved to the nwc.getalby.com Alby fork. There hasn't been much demand for it yet and we will need to review the priority for this one since we are trying to migrate Alby users to run their own node.

@ericpp
Copy link
Author

ericpp commented Jun 19, 2024

Thanks @rolznz . I think this addition will be very helpful for podcast apps that want to use NWC to connect to wallets.

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

2 participants