feat: allow collab and force close DLC channel #1806
Conversation
| tracing::error!( state = %channel.state, "Can't collaboratively close a channel with an open position."); | ||
| bail!("Can't collaboratively close a channel with an open position"); |
There was a problem hiding this comment.
🔧 Why not?
Also, it's weird to log and return an error for the same error.
There was a problem hiding this comment.
Also, it's weird to log and return an error for the same error.
I started doing this because I stopped trusting where errors are being logged (or even ignored) and with this it's easier to find them.
🔧 Why not?
Mhm, good question. We had this in the past, so I kept it. Happy to change it.
There was a problem hiding this comment.
Thinking about it more. It's easier to tell the user to close the position first because otherwise we will need to calculate the PnL here and think about how to handle the otherside of the trade.
There was a problem hiding this comment.
I started doing this because I stopped trusting where errors are being logged (or even ignored) and with this it's easier to find them.
@bonomat: Hmmm, I'm not happy with this because we will end up spamming the logs with redundant messages, which makes debugging harder. We just need to fix the spots where errors are being swallowed. I don't think there can be many of those.
What do you think, @holzeis?
There was a problem hiding this comment.
Thinking about it more. It's easier to tell the user to close the position first because otherwise we will need to calculate the PnL here and think about how to handle the otherside of the trade.
Fair. It's not strictly necessary and it's nice to keep things simple.
| .. | ||
| }) | ||
| | Some(SignedChannel { | ||
| state: SignedChannelState::RenewFinalized { .. }, |
There was a problem hiding this comment.
🤔 Is this really how it works? Doesn't the channel go back to Established after this one? I'd consider this state part of Pending.
There was a problem hiding this comment.
Afaik we end in RenewFinalized or Settled after any off-chain action and Established is only once the channel has been set up in the first place
There was a problem hiding this comment.
Hmm, we should verify this.
| channel_id = %channel_id_hex, | ||
| "Closing DLC channel collaboratively" | ||
| ); | ||
| self.is_safe_to_close_dlc_channel_collaboratively(&channel)?; |
There was a problem hiding this comment.
🔧 It looks like this function does not add any additional value as it is already covered by the match below.
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
bonomat
force-pushed
the
feat/collab-close-channel
branch
from
59a4b7c to
62660ee
Compare
resolves #1785
resolves #1786