alexbosworth / ln-service

Node.js interface to LND
MIT License
318 stars 61 forks source link

Can't pay invoices with more than one hop_hints #159

Closed grunch closed 1 year ago

grunch commented 2 years ago

I tried to pay this invoice using payviapaymentrequest(), I passed it lnd, request and max_fee, I can pay all others invoices without any problem using payviapaymentrequest(), the only new variable is this invoice have two hop_hints, I have a direct channel with that node, I can pay invoices with two hints using lncli payinvoice so I think the problem is with this function.

lnbc1u1p32pw5upp59gxqy2rk6j48wnx8uw7gwnzjpepyj29dwyu94t679e0lxxdx8t9qdqqcqzzgxqyz5vqrzjqw8c7yfutqqy3kz8662fxutjvef7q2ujsxtt45csu0k688lkzu3lduhe3h5f3n95tsqqqqryqqqqfvqqpyrzjqw8c7yfutqqy3kz8662fxutjvef7q2ujsxtt45csu0k688lkzu3lduhe3h5f3n95tsqqqqryqqqqthqqpysp5ylp63qhz38ez5a8mlw53axtjjs353hmca0zal9qfha7sstjc3hxq9qypqsqvjgu3ntzhvdm8rx50hhfkg2gegsxxtwhrn5yf6k807utknau4ejhnc006scgkknmmq23cp2zltuq4vc8ytapw6t50h6x4hp5m2x8hsqpc8lvwk
alexbosworth commented 2 years ago

lncli doesn't use the same settings so it's not a 1:1 mapping, you can try customizing your pay settings

grunch commented 2 years ago

lncli doesn't use the same settings so it's not a 1:1 mapping, you can try customizing your pay settings

What do you mean with customizing pay settings?

alexbosworth commented 2 years ago

Customizing the arguments to the method

grunch commented 2 years ago

I just need to pay the invoice, what argument would let me pay that kind of invoice?

alexbosworth commented 2 years ago

I'm not sure, it depends on the scenario

Something to try is replicating in regtest using https://github.com/alexbosworth/ln-docker-daemons

grunch commented 2 years ago

I forgot to mention that I tried to pay that invoice 4 times and all of them I got this error message 503,PaymentPathfindingFailedToFindPossibleRoute

alexbosworth commented 2 years ago

That indicates LND could not find any possible route to attempt for the given request

grunch commented 2 years ago

Yes, but it works if I just run a lncli payinvoice, I tested with another invoice from them, those are the new invoices from muun wallet

grunch commented 2 years ago

This is happening in production with @lnp2pbot

https://github.com/grunch/p2plnbot/blob/main/ln/pay_request.js#L28

alexbosworth commented 2 years ago

Did you try probing first and then paying via the found route

grunch commented 2 years ago

I will try that

alexbosworth commented 2 years ago

Did you make sure to set the same max fee on both attempts?

grunch commented 2 years ago

Yeah, I have a more than enough max fee and also I have a direct channel with the node that is in the hint, also it's a 100 sats invoice, the fee is very low.

grunch commented 2 years ago

I getting a 404,UnknownPaymentHash error, what I am doing wrong?

    const invoice = parsePaymentRequest({ request });
    if (!invoice) {
      return false;
    }

    // We need to set a max fee amount
    const maxFee = amount * parseFloat(process.env.MAX_ROUTING_FEE);
    const maxFeeMsats = Math.round(maxFee * 1000).toString();

    // Probe to find a successful route
    const { route } = await probeForRoute({
      lnd,
      destination: invoice.destination,
      tokens: amount,
      cltv_delta: invoice.cltv_delta,
      max_fee_mtokens: maxFeeMsats,
    });

    const payment = await payViaRoutes({
      lnd,
      id: invoice.id,
      routes: [route],
    });

Error:

[
  [
    404,
    'UnknownPaymentHash',
    {
      channel: undefined,
      height: 281,
      index: 1,
      mtokens: undefined,
      policy: null,
      timeout_height: undefined,
      update: undefined
    }
  ]
]
alexbosworth commented 2 years ago

That means you reached the end destination but they rejected it

You probably need to include [payment]: <Payment Identifier Hex String> invoice.payment

grunch commented 2 years ago

I added payment and total_mtokens, I got this one now

    const { route } = await probeForRoute({
      lnd,
      destination: invoice.destination,
      mtokens,
      total_mtokens: mtokens,
      cltv_delta: invoice.cltv_delta,
      payment: invoice.payment,
      max_fee_mtokens: maxFeeMsats,
    });

And it works with one hint invoices, but with two hints invoices I get this error

0|bot  | [2022-06-09T15:40:01.214+00:00] error: Cannot read properties of undefined (reading 'hops') TypeError: Cannot read properties of undefined (reading 'hops')
0|bot  |     at /home/micro/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:108:51
0|bot  |     at Array.findIndex (<anonymous>)
0|bot  |     at validate (/home/micro/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:108:25)
0|bot  |     at runTask (/home/micro/p2plnbot/node_modules/async/auto.js:295:13)
0|bot  |     at /home/micro/p2plnbot/node_modules/async/auto.js:233:31
0|bot  |     at processQueue (/home/micro/p2plnbot/node_modules/async/auto.js:243:13)
0|bot  |     at auto (/home/micro/p2plnbot/node_modules/async/auto.js:230:5)
0|bot  |     at /home/micro/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:93:12
0|bot  |     at new Promise (<anonymous>)
0|bot  |     at module.exports (/home/micro/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:92:10)
alexbosworth commented 2 years ago

interesting, can you show me exactly the arguments you are using for the two hints? I think maybe the two hints aren't formatted in the way it's expecting, how did you derive the hints as passed to this method?

grunch commented 2 years ago

TBH I am not doing anything with the hint, I am just getting the route and then used it on payViaRoutes() with the same payment hash.

Here is the invoice:

lnbc1u1p32y92cpp5h30utxca7htf5mntgcgw0zjlj3hs2qhzfldk8uuvn2tzfscvtunsdqqcqzzgxqyz5vqrzjqw8c7yfutqqy3kz8662fxutjvef7q2ujsxtt45csu0k688lkzu3ldmjkhqu5755u2qqqqqryqqqqthqqpysp5dx7rtaa4jxvydyny8sm24cnl2z4zrf3qknssjs0wne33u4jc9hzs9qypqsqx7fc934av9g6m4f3fm4dx82qswkg8cc6pm2p8s58ezwa9md250u4aej9ta96hdmkettpvvqq9afz9g84wwnw9hgmfgfs896td0v6pwspttxhlc

How would be the correct way of proceed with invoices like this one?

alexbosworth commented 2 years ago

I'm confused, you said with one hint it works and two it doesn't, however you don't pass the hints from the payreq into the probe for route?

alexbosworth commented 2 years ago

Another thing to pass to probe_for_route is routes from invoice, are you doing that?

grunch commented 2 years ago

Yeah right, let me clarify it.

When I open this issue I said that I was using payviapaymentrequest(), this function was working ok to pay invoices, but it fails when I try to pay this new invoices with two hints.

You told me to try probing first and then paying via the found route, then I started to test stuff changing the way of pay with probeForRoute() and then payViaRoutes(), it works great with polar, but now I am thinking that this way it will fail with all wallets cause now I am ignoring the hints.

So I change it back to payviapaymentrequest() and I can pay one hint invoices and can't pay invoices with two hints, I at the same place than when I open the issue.

alexbosworth commented 2 years ago

Why ignore the hints with probeForRoute? You can pass the hints in the routes field

alexbosworth commented 2 years ago

Do you have a specific test case you are using with regtest that I can look at?

pay and payviapaymentrequest don't touch the route hints when passing to LND because they are encoded into the bolt11 request string, which is then just given directly to LND

grunch commented 2 years ago

Why ignore the hints with probeForRoute? You can pass the hints in the routes field

There is some things that I don't see clearly, for example, this is the response I have from parsePaymentRequest({ request: 'lnbc1u1p32yyu5pp528mep0fty3zvqjdqdmyhw8v73nlz7r9ms5eghvafv4uzs74f8q7qdqqcqzzgxqyz5vqrzjqw8c7yfutqqy3kz8662fxutjvef7q2ujsxtt45csu0k688lkzu3ldgndjcr8ln0whsqqqqryqqqqthqqpyrzjqwnvuc0u4txn35cafc7w94gxvq5p3cu9dd95f7hlrh0fvs46wpvhdgndjcr8ln0whsqqqqryqqqq86qqpysp5ganyq0k0syfvld7xn53ret9mnca333gq4tmxhxvz4dwj4xfpjn3q9qypqsqjq4kdju96l9xjvxjy69ddme463uc749ys7h8nkawllvsfpzylk3q2dhggdvxyzgl799340e9ze8vleatxs5ext5zule58kfjs7qyn8gqajllw2' }):

{
  destination: '03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af',
  network: 'bitcoin',
  chain_addresses: undefined,
  cltv_delta: 72,
  created_at: '2022-06-09T15:36:52.000Z',
  expires_at: '2022-06-10T15:36:52.000Z',
  features: [
    { bit: 15, is_required: false, type: 'payment_identifier' },
    { bit: 9, is_required: false, type: 'tlv_onion' }
  ],
  mtokens: '100000',
  routes: [
  [
    {
      public_key: '038f8f113c580048d847d6949371726653e02b928196bad310e3eda39ff61723f6'
    },
    {
      base_fee_mtokens: '100',
      channel: '10644886x425933x61116',
      cltv_delta: 9,
      fee_rate: 1500,
      public_key: '03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af'
    }
  ],
  [
    {
      public_key: '03a6ce61fcaacd38d31d4e3ce2d506602818e3856b4b44faff1dde9642ba705976'
    },
    {
      base_fee_mtokens: '100',
      channel: '10644886x425933x61116',
      cltv_delta: 9,
      fee_rate: 1000,
      public_key: '03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af'
    }
  ]
],
  safe_tokens: 100,
  tokens: 100,
  id: '51f790bd2b2444c049a06ec9771d9e8cfe2f0cbb85328bb3a96578287aa9383c',
  payment: '4766403ecf8112cfb7c69d223cacbb9e3b18c500aaf66b9982ab5d2a992194e2',
  is_expired: false,
  description: ''
}

That's a bit different that lncli decodepayreq:

{
    "destination": "03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af",
    "payment_hash": "51f790bd2b2444c049a06ec9771d9e8cfe2f0cbb85328bb3a96578287aa9383c",
    "num_satoshis": "100",
    "timestamp": "1654789012",
    "expiry": "86400",
    "description": "",
    "description_hash": "",
    "fallback_addr": "",
    "cltv_expiry": "72",
    "route_hints": [
        {
            "hop_hints": [
                {
                    "node_id": "038f8f113c580048d847d6949371726653e02b928196bad310e3eda39ff61723f6",
                    "chan_id": "11704175961263959740",
                    "fee_base_msat": 100,
                    "fee_proportional_millionths": 1500,
                    "cltv_expiry_delta": 9
                }
            ]
        },
        {
            "hop_hints": [
                {
                    "node_id": "03a6ce61fcaacd38d31d4e3ce2d506602818e3856b4b44faff1dde9642ba705976",
                    "chan_id": "11704175961263959740",
                    "fee_base_msat": 100,
                    "fee_proportional_millionths": 1000,
                    "cltv_expiry_delta": 9
                }
            ]
        }
    ],
    "payment_addr": "4766403ecf8112cfb7c69d223cacbb9e3b18c500aaf66b9982ab5d2a992194e2",
    "num_msat": "100000",
    "features": {
        "9": {
            "name": "tlv-onion",
            "is_required": false,
            "is_known": true
        },
        "15": {
            "name": "payment-addr",
            "is_required": false,
            "is_known": true
        }
    }
}
alexbosworth commented 2 years ago

what is different exactly? the naming of the fields?

grunch commented 2 years ago

Do you have a specific test case you are using with regtest that I can look at?

pay and payviapaymentrequest don't touch the route hints when passing to LND because they are encoded into the bolt11 request string, which is then just given directly to LND

I am testing on polar locally, yes what you say makes sense but still I can't pay an invoice with two hints payviapaymentrequest, do you think that is just coincidence and if I keep trying in some point the payment will success?

alexbosworth commented 2 years ago

if you want to pass hints to probe for route take the routes you get from parsePaymentRequest and pass them to probeForRoute as routes

alexbosworth commented 2 years ago

Maybe there is an issue but I don't have enough information to say one way or the other since I can't see exactly what you are doing and I'd need to construct my own new test scenario to say for sure

grunch commented 2 years ago

if you want to test, it's just trying to pay lnbc1u1p32yyu5pp528mep0fty3zvqjdqdmyhw8v73nlz7r9ms5eghvafv4uzs74f8q7qdqqcqzzgxqyz5vqrzjqw8c7yfutqqy3kz8662fxutjvef7q2ujsxtt45csu0k688lkzu3ldgndjcr8ln0whsqqqqryqqqqthqqpyrzjqwnvuc0u4txn35cafc7w94gxvq5p3cu9dd95f7hlrh0fvs46wpvhdgndjcr8ln0whsqqqqryqqqq86qqpysp5ganyq0k0syfvld7xn53ret9mnca333gq4tmxhxvz4dwj4xfpjn3q9qypqsqjq4kdju96l9xjvxjy69ddme463uc749ys7h8nkawllvsfpzylk3q2dhggdvxyzgl799340e9ze8vleatxs5ext5zule58kfjs7qyn8gqajllw2 with this function

https://github.com/grunch/p2plnbot/blob/main/ln/pay_request.js#L9-L35

alexbosworth commented 2 years ago

yeah that seems pretty simple, but maybe the complexity is on the create invoice side

when I test probing it I get a successful result myself

some things I'd look at trying:

grunch commented 2 years ago

I am trying to add routes in probeForRoute but I get an error, I am not sure if the documentation is ok or I am missing something. Here https://github.com/alexbosworth/ln-service#probeforroute we can see the routes param should be

  [routes]: [[{
    [base_fee_mtokens]: <Base Routing Fee In Millitokens Number>
    [channel_capacity]: <Channel Capacity Tokens Number>
    [channel]: <Standard Format Channel Id String>
    [cltv_delta]: <CLTV Blocks Delta Number>
    [fee_rate]: <Fee Rate In Millitokens Per Million Number>
    public_key: <Forward Edge Public Key Hex String>
  }]]

But parsePaymentRequest get me this in routes:

[
  [
    {
      public_key: '038f8f113c580048d847d6949371726653e02b928196bad310e3eda39ff61723f6'
    },
    {
      base_fee_mtokens: '100',
      channel: '10644886x425933x61116',
      cltv_delta: 9,
      fee_rate: 1500,
      public_key: '03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af'
    }
  ],
  [
    {
      public_key: '03a6ce61fcaacd38d31d4e3ce2d506602818e3856b4b44faff1dde9642ba705976'
    },
    {
      base_fee_mtokens: '100',
      channel: '10644886x425933x61116',
      cltv_delta: 9,
      fee_rate: 1000,
      public_key: '03dc3dc5dc021b540b11fa986a293f8c2310c005112cf4170554c2fe0607db19af'
    }
  ]
]

It's not exactly the same.

This is the code:

const payRequest = async ({ request, amount }) => {
  try {
    const invoice = parsePaymentRequest({ request });
    if (!invoice) {
      return false;
    }

    // If the invoice is expired we return is_expired = true
    if (invoice.is_expired) {
      return invoice;
    }

    // We need to set a max fee amount
    const maxFee = amount * parseFloat(process.env.MAX_ROUTING_FEE);
    const maxFeeMsats = Math.round(maxFee * 1000).toString();
    const mtokens = (amount * 1000).toString();
    console.log(invoice.routes);
    // Probe to find a successful route
    const { route } = await probeForRoute({
      lnd,
      destination: invoice.destination,
      mtokens,
      total_mtokens: mtokens,
      cltv_delta: invoice.cltv_delta,
      payment: invoice.payment,
      routes: invoice.routes,
    });

    const payment = await payViaRoutes({
      lnd,
      id: invoice.id,
      routes: [route],
    });

    return payment;
  } catch (error) {
    logger.error(error);
    return false;
  }
};

This is the error I am getting

[2022-06-09T14:34:00.081-03:00] error: Cannot read properties of undefined (reading 'hops') TypeError: Cannot read properties of undefined (reading 'hops')
    at /home/grunch/dev/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:108:51
    at Array.findIndex (<anonymous>)
    at validate (/home/grunch/dev/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:108:25)
    at runTask (/home/grunch/dev/p2plnbot/node_modules/async/auto.js:295:13)
    at /home/grunch/dev/p2plnbot/node_modules/async/auto.js:233:31
    at processQueue (/home/grunch/dev/p2plnbot/node_modules/async/auto.js:243:13)
    at auto (/home/grunch/dev/p2plnbot/node_modules/async/auto.js:230:5)
    at /home/grunch/dev/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:93:12
    at new Promise (<anonymous>)
    at module.exports (/home/grunch/dev/p2plnbot/node_modules/lightning/lnd_methods/offchain/pay_via_routes.js:92:10)
grunch commented 2 years ago

yeah that seems pretty simple, but maybe the complexity is on the create invoice side

when I test probing it I get a successful result myself

some things I'd look at trying:

* remove max fee

* reset mission control before paying

Is it possible to reset mission control with this library?

alexbosworth commented 2 years ago

you can reset using deleteForwardingReputations

alexbosworth commented 2 years ago

thanks for the code example I will take a look

alexbosworth commented 2 years ago

Ah I see the problem is you are giving an undefined route to payViaRoutes, so in probeForRoute it is not finding a route and returning {}

Potentially validation could be improved there to give a better error, but you should check if route was existing in the result before paying

I checked paying a request when there are two route hints using this method, it seems to work as normal so I couldn't reproduce that part

alexbosworth commented 2 years ago

Something also to try is to bump the cltv delta

So instead of invoice.cltv_delta put invoice.cltv_delta + 3

grunch commented 2 years ago

This is the last version so far, it works paying to nodes but it fails with mobile wallets, I rolled back to the version with payViaPaymentRequest while I do more research on why is not working.

const payRequest = async ({ request, amount }) => {
  try {
    const invoice = parsePaymentRequest({ request });
    if (!invoice) return false;

    // If the invoice is expired we return is_expired = true
    if (invoice.is_expired) return invoice;

    // Delete all routing reputations to clear pathfinding memory
    await deleteForwardingReputations({ lnd });

    // We need to set a max fee amount
    const maxFee = amount * parseFloat(process.env.MAX_ROUTING_FEE);
    const maxFeeMsats = Math.round(maxFee * 1000).toString();
    const mtokens = (amount * 1000).toString();
    const params = {
      lnd,
      destination: invoice.destination,
      mtokens,
      cltv_delta: invoice.cltv_delta + 3,
      max_fee_mtokens: maxFeeMsats,
      total_mtokens: mtokens,
      payment: invoice.payment,
    };

    if (invoice.routes) params.routes = invoice.routes;

    // Probe to find a successful route
    const { route } = await probeForRoute(params);

    if (!route) return false;

    const payment = await payViaRoutes({
      lnd,
      id: invoice.id,
      routes: [route],
    });

    return payment;
  } catch (error) {
    logger.error(error);
    return false;
  }
};
alexbosworth commented 2 years ago

You aren't giving the route hints from the invoice to the probeForRoute so that's why it wouldn't work for mobile

grunch commented 2 years ago

I am doing it here if (invoice.routes) params.routes = invoice.routes;

alexbosworth commented 2 years ago

Yeah I missed that, ok another thing to try is add the features to the params

grunch commented 2 years ago

I added features and it works paying to nodes with a private channel and hints on the invoice in polar, but I test it on production and it doesn't work, that's weird, the only difference I see is polar is using 0.14.3 and I am using lnd 0.14.2-beta in production

alexbosworth commented 2 years ago

You could try subscribeToProbeForRoute and look at what events are generated? I am guessing it is not working to find a route is that right?

grunch commented 2 years ago

I did this to test making payments with lightning library, it works fine for me locally with polar, but in production with my node probePay() doesn't work, I've been trying with current muun invoices, with a single hint, but it doesn't work, probeForRoute doesn't create the route, I get this error 400,ExpectedArrayOfRoutesToAttemptPayingOver

https://github.com/grunch/lnpayments

alexbosworth commented 2 years ago

You mean probeForRoute doesn't find a route?

here is an example of probing for a route: https://github.com/alexbosworth/balanceofsatoshis/blob/master/network/execute_probe.js#L118

pseudozach commented 1 year ago

I'm having the same issue with using payviapaymentrequest. This fails while directly calling lncli payinvoice succeeds.

Is there any easy way to find out what the difference in default parameters are? I think this library should also have the same defaults as the expectation is to use the underlying node commands.

alexbosworth commented 1 year ago

what is the issue? can you make a new issue with your specific case?