-
Notifications
You must be signed in to change notification settings - Fork 43
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
LTC/BTC swap failure - Taker's xud crashed #984
Comments
lndltc log shows this:
Links
xud-log:
|
Interesting finding:
looks like that if the first leg is LTC, it works well. |
I'm stumped as to why the taker's xud crashed. Is this reproducible for you? It would also be very helpful to have the panic stack trace when it crashed if you can access it, that won't be in the actual log files though. |
Managed to reproduce the issue. Just like Offer said - direct channels, sanity checks pass, real swap fails. What is interesting is that I used the same xud nodes for non-direct channel swap and it succeeded without problems. maker-command
maker-xud.log
taker-command
taker-xud.log
|
@kilrau Adding P1 back again. I see this from time to time. Not sure you want to see it when you demo in Munich |
Certainly not. @sangaman |
This issue can be divided into 2 parts:
|
Since 1. is fixed in #997, this issue is about fixing 2. |
I have the same problem with sell orders of WETH/BTC. The second leg is BTC and it fails with This can be LND issue so we should try to reproduce with LND's command line (using send to route) |
I have noticed some related issues on LND. |
We discussed on the call to just use |
Let's discuss it before implementation.
…On Tue, 4 Jun 2019 at 17:15 Kilian Rausch ⚡️ ***@***.***> wrote:
We discussed on the call to just use sendpayment and get rid of
sendtoroute. It allows to set cltv delta now. @erkarl
<https://github.com/erkarl>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#984?email_source=notifications&email_token=ACKDI4QPC6RNA2AT5GWCHXLPY2BK7A5CNFSM4HPVLJKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW4434Q#issuecomment-498716146>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKDI4U2VSZDZTP6PPUEY7LPY2BK7ANCNFSM4HPVLJKA>
.
|
Sure, please jump in with implementation details to review @erkarl |
First, we have a problem that we don’t understand and not sure how to reproduce. Changing the payment api that we use due to this problem is just a guess. Probably not a good one. I would not change this before we are live and before we have a good reason for it. And, we should understand how we use cltv without the route. Have a feeling that the reason for the problem has to do with the open htlcs that we have when we used sanity swaps. Maybe the number of open Htlc got to high? |
@offerm I'm able to reproduce the problem on my machines 100% of the time. I did a quick proof of concept with |
Upon further inspection I discovered 2 problems.
The second problem I noticed on xud level was that we're not waiting for the hold invoice state to become I suggest we move away from |
SGTM, please go ahead. Not sure if |
Nice finding with the UnknownPaymentHash !!!
About SendToRouteSync replcing SendPayment:
- Can we find first what is the reason for the problem and why we don't see
it [any more] on simnet?
- Also before we switch - how are we going to decide which value to provide
as final_cltv_delta?
…On Fri, Jun 7, 2019 at 5:38 PM Karl Ranna ***@***.***> wrote:
@offerm <https://github.com/offerm>
Upon further inspection I discovered 2 problems.
lndrpc.SendToRouteSync is broken when we pass in a route that has only 1
hop. It will error with UnknownNextPeer message. I also tried on current
lnd master v0.6.1-beta-228-gaa5156a1a985ef3e0b1e6ad48a081125a465870d
where SendToRouteRequest.routes is deprecated in favor of
SendToRouteRequest.route - same result. What is interesting is that I can
only replicate the problem on testnet, but not on my local simnet.
The second problem I noticed on xud level was that we're not waiting for
the hold invoice state to become OPEN and in case of low latency the
taker can attempt to pay us "too fast" when accepting the deal, causing the
payment to occasionally error with UnknownPaymentHash. This is the reason
I mentioned yesterday that switching to lndrpc.SendPayment didn't work.
I suggest we move away from lndrpc.SendToRouteSync to using
lndrpc.SendPayment and create an issue to lnd. We're already using it for
sanity swaps. To address Offer's concern above - it also allows to specify
the final_cltv_delta.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#984?email_source=notifications&email_token=ACKDI4XMSG5HNOW5NS7O463PZJXE5A5CNFSM4HPVLJKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXGAOBI#issuecomment-499910405>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKDI4VYRQCMA2YWZYG7UF3PZJXE5ANCNFSM4HPVLJKA>
.
|
please attach the LND logs for the taker and maker when getting the
`UnknownNextPeer`
error.
…On Fri, Jun 7, 2019 at 6:10 PM Offer Markovich ***@***.***> wrote:
Nice finding with the UnknownPaymentHash !!!
About SendToRouteSync replcing SendPayment:
- Can we find first what is the reason for the problem and why we don't
see it [any more] on simnet?
- Also before we switch - how are we going to decide which value to
provide as final_cltv_delta?
On Fri, Jun 7, 2019 at 5:38 PM Karl Ranna ***@***.***>
wrote:
> @offerm <https://github.com/offerm>
>
> Upon further inspection I discovered 2 problems.
>
> lndrpc.SendToRouteSync is broken when we pass in a route that has only 1
> hop. It will error with UnknownNextPeer message. I also tried on current
> lnd master v0.6.1-beta-228-gaa5156a1a985ef3e0b1e6ad48a081125a465870d
> where SendToRouteRequest.routes is deprecated in favor of
> SendToRouteRequest.route - same result. What is interesting is that I
> can only replicate the problem on testnet, but not on my local simnet.
>
> The second problem I noticed on xud level was that we're not waiting for
> the hold invoice state to become OPEN and in case of low latency the
> taker can attempt to pay us "too fast" when accepting the deal, causing the
> payment to occasionally error with UnknownPaymentHash. This is the
> reason I mentioned yesterday that switching to lndrpc.SendPayment didn't
> work.
>
> I suggest we move away from lndrpc.SendToRouteSync to using
> lndrpc.SendPayment and create an issue to lnd. We're already using it
> for sanity swaps. To address Offer's concern above - it also allows to
> specify the final_cltv_delta.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#984?email_source=notifications&email_token=ACKDI4XMSG5HNOW5NS7O463PZJXE5A5CNFSM4HPVLJKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXGAOBI#issuecomment-499910405>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACKDI4VYRQCMA2YWZYG7UF3PZJXE5ANCNFSM4HPVLJKA>
> .
>
|
That is a very good question that bothers me.
If we're the the taker we'll use lnd logs for UnknownNextPeerpayer
payee
|
Please also note that I had to use the following patch on xud level in order to produce the
Yes, includes it. |
Can you please check status of links? Make sure you include Link
1105321:1:0
…On Fri, Jun 7, 2019 at 8:29 PM Karl Ranna ***@***.***> wrote:
@offerm <https://github.com/offerm>
Please also note that I had to use the following patch in order to produce
the UnknownNextPeer error. This is due to fact that I upgraded to latest
master lnd gaa5156a1a985ef3e0b1e6ad48a081125a465870d and cannot downgrade
because of database migrations.
@kilrau <https://github.com/kilrau>
Not sure if v0.6.1-beta-228-gaa5156a1a985ef3e0b1e6ad48a081125a465870d
includes the new error output of lightningnetwork/lnd#3113
<lightningnetwork/lnd#3113>, but what exactly did
it throw?
Yes, includes it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#984?email_source=notifications&email_token=ACKDI4Q4UEYVN7A2BTGYUDTPZKLGFA5CNFSM4HPVLJKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXGPJCY#issuecomment-499971211>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACKDI4RPBYE2ULY4VRR7J4TPZKLGFANCNFSM4HPVLJKA>
.
|
Prior to executing the payment:
Are you not able to reproduce the issue on testnet? |
Failure to take an order.
Notes:
Taker issues a buy order:
Response:
Taker log:
Maker log:
The text was updated successfully, but these errors were encountered: