Feature/keysend #699

kiwiidb · 2022-03-16T08:10:56Z

Link this PR to an issue

Fixes #671

Type of change

New feature (non-breaking change which adds functionality)

Describe the changes you have made in this PR -

Added a new keysend screen that is mostly copied over from the LNurlPay screen. This screen is triggered either by pasting in a pubkey in the send box (by checking for length and starting characters), or by a webln.keysend call.
Added more fields to the sendPaymentArgs to allow for keysend and offer payments (later).
Implements the keysend method for the LND and LNDhub connectors.
Adds a webLN keysend function.

Screenshots of the changes (If any) -

-> Be aware that the pubkey might not be associated with the website. It could be another user of the website, or a pubkey associated with a podcast feed.

How Has This Been Tested?

webLN tested on https://amboss.space - check out the keysend billboard. Tested with lndhub.go backend.
by pasting in random pubkeys and doing payments. Get a pubkey from lnd-2 on rtl.regtest.getalby.com - connect to alby-simnet-lnd-1 in the extension - make payment. Check status on RTL.

I don't have any tests and also the code could use some refactoring since I have copy-pasted quite a bit. I think it would be more opportune if someone else picks this up.

Checklist:

My code follows the style guidelines of this project and performed a self-review of my own code
New and existing tests pass locally with my changes
I have made corresponding changes to the documentation (If Any)

Implement keysend for lndhub-based accounts.

Initial work on keysend for lnd connector

Keysend working with LND.

WebLN keysend PoC working

Save payments and allowances. Allow automated payments.

Use an object as the webln argument

github-actions · 2022-03-16T08:17:31Z

Build files:

bumi · 2022-03-16T09:05:27Z

src/extension/background-script/connectors/lnd.ts

+        dest: dest_pubkey_base64,
+        amt: args.amount,
+        payment_hash: hash,
+        dest_custom_records: {


can we allow passing in of this custom records? that developers can decide what records to set?

e.g. I think the podcasting uses a special one: https://github.com/lightning/blips/blob/master/blip-0010.md

AaronDewes · 2022-03-16T09:36:17Z

I'll implement Keysend in Citadel 0.0.2 too, so the Citadel connector can be updated.

bumi · 2022-03-17T13:26:11Z

src/app/screens/Keysend.tsx

+
+      setSuccessAction({
+        tag: "message",
+        message: `Keysend paid! Preimage: ${payment.preimage}`,


maybe just Payment sent! ? people don't need to know it's a keysend.

bumi · 2022-03-17T13:28:55Z

src/extension/background-script/connectors/connector.interface.ts

@@ -88,6 +92,7 @@ export default interface Connector {
  getBalance(): Promise<GetBalanceResponse>;
  makeInvoice(args: MakeInvoiceArgs): Promise<MakeInvoiceResponse>;
  sendPayment(args: SendPaymentArgs): Promise<SendPaymentResponse>;
+  sendPaymentKeySend(args: SendPaymentArgs): Promise<SendPaymentResponse>;


I'd call this keysend? why add sendPayment?

bumi · 2022-03-17T13:29:33Z

src/extension/background-script/connectors/eclair.ts

@@ -92,6 +92,11 @@ class Eclair implements Connector {
    };
  }

+  async sendPaymentKeySend(
+    args: SendPaymentArgs


those should be KeysendArgs - it's different to what sendpayment accepts

bumi · 2022-03-17T13:30:00Z

src/extension/background-script/connectors/lnd.ts

@@ -81,7 +83,58 @@ class Lnd implements Connector {
      };
    });
  }
+  async sendPaymentKeySend(
+    args: SendPaymentArgs


those should be KeysendArgs - it's different to what sendpayment accepts

bumi · 2022-03-17T13:36:42Z

src/extension/background-script/connectors/connector.interface.ts

@@ -43,6 +43,10 @@ export type SendPaymentResponse =

 export interface SendPaymentArgs {
  paymentRequest: string;
+  offer: string;


SendPaymentArgs are very different to what is passed to keysend. We should create a new interface for that.

src/extension/background-script/connectors/lnd.ts

bumi · 2022-03-17T13:40:41Z

src/extension/background-script/connectors/lnd.ts

@@ -1,5 +1,7 @@
 import Base64 from "crypto-js/enc-base64";
 import UTF8 from "crypto-js/enc-utf8";
+import CryptoJS from "crypto-js";
+import _ from "lodash";


we load only the module we need.

import random from "lodash/random";

bumi · 2022-03-17T13:42:06Z

src/extension/background-script/connectors/lnd.ts

+    for (let i = 0; i < 32; i++) {
+      const randomNumber = _.random(0, 255);
+      byteArray[i] = randomNumber;
+    }


we should move this into a utils function: utils.randomHex(32) or something.
@dylancom do you know a nicer way on how to get a random hex string?

This currently won't work, need to serialize the custom record map to json somehow.

kiwiidb · 2022-03-17T16:03:06Z

I've fixed @bumi 's comments.

However the code currently does not work, I'm not sure on how to pass in a map all the way to the network call. I used a TypeScript Map, but this won't serialize into json. How to best do this?

Also, would we only allow this to be called by webln, or would we also allow people just pasting in a pubkey? Would we allow custom records in the latter case? Or just a comment (TLV_WHATSAT_MESSAGE)?

dylancom · 2022-03-17T16:15:06Z

I've fixed @bumi 's comments.

However the code currently does not work, I'm not sure on how to pass in a map all the way to the network call. I used a TypeScript Map, but this won't serialize into json. How to best do this?

Also, would we only allow this to be called by webln, or would we also allow people just pasting in a pubkey? Would we allow custom records in the latter case? Or just a comment (TLV_WHATSAT_MESSAGE)?

I would suggest using an object of shape: Record<string, string>.

bumi · 2022-03-17T18:29:33Z

Also, would we only allow this to be called by webln, or would we also allow people just pasting in a pubkey? Would we allow custom records in the latter case? Or just a comment (TLV_WHATSAT_MESSAGE)?

I think this can be done later. I'd release it without the comment option and add the comment option later.
Imo it's better to release small things fast and thus also reduce complexity

Keysend can now only be triggered from webLN.

kiwiidb · 2022-03-18T10:47:07Z

Left out the option of a manual comment.
Keysend can now only be triggered from webLN:

await webln.keysend({destination: "025c1d5d1b4c983cc6350fc2d756fbb59b4dc365e45e87f8e3afe07e24013e8220", amount: "100", customRecords: {"test123":"test"}})

bumi · 2022-03-18T14:10:23Z

src/extension/background-script/actions/webln/keysendOrPrompt.ts

+  }
+
+  if (await checkAllowance(message.origin.host, parseInt(amount))) {
+    return sendPaymentWithAllowance(message, true);


I am not a fan of magic boolean flags. When I read this code here it is unclear what this true means.
looking into sendPaymentWithAllowance this function basically now does two things based on the boolean flag.
I would split this up and use here a new keySendWithAllowance method. it is more descriptive and we don't need to add some if in those methods sendPayment methods

bumi · 2022-03-18T14:10:23Z

src/extension/background-script/actions/webln/keysendOrPrompt.ts

+  }
+
+  if (await checkAllowance(message.origin.host, parseInt(amount))) {
+    return sendPaymentWithAllowance(message, true);


I am not a fan of magic boolean flags. When I read this code here it is unclear what this true means.
looking into sendPaymentWithAllowance this function basically now does two things based on the boolean flag.
I would split this up and use here a new keySendWithAllowance method. it is more descriptive and we don't need to add some if in those methods sendPayment methods

This tries to follow the exact same structure as the webln.sendPayment call. With that it also introduces a new "confirmKeysend" screen that is used in the prompt to confirm the keysend payment. This is only to confirm webln payments and thus for example the amount can also not be changed. When doing manual keysend payments we should use a new screen that allows the user to set the amount and potentially also other fields as a comment/message.

this allows users to do keysend payments from the send screen

repy before showing the success message. this typically closes the prompt but just in case the component is used somewhere else we still show the success message

Some keysend suggestions

kiwiidb · 2022-03-21T09:37:02Z

I've added some commits that handle the encoding of the custom records. The custom record values passed in through webln should now be plain strings (these strings can be serialized json). Depending on the connector used, they are either encoded as base64 (LND connector) or as hex (LNDhub connector).

* master: refactor: revert makeinvoice changes refactor: revert makeinvoice changes refactor: get rid of union type refactor: add error response type Revert "refactor: throw instead of succeeding with an error" refactor: throw instead of succeeding with an error

import only needed lib

kiwiidb added 6 commits March 11, 2022 09:33

feat: keysend

7e363b7

Implement keysend for lndhub-based accounts.

feat: keysend

f293622

Initial work on keysend for lnd connector

feat: keysend

78e10a3

Keysend working with LND.

feat: keysend

9a97506

WebLN keysend PoC working

feat: keysend

3062180

Save payments and allowances. Allow automated payments.

feat: keysend

fb3c1d9

Use an object as the webln argument

kiwiidb requested review from bumi and dylancom March 16, 2022 08:10

bumi reviewed Mar 16, 2022

View reviewed changes

kiwiidb added 2 commits March 17, 2022 14:34

Merge branch 'master' into feature/keysend

94b9fb1

fix: add placeholder keysend to citadel connector

2354f84

bumi reviewed Mar 17, 2022

View reviewed changes

fix: address bumi's PR comments

5af18b6

This currently won't work, need to serialize the custom record map to json somehow.

Merge branch 'master' into feature/keysend

ee3695f

kiwiidb added 2 commits March 17, 2022 18:04

fix: serializing custom records

68ead12

fix: webln customRecords: use Record

2216aff

fix: keysend: don't allow comment

5ae2c78

Keysend can now only be triggered from webLN.

kiwiidb requested a review from bumi March 18, 2022 10:47

bumi reviewed Mar 18, 2022

View reviewed changes

bumi added 4 commits March 18, 2022 16:32

chore: move random Hex generation to utils function

b80fba6

fix: typo

682a7ec

feat: add keysend option to manual send flow

8fe26df

this allows users to do keysend payments from the send screen

bumi and others added 4 commits March 19, 2022 10:57

chore: break long words like the node pubkey

fe577e1

chore: cleanup

ffc5566

repy before showing the success message. this typically closes the prompt but just in case the component is used somewhere else we still show the success message

chore: formatting

b28d3cd

Merge pull request #708 from getAlby/feature/keysend-bumi

a5ecb92

Some keysend suggestions

kiwiidb requested a review from bumi March 19, 2022 14:14

kiwiidb added 2 commits March 21, 2022 10:30

fix: lnd connector: base64 encode custom records

31e7928

fix: lndhub connector: hex encode custom records

6d34e24

bumi added 10 commits March 21, 2022 14:40

Merge branch 'master' into feature/keysend

4d02547

docs: add back reference comment

ba60e1b

chore: cleanup as we now have confirmPayment and keysend separate

57718b4

chore: add keysend placeholder function to galoy

e7409e9

chore: follow error handling convention

dd10602

chore: follow keysend naming convention

32a17a3

chore: rename keysend file to follow naming convention

e7f78fa

chore: use crypto-js encoding and randomness

9b68793

chore: cleanup

1ca58a2

import only needed lib

dylancom merged commit 2524351 into master Mar 22, 2022

dylancom deleted the feature/keysend branch March 22, 2022 09:11

kiwiidb mentioned this pull request Mar 22, 2022

Add keysend method definition joule-labs/webln#42

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/keysend #699

Feature/keysend #699

kiwiidb commented Mar 16, 2022 •

edited

Loading

github-actions bot commented Mar 16, 2022

bumi Mar 16, 2022

AaronDewes commented Mar 16, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

bumi Mar 17, 2022

kiwiidb commented Mar 17, 2022

dylancom commented Mar 17, 2022 •

edited

Loading

bumi commented Mar 17, 2022

kiwiidb commented Mar 18, 2022

bumi Mar 18, 2022

bumi Mar 18, 2022

kiwiidb commented Mar 21, 2022

Feature/keysend #699

Feature/keysend #699

Conversation

kiwiidb commented Mar 16, 2022 • edited Loading

Link this PR to an issue

Type of change

Describe the changes you have made in this PR -

Screenshots of the changes (If any) -

How Has This Been Tested?

Checklist:

github-actions bot commented Mar 16, 2022

Choose a reason for hiding this comment

AaronDewes commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiwiidb commented Mar 17, 2022

dylancom commented Mar 17, 2022 • edited Loading

bumi commented Mar 17, 2022

kiwiidb commented Mar 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiwiidb commented Mar 21, 2022

kiwiidb commented Mar 16, 2022 •

edited

Loading

dylancom commented Mar 17, 2022 •

edited

Loading