-
Notifications
You must be signed in to change notification settings - Fork 901
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
invoice: allow creation of giant invoices. #4606
invoice: allow creation of giant invoices. #4606
Conversation
l2.rpc.invoice(4294967295 + 1, 'inv3', '?') | ||
# Make sure wumbo invoices warn about mpp being needed. | ||
inv = l2.rpc.invoice(4294967295 + 1, 'inv3', '?') | ||
assert 'warning_mpp' in inv | ||
l2.rpc.invoice(4294967295, 'inv3', '?') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Github actions
/home/runner/work/lightning/lightning/contrib/pyln-client/pyln/client/lightning.py:388>]
test_invoice failed; it passed 0 out of the required 1 times.
<class 'pyln.client.lightning.RpcError'>
RPC call failed: method: invoice, payload: {'msatoshi': 4294967295, 'label': 'inv3', 'description': '?'}, error: {'code': 900, 'message': "Duplicate label 'inv3'"}
[<TracebackEntry /home/runner/work/lightning/lightning/tests/test_invoices.py:60>, <TracebackEntry /home/runner/work/lightning/lightning/contrib/pyln-client/pyln/client/lightning.py:845>, <TracebackEntry /home/runner/work/lightning/lightning/contrib/pyln-testing/pyln/testing/utils.py:623>, <TracebackEntry /home/runner/work/lightning/lightning/contrib/pyln-client/pyln/client/lightning.py:388>]
This is a bit strange, why we don't see it in previous tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we originally had it failing the first inv3 invoice creation, now it succeeds, so the next one fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTM.
ack 3c68a24 with a small comment
lightning/bolts#877 talks about removing this restriction (only Electrum actually enforced it on receive), so start by allowing creation of giant invoices, though we mark them as requiring mpp. Changelog-Changed: JSON-RPC: `invoice` now allows creation of giant invoices (>= 2^32 msat) Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
3c68a24
to
e431670
Compare
Yep, fixed. I obv failed to run local tests :( |
Restarted CI since the failure seems to be a flaky reorg test ACK e431670 |
lightning/bolts#877 talks about
removing this restriction (only Electrum actually enforced it on
receive), so start by allowing creation of giant invoices, though
we mark them as requiring mpp.