-
Notifications
You must be signed in to change notification settings - Fork 912
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
Spurious WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS #3367
Comments
mmh it happened to one of my tests, and it seems it does not happen again. Is it some kind of wizard secret privacy feature? Though another test failed with |
There seems to have something strange on the payments. This randomly fails either with Can it be something that happen if you create two different invoice for the same amount in short period of time? |
I will stop working on it, we are stuck on
Let me know if you want me to test something. |
The Regarding the overpayment of invoices, that's on purpose to hide the actual However the fuzzed amounts do respect the 0.5% fee allowance when computing Can you provide instructions on how to reproduce the |
I can probably send you the RPC logs between the two nodes one moment. |
@cdecker so here are the RPC logs. Scenario is: B create two invoices. (Source code is here) I am copying here different logs:
Logs of my test:
A RPC logs:
Lightning logs for A
B RPC logs
Lightnind logs for B
|
Not paying the exact amount is actually part of the shadow routes feature, which is necessary for privacy, thus not a bug. Suggest changing this issue to be the spurious failure of paying a second invoice. The lightning logs seem incomplete -- the logs for B do not include the event where it received the Can you try inserting |
@ZmnSCPxj sqlite, will bring better log. |
Okay, so not likely to be a strange interaction with postgresql. |
@ZmnSCPxj here are 100% of the logs. I needed to execute the tests several time before reproducing it. |
Got it:
in the past, this had its own error, The most common reason for the final payee rejecting the incoming CLTV is that a block was found randomly between the
Since This seems to suggest to me that we need to spend one or two blocks in our A more sophisticated solution:
|
I am a bit surprised. I am not mining any block between the time I create the invoice and the time I pay for it. |
What should I do? Should I release 0.8 as is because this is normally a scenario happening rarely in real situation or wait a patch? |
The |
Lemme make a patch for now. How long does it take for you to verify that it does not happen again?
The issue is mining a block between the time you start paying and the time that the receiver gets an HTLC in an irrevocably committed state, which takes a few milliseconds of time and some message turnarounds. Do you not have an automatic miner model in the background of your test env? Because if not this is indeed a strange error to have (though there is still a need to fix this particular error where a block is mined between |
@ZmnSCPxj I can verify easily if I have the patch |
I don't have a miner but I noticed that when by test start I have a process to make sure there is a channel between payer and payee. This process may mine a block. If I repeat my test without this part, I don't seem to reproduce the bug. |
Okay, so it is possible that the background process is indeed possibly mining a block between when Am making a patch, hopefully it works, unfortunately it has been a while since I have seen the code and there have been many changes, not sure if the code I am hacking is still in the same shape as I saw them before. |
I have a branch here: https://github.com/ZmnSCPxj/lightning/tree/blockheight-disagreement Untested as of now, though it is compiling successfully. Here is travis for my copy: https://travis-ci.org/ZmnSCPxj/lightning/builds/628688151 Unfortunately hardware resources I need will be in use elsewhere, can you @NicolasDorier check the status in a while? |
I think I know what is happening here: you set up the network by opening Let's say you sync nodes on blockheight 100, the nodes have some funds and now The situation is as follow:
This is the case where the payment goes through. Then B polls
This explains why you are not generating blocks while sending payments but The trick here is to give all nodes sufficient time to catch up with the |
@cdecker it is possible, though I generate normally only 2 blocks and never had issue before. |
@ZmnSCPxj it seems travis not happy. |
Regardless, it seems to me a blockheight disagreement with the receiver is still possible on a real non- We use lightning/lightningd/invoice.c Line 817 in 051b5bd
The check is based on the same lightning/lightningd/peer_htlcs.c Lines 316 to 331 in 051b5bd
For reference the default lightning/lightningd/options.c Line 587 in 051b5bd
Thus it is still possible on an edge case on a mainnet network. This becomes unlikely if you have any shadow route at all (which gives additional budget to the final CLTV). But in the test case there is no possible shadow route due to it being just two nodes. Further, even in Unfortunately for my patch, it does not work since it is not possible to mix synchronous commands to asynchronous commands arising from the plugin, so I have to do a more drastic modification to get the current block height from |
@cdecker : xref: #638 (comment) In the past, this case had its own failure code, However this specific sub-case (blockheight disagreement) is in fact a non-permanent error! So we should specially identify and handle this case. Thus this is a real regression, caused by a BOLT spec change, which folded a non- |
I will then keep off the update to 0.8 until there is a patch I can test, let me know when you have something I can test. |
The original report asked about us overpaying to the receiver, but that is AFeatureNotABug. This issue now tracks another problem: spurious
incorrect_or_unknown_payment_details
failure.Issue and Steps to Reproduce
Do the below a few hundred times in an environment where miners are independent of the test suite (our test suite has block discovery tightly coupled to the test, i.e. blocks are only discovered if the test case says so):
invoice
on one node andpay
it on the other.Above text by @ZmnSCPxj
Original report by @NicolasDorier
Issue and Steps to Reproduce
msatoshi_received
is 10_004 millisatoshiThe text was updated successfully, but these errors were encountered: