lnwallet/chancloser: remove the commit fee clamp, introduce max fee #6770
Conversation
Roasbeef
changed the title
|
cc @sputn1ck |
Roasbeef
added
channel closing
In this commit, we stop clamping the ideal fee rate to the commitment fee of the channel. This catches us up to this PR of the spec: lightning/bolts#847. We also do away with the old 3x ideal fee "max fee", and replace that with an explicit max fee. This new max fee will either be the default multiplier of the ideal fee, or a new user specified max fee value.
This lets callers get the thaw height without needing to first obtain a snapshot of the channel state.
…terface In this commit, we remove the raw channel state machine pointer from the chan closer and instead replace that with an interface that captures *just* the methods we need in order to do the co-op close dance. This is a preparatory refactoring for some upcoming unit tests.
In this commit, we add a new max_fee field that we'll use to decide when to bail out of a co-op close dance as the initiator.
In this commit, we parse the new max fee field, and pass it through the switch, all the way to the peer where it's ultimately passed into the chan closer state machine.
Roasbeef
force-pushed
the
co-op-close-with-more-spec-innit
branch
from
62e9c6c to
046aa7f
Compare
| // specified explicit max fee. | ||
| maxFee := idealFeeSat * defaultMaxFeeMultiplier | ||
| if cfg.MaxFee > 0 { | ||
| maxFee = cfg.Channel.CalcFee(cfg.MaxFee) |
There was a problem hiding this comment.
Similar to the ideal fee, the max fee isn't respected on restart
There was a problem hiding this comment.
Hmm, I think one way to resolve this would be to commit this information to disk when we call MarkCoopBroadcasted.
There was a problem hiding this comment.
So we'd store persistently the preferences of the user at that point, and for the case where we're the responder (but still the initiator), we'd do something like base it off some (larger?) multiplier of the max commitment fee.
There was a problem hiding this comment.
I don't think it should be stored in MarkCoopBroadcasted since #6760 will end up changing that function's usage. This is because the link may still need to be loaded, but we still want the user's preferences. I made a function called PersistDeliveryScript which could be extended to persist other information before calling MarkCoopBroadcasted
I also think we could check here that the ideal fee isn't greater than the max fee if one exists
There was a problem hiding this comment.
where we're the responder (but still the initiator), we'd do something like base it off some (larger?) multiplier of the max commitment fee.
and/or a config option?
There was a problem hiding this comment.
So from my reading, we'll retain the "ideal" fee on restart, since we'll compute the "ideal" fee based on CoopCloseTargetConfs (defaults to 6 blocks). At this point, the req will be nil, so we'll then based the max fee off 3x this ideal fee.
So I think we can either leave it as is, persist it in #6760, or add a config option (yet another one, that ppl prob won't set tbh) to let ppl express what this max multiplier should be.
There was a problem hiding this comment.
There are two ways the ideal fee is set:
- through rpc if initiating the close
- via EstimateFeePerKW on CoopCloseTargetConfs
The first case probably won't have the same fee on restart. The second may if the fee estimation doesn't change. I don't think this should be persisted in this PR. It could be done in #6760. But I think any persistence changes should be done in the same release so we don't have to check whether something is stored or not
There was a problem hiding this comment.
SGTM. I'd say a user's expectation would be for things to resume with their desired fee, so we should strive for that. Agree we can do it in another PR, though rn #6760 is marked for 0.16 (and in a draft state).
Agreed, though I think it's outside the scope of this PR. Ideally we go ahead with the proposed overhaul of the co-op close in the protocol generally, which should really simplify all the code related to this. |
In this commit, we stop clamping the ideal fee rate to the commitment fee of the channel. This catches us up to this PR of the spec: lightning/bolts#847.
We also do away with the old 3x ideal fee "max fee", and replace that with an explicit max fee. This new max fee will either be the default multiplier of the ideal fee, or a new user specified max fee value.
We also add a new
max_feefield to the co-op close RPC which allow the caller to specify when we should bail out due to fees being too high. This field is only observed if it's set by the initiator however. Once we finish #5644, both sides will be able to properly set this field.Fixes #6781