loopd: don't fail external loop in on fee estimation failure #86
Conversation
cmd/loop/loopin.go
Outdated
| @@ -67,6 +67,21 @@ func loopIn(ctx *cli.Context) error { | |||
| } | |||
|
|
|||
| limits := getInLimits(amt, quote) | |||
| external := ctx.Bool("external") | |||
|
|
|||
| // There could have been an error in the on-chain fee estimation if | |||
There was a problem hiding this comment.
Following from my last comment, we can use Is/As here instead and gate the warning or exit based on that.
looprpc/client.pb.go
Outdated
| XXX_NoUnkeyedLiteral struct{} `json:"-"` | ||
| XXX_unrecognized []byte `json:"-"` | ||
| XXX_sizecache int32 `json:"-"` | ||
| MinerFee int64 `protobuf:"varint,3,opt,name=miner_fee,json=minerFee,proto3" json:"miner_fee,omitempty"` |
There was a problem hiding this comment.
This regeneration with the correct version of the proto tools, can it be a separate commit?
looprpc/client.proto
Outdated
| /** | ||
| If an error happens during on-chain fee estimation in lnd, this contains | ||
| the error. The client should decide if it's ok to continue (i.e. external | ||
| loop in) or not. |
There was a problem hiding this comment.
I am wondering what the user expects in a quote response when the intention is to use an external address. A zero miner fee? Or a happy case fee (1 inputs, 1 htlc out, 1 change out)?
How about passing in an external bool to the quote request to skip the lnd wallet fee estimation? It makes it all a bit more transparent. Returning an error sometimes for an external funded swap quote may be confusing.
There was a problem hiding this comment.
Good idea! I like that much more than passing an error back.
I wasn't sure if it was OK to just skip the fee estimation completely on an external swap in.
guggero
force-pushed
the
external-loop-in-fee-est
branch
from
b0faa97
to
c00f831
Compare
|
I did look into returning an error in the response message directly. I haven't found anything useful, it looks like there is no error primitive in protobuf corresponding to a golang error. We could have used numeric error codes. Or send some additional data in the GRPC error body and then parse that in the client (to still get the swap fee that was calculated). But I like Joost's suggestion much more to just skip on-chain fee estimation completely for an external loop in. |
In the
LoopInQuotefunction ofclient.goan on-chain fee estimation for paying the HTLC is done by callingEstimateFeeToP2WSHon lnd.This fails in case the lnd wallet has no UTXOs left to construct an estimation transaction with.
For an internal loop in, this can be seen as a hard failure and the swap can't continue.
For an external loop however, it doesn't matter that the lnd wallet has no funds. So we just warn about not being able to estimate the on-chain fee.
Fixes #84
I'm not too happy about sending back an error as a string. The two operations (getting a swap fee quote from the swap server and doing the on-chain fee estimation in lnd) would ideally be separate calls so the client could handle the distinct errors. But this would be a bigger refactor. Open to suggestions here.
Note: The large diff in the
looprpcpackage is mainly due to #74 which was probably generated with a wrong version ofgithub.com/grpc-ecosystem/grpc-gateway. It also included some manual changes in theclient.swagger.jsonthat are overwritten when generating again. This can also go into a separate PR if it's too big for this bugfix.