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

feat: add lawallet extension support #2934

Merged
merged 56 commits into from
May 15, 2024

Conversation

agustinkassis
Copy link
Contributor

Describe the changes you have made in this PR

Adding LaWallet.io extension.
Full support with NIP-05 and LUD-16.

Added nostr-tools as lawallet stack requires nostr websocket connection.

Type of change

  • feat: New feature (non-breaking change which adds functionality)

How has this been tested?

You can generate a random private key and test if for yourself.

  • Create Account
  • Receive funds (Generate Invoice)
  • Notify transaction (Zap)
  • Pay Invoice
  • Retrieve Lightning address

Checklist

  • Self-review of changed code
  • Manual testing

@unllamas
Copy link
Contributor

Please 🫶🏻

@Braianalvez
Copy link

Interesting contribution on lightning.

@Fierillo
Copy link

Please consider this PR

LaWallet is a Custodial Bitcon comparable to WoS but linking all the entries up to Nostr in a smart way to provide a good UX and remain secure.

@rapax00
Copy link

rapax00 commented Dec 20, 2023

I need LaWallet in Alby please🤩

@unllamas
Copy link
Contributor

unllamas commented Jan 4, 2024

Hi @bumi @pavanjoshi914 , could you take a look at this?

LaWallet.ar is a custody wallet that we are developing 🫶🏻

@bumi
Copy link
Collaborator

bumi commented Jan 4, 2024

@agustinkassis thanks for the PR!!
@unllamas we're on it and it's on the list.
It would probably be nice to tell users where they can signup for the wallet.

@rolznz
Copy link
Contributor

rolznz commented Mar 8, 2024

For testing:

  1. Create account at https://app.btcpp.ar/

  2. Copy private key from https://app.btcpp.ar/settings/recovery/

  3. Set identity endpoint to https://btcpp.ar

Receiving, sending, transaction list look great. Lightning address works fine.

Things I noticed:

  • In the connector setup it says "Connect to your LaWallet Stack" - maybe just "Connect to LaWallet" (less technical)?
  • When first connecting the balance shows 0. But afterwards the balance is ok. Maybe this should be fixed? otherwise users will think there is an issue with their account.
  • Having to set the identity endpoint was not obvious - I luckily saw you do it in the demo. Will everyone have to do this?
    • If so, can you give instructions on how the user can connect?
    • If you don't have an account or the identity endpoint is wrong, the setup just spins and doesn't show any error

yarn.lock Outdated Show resolved Hide resolved
@rolznz
Copy link
Contributor

rolznz commented Mar 8, 2024

Does Lawallet support re-connecting to the relay? the connector became unresponsive and I had to restart my browser to re-connect.

Also, my balance then showed 0 again and took a while to update.

@reneaaron
Copy link
Contributor

Just tested it and works quite great! 🚀 However I noticed some problems during testing:

  1. Memos for invoices

When I create an invoice and enter a memo the memo seems not to be added to the invoice and won't show up in the payment screen as well as the transaction list.

  1. The WebLN integration also seems to have some problems currently:
  • the preimage isn't returned for webln.sendPayment() calls
  • the payment_hash field seems to contain the invoice
  • the alias seems to contain the username (while that should be something to identify the node, e.g. lacrypta-villanueva)

@agustinkassis
Copy link
Contributor Author

Done with every requirement, please let me know if you have any more suggestions.

@reneaaron reneaaron added next-release To be included in the next release and removed after-next-release Waiting for the next release (e.g. translations) labels Mar 25, 2024
@pavanjoshi914
Copy link
Contributor

pavanjoshi914 commented Apr 18, 2024

@agustinkassis from where i can generate random private key? on btcapp.ar I only see login option which asks me for private key or from where I can create account for lawallet

@agustinkassis
Copy link
Contributor Author

Hey pavan, lawallet.io is a stack, so everyone has a different policy on signing up for a walias (lightning address).
You can get one from https://app.lawallet.ar/signup

A new private key is generated
https://github.com/lawalletio/la-wallet-monorepo/blob/8bcc6dc58bfc58882f2a699b49f152a7c701696f/packages/utils/src/interceptors/identity.ts#L16

@pavanjoshi914
Copy link
Contributor

@agustinkassis any updates here?

@agustinkassis
Copy link
Contributor Author

@agustinkassis any updates here?

Done with all the comments. Everything is up to as requested.

Copy link
Contributor

@pavanjoshi914 pavanjoshi914 left a comment

Choose a reason for hiding this comment

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

there are few minor changes can you please address?

@agustinkassis
Copy link
Contributor Author

agustinkassis commented May 14, 2024

I addressed all the mentioned issues. You can always add customization or optimizations to infinity.
The PR has been delayed for months already. Let's merge it as we agreed before and we can then add more resources for future tweaks.

Copy link
Contributor

@pavanjoshi914 pavanjoshi914 left a comment

Choose a reason for hiding this comment

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

tAck

@pavanjoshi914
Copy link
Contributor

Thanks for the PR @agustinkassis works great! merging it 🚀

@pavanjoshi914 pavanjoshi914 merged commit a87ce1c into getAlby:master May 15, 2024
7 checks passed
@rapax00
Copy link

rapax00 commented May 15, 2024

AMAZING!!! I'm very happy. Thanks @pavanjoshi914 and Alby🐝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-release To be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants