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 Human Readable Offers #1989

Closed
wants to merge 19 commits into from
Closed

Bolt12 Human Readable Offers #1989

wants to merge 19 commits into from

Conversation

chdwlch
Copy link
Contributor

@chdwlch chdwlch commented Feb 20, 2024

Description

An example of human readable offers using DNS TXT records. This is an early POC attempt inspired by the latest discussions around human readable names mapping to payment info. The goal of this draft is to gauge interest and hopefully help push bolt12 forward.
The bolt12 address option is currently only enabled for core lightning nodes with experimental offers enabled.

Simulator Screenshot - iPhone SE (3rd generation) - 2024-02-19 at 18 42 30
Simulator Screenshot - iPhone SE (3rd generation) - 2024-02-19 at 18 42 45
Simulator Screenshot - iPhone SE (3rd generation) - 2024-02-19 at 18 42 50
Screenshot 2024-02-19 at 6 47 16 PM
Screenshot 2024-02-19 at 6 47 38 PM
Screenshot 2024-02-19 at 6 47 48 PM
Screenshot 2024-02-19 at 6 47 57 PM

This pull request is categorized as a:

  • New feature

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

  • Embedded LND
  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (c-lightning-REST)
  • LndHub
  • [DEPRECATED] Core Lightning (Spark)
  • [DEPRECATED] Eclair

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

@kaloudis
Copy link
Contributor

Awesome first time contribution!

Copy link
Contributor

Choose a reason for hiding this comment

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

There is an AddressSvg you can use here instead that is effectively the same symbol

@@ -7,5 +7,5 @@
#
# Customize the NODE_BINARY variable here.
# For example, to use nvm with brew, add the following line
# . "$(brew --prefix nvm)/nvm.sh" --no-use
. "$(brew --prefix nvm)/nvm.sh" --no-use
Copy link
Contributor

Choose a reason for hiding this comment

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

was this change required for your env? would rather it be unchanged

Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific changes here necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to remove these tags for NFC?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to remove these tags for NFC?

@@ -839,6 +840,10 @@
"views.Settings.Contacts.noAddress": "No Address",
"views.Settings.Contacts.to": "To",
"views.Settings.Contacts.deleteAllContacts": "Delete all contacts",
"views.Settings.Bolt12Address.bolt12Address": "Bolt12 Address",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we change between styling it as Bolt 12 and Bolt12. We should be consistent


if (BackendUtils.supportsOffers()) {
const [localPart, domain] = value.split('@');
const dnsUrl = 'https://cloudflare-dns.com/dns-query';
Copy link
Contributor

Choose a reason for hiding this comment

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

love to see the query code already in place. this is a well trusted resolver but I'm curious if there are any other ways we can do the query

}

try {
const res = await fetch('https://twelve.cash/record', {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have some qualms about adding a third party service here. I'll have to do some more research about it.

getNewOffer = () =>
this.postRequest('/v1/offers/offer', {
amount: 'any',
description: 'Bolt12 Payment Address'
Copy link
Contributor

Choose a reason for hiding this comment

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

do we perhaps want to add the ability to generate Bolt12 offers manually on the receive page too?

@kaloudis
Copy link
Contributor

Closing in favor of #2207

Want to leave this branch intact just in case we need to review old changes for whatever reason.

@kaloudis kaloudis closed this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants