feat(lightning): add CLN NUT-15 MPP partial payment support#1018
feat(lightning): add CLN NUT-15 MPP partial payment support#1018christoph865 wants to merge 3 commits into
Conversation
|
If the aim of the PR is to add support for multinut payments to the wallet, then why does the diff show changes to clnrest.py? |
|
The intention of the PR is to add multinut payment support for wallet flows on CLN-backed mints. Although the feature is wallet-facing, the actual outgoing Lightning payment is executed by the configured backend. For CLN-backed mints, that backend is |
|
Hi @christoph865 |
|
Hi @edgarmuyomba, thanks for pointing this out. After re-checking What this PR mainly adds on top is:
|
|
yea, I think you should narrow the PR down to test-only changes. |
|
Done. Narrowed to test-only changes. The diff now adds |
| @pytest.mark.asyncio | ||
| async def test_mpp_disabled_returns_error(wallet: CLNRestWallet): | ||
| wallet.supports_mpp = False | ||
|
|
||
| with patch("cashu.lightning.clnrest.decode") as mock_decode: | ||
| mock_invoice = Mock(spec=Bolt11) | ||
| mock_invoice.payment_hash = "hash123" | ||
| mock_invoice.amount_msat = 1000000 | ||
| mock_decode.return_value = mock_invoice | ||
|
|
||
| quote = create_melt_quote(amount=600000, unit="msat") | ||
|
|
||
| result = await wallet.pay_invoice(quote, 1000) | ||
|
|
||
| assert result.result == PaymentResult.FAILED | ||
| assert result.error_message is not None | ||
| assert "does not support MPP" in result.error_message | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_full_payment_no_mpp(wallet: CLNRestWallet): | ||
| with patch("cashu.lightning.clnrest.decode") as mock_decode: | ||
| mock_invoice = Mock(spec=Bolt11) | ||
| mock_invoice.payment_hash = "full_payment_hash" | ||
| mock_invoice.amount_msat = 1000000 | ||
| mock_decode.return_value = mock_invoice | ||
|
|
||
| quote = create_melt_quote(amount=1000000, unit="msat") | ||
|
|
||
| wallet.client.post = AsyncMock( | ||
| return_value=Mock( | ||
| is_error=False, | ||
| json=lambda: { | ||
| "payment_hash": "full_payment_hash", | ||
| "payment_preimage": "preimage123", | ||
| "amount_sent_msat": 1000100, | ||
| "amount_msat": 1000000, | ||
| "status": "complete", | ||
| }, | ||
| ) | ||
| ) | ||
|
|
||
| result = await wallet.pay_invoice(quote, 1000) | ||
|
|
||
| assert result.result == PaymentResult.SETTLED | ||
| assert result.preimage == "preimage123" | ||
| call_data = wallet.client.post.call_args.kwargs["data"] | ||
| assert "partial_msat" not in call_data, "partial_msat must not be sent for full payments" |
There was a problem hiding this comment.
Nicely done on trimming the PR. Looking at the existing tests though, there's already an established pattern for testing Lightning backends in tests/lightning/test_lightning_backends_mocked.py, including a test for CLNRest without MPP support.
These new files duplicate some of the tests there with a different approach as well.
Could you have a look at the existing file and follow the pattern there to avoid fragmenting the testing convention?
|
Done. Moved the MPP tests into |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1018 +/- ##
==========================================
+ Coverage 75.02% 75.11% +0.09%
==========================================
Files 111 110 -1
Lines 12239 12093 -146
==========================================
- Hits 9182 9084 -98
+ Misses 3057 3009 -48 ☔ View full report in Codecov by Harness. |
3 participants
Summary:
Add CLN NUT-15 MPP support for partial melts.
Route amount mismatches to partial payments when MPP is enabled.
Return a clear error when MPP is required but disabled.
Add focused CLN MPP tests in tests/lightning/test_clnrest_mpp.py, tests/lightning/test_lightning_backends_mocked.py, and tests/lightning/conftest.py.
Validation:
poetry run pytest -q tests/lightning
poetry run pytest -q test_clnrest_mpp.py test_lightning_backends_mocked.py -k "clnrest or mpp"
Closes #536