ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 49 forks source link

fix(lnd): handling hold invoice check errors #1969

Closed sangaman closed 3 years ago

sangaman commented 3 years ago

This adds better error handling for when the test calls to verify lnd hold invoices are available fail due to connectivity reasons. Previously any error that occurred at this step would cause us to set lnd's status to NoHoldInvoiceSupport including connection issues. There was also a bug that caused us to try to set the status to connected even when a hold invoice status check failed.

This could result in the unusual behavior of status going to Disconnected upon a call failing due to the grpc UNAVAILABLE error status, then being set to NoHoldInvoiceSupport and then to ConnectionVerified. Now we only set NoHoldInvoiceSupport when the test calls fail for a reason other than UNAVAILABLE, and we only set the status to ConnectionVerified when the hold invoice calls succeed.

Closes #1968.

sangaman commented 3 years ago

I believe we can merge this even though it doesn't solve the mysterious related docker issue whereby the hold invoice check legitimately gets a grpc UNAVAILABLE error from lnd, right?

kilrau commented 3 years ago

Well it should be tested to ensure it doesn't break anything:

kilrau commented 3 years ago
* works, but im not sure why most of fields are empty. We can get all required data from lnd, so we should fill them. (or remove `Active 0` too).

That's because the Active: X is hardcoded in the response and just takes the result of lnd's listchannels count. Empty fields are fine because it attracts attention and we don't need to spend time optimizing for this edge case. IMO it's fine as is and can be merged.