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

Creating invoice with big amount leads to invoice without an amount #163

Open
adambor opened this issue Aug 12, 2022 · 8 comments
Open

Creating invoice with big amount leads to invoice without an amount #163

adambor opened this issue Aug 12, 2022 · 8 comments

Comments

@adambor
Copy link

adambor commented Aug 12, 2022

When I create a hodl invoice by:

    const secret = crypto.randomBytes(32);
    console.log("Generated secret preimage: "+secret.toString("hex"));
    const invoiceId = crypto.createHash('sha256').update(secret).digest('hex');

    const invoice = await lncli.createHodlInvoice({
        lnd,
        tokens: "104500000000000000000000000000000000000000000000000000000000000000000000000000000000000",
        id: invoiceId
    });

    console.log(invoice);

Following response is returned (payment request without an amount, but tokens returned are 1.045e+86):

{
  chain_address: undefined,
  created_at: '2022-08-12T13:56:24.000Z',
  description: undefined,
  id: 'f222dbd0c96195e2b6513a6cccd7a835b1d828e1d72300e73fe14cda5b8edc21',
  mtokens: '104500000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
  request: 'lnbc1p30vhqgpp57g3dh5xfvx279dj38fkve4agxkcas28p6u3speelu9xd5kuwmsssdqqcqzpgxqyz5vqsp5f6rxyrgzp6w6rcpq3dh59vk368cnjvnyugn4y3m6hm2ymzq267xs9qyyssq3h3d9f8wxhk5kf0mwds3t5udjqzywvq5sw2hqjq4ztmgfvlsh0kpfa504
esknpmdt9rfn7kkhhvkthvwzgczumqqh2qehn5yaft0gqqqf3478x',
  secret: undefined,
  tokens: 1.045e+86
}

If I use createInvoice the response then is (at least the tokens returned are 0):

{
  chain_address: undefined,
  created_at: '2022-08-12T13:59:43.000Z',
  description: undefined,
  id: '336400e8539dacf95677d8aabfba1d8751478dbaef61ea465f702a2a28f4168c',
  mtokens: '0',
  payment: 'c5e5f07d3bb4f5517f25da66502f74eb97ba2db9545f959c3b33dc6e3c8ca39c',
  request: 'lnbc1p30vhx0pp5xdjqp6znnkk0j4nhmz4tlwsasag50rd6aas753jlwq4z5285z6xqdqqcqzpgxqr23ssp5chjlqlfmkn64zle9mfn9qtm5awtm5tde230et8pmx0wxu0yv5wwq9qyyssqntchagjhewgk7ef4z4ncuk26v273kyaflvsz2n086rgcursnxc4juxwfz9
9q5hv4q45mk2yxjkqqgzk3tnjx72slkvjzplhksaqdvlsqa6e034',
  secret: '73fdcef61a5b3a1086567c9f622ae35f1f25840483b4ab076c25c893b70772fa',
  tokens: 0
}

However this might lead to some bugs in implementation (like it happened for me) expecting that when the invoice is paid I should increment the user's balance by 1.045e+86, even though the user might've just paid 1 sat.

I would expect the lib to throw an error in such a case, and not an invoice without an amount.

@adambor adambor changed the title Creating invoice with big amount leads to invoice with 0 amount Creating invoice with big amount leads to invoice without an amount Aug 12, 2022
@alexbosworth
Copy link
Owner

invoices without amounts are used for overpayment purposes, that's part of the LN protocol

if you want to see how much was paid, please use getInvoice or subscribeToInvoice

technically speaking in LN protocol any invoice can be overpaid

@adambor
Copy link
Author

adambor commented Aug 12, 2022

Yep, I get that, and that's how I fixed the issue. I just think that when you specify a non-zero amount for an invoice, that function call should either return the invoice of that non-zero amount or throw an error, not return an invoice without any amount... In this case it can result in underpayment, not overpayment.

@alexbosworth
Copy link
Owner

is the problem that you used an incorrect number for the amount? it should be a JS number not a string

@adambor
Copy link
Author

adambor commented Aug 12, 2022

Yep, using string instead of the number, so that looks like it is a source of the issue. Very much unpredictable with strings, for smaller amounts it works, for larger it overflows and tries to create negative value invoice, and for super large ones it creates an invoice without an amount. Maybe there should be a type check? I am just trying to make sure no one else falls for this, because this resulted in some amount of BTC getting stolen from me.

@alexbosworth
Copy link
Owner

Yeah I think it could type check but this is a general issue with JS in general that large numbers are undefined in their behavior and types aren't built in to the language

There is also typescript support that you could look at, I don't use that myself but potentially it can help with type enforcement

@adambor
Copy link
Author

adambor commented Aug 12, 2022

Yep, those pesky JS numbers, that's also why I am not using them at all and rather resort to js-big-decimal library and that's also a reason I am passing that number as string instead of number. Will be looking to move the whole codebase to typescript soon. But I think that simple type check might save someone some money :)

@alexbosworth
Copy link
Owner

I think another approach is to specify the amount as mtokens, this takes a string and that's how you can specify large numbers

@alexbosworth
Copy link
Owner

I added validation of tokens to be numeric here alexbosworth/lightning@7bd1794

But the real way to stop problems is to totally lock down user input and that has to live at the application level

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants