From 6e3b33629c32215629c44da75d358b6f9876515b Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Jul 2021 15:23:06 -0700 Subject: [PATCH 1/4] cmd/lncli: always parse the pay_addr field for sendpayment In this commit, we fix a bug that would cause attempts to re-use an AMP invoice to fail. Without this commit, we would only attempt to parse the payment addr if no invoice was specified, so a user manually specifying the pubkey of the detonation. The fix is straight forward: always parse the `pay_addr` field as the user may be attempting to re-use an AMP invoice w/o open coding each of the sections. --- cmd/lncli/cmd_pay.go | 54 ++++++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/cmd/lncli/cmd_pay.go b/cmd/lncli/cmd_pay.go index 76f0d5d11..cb9d449aa 100644 --- a/cmd/lncli/cmd_pay.go +++ b/cmd/lncli/cmd_pay.go @@ -235,6 +235,31 @@ func confirmPayReq(resp *lnrpc.PayReq, amt, feeLimit int64) error { return nil } +func parsePayAddr(ctx *cli.Context) ([]byte, error) { + var ( + payAddr []byte + err error + ) + switch { + case ctx.IsSet("pay_addr"): + payAddr, err = hex.DecodeString(ctx.String("pay_addr")) + case ctx.Args().Present(): + payAddr, err = hex.DecodeString(ctx.Args().First()) + } + + if err != nil { + return nil, err + } + + // payAddr may be not required if it's a legacy invoice. + if len(payAddr) != 0 && len(payAddr) != 32 { + return nil, fmt.Errorf("payment addr must be exactly 32 "+ + "bytes, is instead %v", len(payAddr)) + } + + return payAddr, nil +} + func sendPayment(ctx *cli.Context) error { // Show command help if no arguments provided if ctx.NArg() == 0 && ctx.NumFlags() == 0 { @@ -242,6 +267,8 @@ func sendPayment(ctx *cli.Context) error { return nil } + args := ctx.Args() + // If a payment request was provided, we can exit early since all of the // details of the payment are encoded within the request. if ctx.IsSet("pay_req") { @@ -250,6 +277,16 @@ func sendPayment(ctx *cli.Context) error { Amt: ctx.Int64("amt"), } + // We'll attempt to parse a payment address as well, given that + // if the user is using an AMP invoice, then they may be trying + // to specify that value manually. + payAddr, err := parsePayAddr(ctx) + if err != nil { + return err + } + + req.PaymentAddr = payAddr + return sendPaymentRequest(ctx, req) } @@ -259,8 +296,6 @@ func sendPayment(ctx *cli.Context) error { err error ) - args := ctx.Args() - switch { case ctx.IsSet("dest"): destNode, err = hex.DecodeString(ctx.String("dest")) @@ -352,22 +387,11 @@ func sendPayment(ctx *cli.Context) error { req.FinalCltvDelta = int32(delta) } - var payAddr []byte - switch { - case ctx.IsSet("pay_addr"): - payAddr, err = hex.DecodeString(ctx.String("pay_addr")) - case args.Present(): - payAddr, err = hex.DecodeString(args.First()) - } - + payAddr, err := parsePayAddr(ctx) if err != nil { return err } - // payAddr may be not required if it's a legacy invoice. - if len(payAddr) != 0 && len(payAddr) != 32 { - return fmt.Errorf("payment addr must be exactly 32 "+ - "bytes, is instead %v", len(payAddr)) - } + req.PaymentAddr = payAddr return sendPaymentRequest(ctx, req) From c235b46e66990e2d8c277aa7d44ccd5c6cf0947e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 4 Aug 2021 18:54:42 -0700 Subject: [PATCH 2/4] cmd/lncli: add new option for easy AMP invoice re-use In this commit, we add a new option (`--amp-reuse`) that allows for easy AMP invoice re-use when using either the `sendpayment` command. --- cmd/lncli/cmd_pay.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cmd/lncli/cmd_pay.go b/cmd/lncli/cmd_pay.go index cb9d449aa..dcb7d96be 100644 --- a/cmd/lncli/cmd_pay.go +++ b/cmd/lncli/cmd_pay.go @@ -93,6 +93,12 @@ var ( Usage: "if set to true, then AMP will be used to complete the " + "payment", } + + ampReuseFlag = cli.BoolFlag{ + Name: "amp-reuse", + Usage: "if set to true, then a random payment address will " + + "be generated to enable re-use of an AMP invoice", + } ) // paymentFlags returns common flags for sendpayment and payinvoice. @@ -138,6 +144,7 @@ func paymentFlags() []cli.Flag { }, dataFlag, inflightUpdatesFlag, maxPartsFlag, jsonFlag, maxShardSizeSatFlag, maxShardSizeMsatFlag, ampFlag, + ampReuseFlag, } } @@ -243,6 +250,16 @@ func parsePayAddr(ctx *cli.Context) ([]byte, error) { switch { case ctx.IsSet("pay_addr"): payAddr, err = hex.DecodeString(ctx.String("pay_addr")) + + case ctx.IsSet(ampReuseFlag.Name): + var addrBytes [32]byte + if _, err := rand.Read(addrBytes[:]); err != nil { + return nil, fmt.Errorf("unable to generate pay "+ + "addr: %v", err) + } + + payAddr = addrBytes[:] + case ctx.Args().Present(): payAddr, err = hex.DecodeString(ctx.Args().First()) } From 57d70ee9ce03bca1a2028ad2441c9b9ebcf3fb50 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Jul 2021 15:29:25 -0700 Subject: [PATCH 3/4] docs/release-notes: add item for sendpayment pay_addr bug fix --- docs/release-notes/release-notes-0.13.2.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 docs/release-notes/release-notes-0.13.2.md diff --git a/docs/release-notes/release-notes-0.13.2.md b/docs/release-notes/release-notes-0.13.2.md new file mode 100644 index 000000000..d94939c30 --- /dev/null +++ b/docs/release-notes/release-notes-0.13.2.md @@ -0,0 +1,12 @@ +# Release Notes + +# Bug Fixes + +[A bug has been fixed in the command line argument parsing for the +`sendpayment` command that previously prevented users from being able to re-use +a fully +specified AMP](https://github.com/lightningnetwork/lnd/pull/5554) invoice by +generating a new `pay_addr` and including it as a CLI arg. + +# Contributors (Alphabetical Order) +* Olaoluwa Osuntokun From 9505402ec2f3908379857b7f54c050c7c3b24c47 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 4 Aug 2021 18:57:08 -0700 Subject: [PATCH 4/4] docs/release-notes: add item for easy AMP CLI re-use --- docs/release-notes/release-notes-0.13.2.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/release-notes-0.13.2.md b/docs/release-notes/release-notes-0.13.2.md index d94939c30..f76cbedc8 100644 --- a/docs/release-notes/release-notes-0.13.2.md +++ b/docs/release-notes/release-notes-0.13.2.md @@ -1,6 +1,12 @@ # Release Notes -# Bug Fixes +## AMP + +A new command line option (`--amp-reuse`) has been added to make it easier for +users to re-use AMP invoice on the command line using either the `payinvoice` +or `sendpayment` command. + +## Bug Fixes [A bug has been fixed in the command line argument parsing for the `sendpayment` command that previously prevented users from being able to re-use