LND/connext: uncooperative `closechannel`

[kilrau]

A concern while working on the closechannel functionality is that we rely on peers to tell us what their lnd pub keys are and we don't persist them anywhere. That means if they aren't online or not cooperative, we won't know their lnd pub key to close the channel we have with that particular xud peer. There's also the concern that peers may give us someone else's pub key, and we unintentionally close a channel with someone else. So we may describe the current version of closechannel as "naive" since we are relying on our xud peer to be online and truthfully tell us their lnd's pubkey.

This issue is about implementing a hardened version, persisting the lnd pubkeys and channel ids (lndpubkey+channelid <-> xud node key relation) in the db. In a second step we'd need to adjust closechannel to lookup channel data from the db and don't request any info from the peer. If the counterparties lnd is offline, it should throw the lnd's standard warning of a force close and let the user confirm that she's ok with forceclosing the channel.

The same should go for connext, but we might need to open a follow-up issue to make behavior align here, to wait for the switch to vector to be through.

[rsercano]

I took a closer look at it, first of all keeping channels in the db requires a lot of effort to make it sync since opening and closing can happen outside of xud.

Apart from that, I guess with below sentence:

In a second step we'd need to adjust closechannel to lookup channel data from the db and don't request any info from the peer

You mean to take only channel id parameter for closechannel command? If so, why don't we implement a new grpc call listchannels which would print, channel id, remote pub key and alias and send request directly to lnd so that we get what's recent always?

In other case we would still need such a call to show channel ids to user so he/she can know what to close.

Therefore what I suggest for this issue:

• Implement listchannels and print channel_id, remote_pub_key, alias at least to output.
• Change closechannel command to take channel_id instead of pub_key and call closechannel from LND after finding necessary information from the list of channels by id.
• Make these both changes as much as generic so that we can use them for connext soon too.

So that:

• With this way peer doesn't have to be "cooperative or online" because we'll get channel ids from lnd directly and will show alias/pubkey to user so he/she will know which channel he/she is closing. Only concern can be about alias I'm not sure if we can reach it if the peer is offline @sangaman
• I don't think somebody would unintentionally close another channel with this way, because we'll print what he/she has in terms of channels.
• We can still check peer's offline status and throw force close error.

If that's not the intention, and I misunderstood, please let me know.

[erkarl]

@rsercano thanks for doing the research on this one.

One thing to consider is that remote peer's lnd pubkey could also change over time when running xud without docker.

As for closing the channels I think the only thing we need is the channel point (funding transaction id + output index). Could we persist those + link it to xud public key?

We can still check peer's offline status and throw force close error.

We could attempt a cooperative close. If peer offline -> attempt force as fallback when --force was provided?

[rsercano]

@rsercano thanks for doing the research on this one.

One thing to consider is that remote peer's lnd pubkey could also change over time when running xud without docker.

As for closing the channels I think the only thing we need is the channel point (funding transaction id + output index). Could we persist those + link it to xud public key?

We can still check peer's offline status and throw force close error.

We could attempt a cooperative close. If peer offline -> attempt force as fallback when --force was provided?

So what the exact closechannel command would be like for a peer? If we persist funding tx id + output index we would require one of these information to find from db during a close command, if so how will user know this information within xud?

And for the case we persisted it in the db, if somebody close the channel from outside of xud, in this case we still have it in the db, so what should we do in this case? Check channels on a regular basis? This can come a point that hard to track.

[sangaman]

I think perhaps a more fundamental issue we should address first is verifying the peer's lnd identies up front, which should be possible (and not too challening) using the SignMessage and VerifyMessage lnd commands. Without this, peers could still lie about their identity to us, and information that the peer has given us and which we store in the database could still be wrong. Verifying their identity eliminates this question.

The other concern is when and how often we update our db with channels. Do we periodically want to check our list of channels and compare them against peers? Maybe just on each xud startup? We'd probably also want to update every time we open a new channel through xud.

We could attempt a cooperative close. If peer offline -> attempt force as fallback when --force was provided?

This makes sense, if we can't cooperatively close (on the lnd level) then we force close (again on the lnd level) but either way we don't rely on the xud peer to be online to get this information.

So what the exact closechannel command would be like for a peer? If we persist funding tx id + output index we would require one of these information to find from db during a close command, if so how will user know this information within xud?

I think the closechannel command would stay the same, where we just specify an xud peer, the difference being we'd use the channel information from our local database instead of from the xud peer.

[kilrau]

I think perhaps a more fundamental issue we should address first is verifying the peer's lnd identies up front, which should be possible (and not too challening) using the SignMessage and VerifyMessage lnd commands. Without this, peers could still lie about their identity to us, and information that the peer has given us and which we store in the database could still be wrong. Verifying their identity eliminates this question.

:+1:

The other concern is when and how often we update our db with channels. Do we periodically want to check our list of channels and compare them against peers? Maybe just on each xud startup? We'd probably also want to update every time we open a new channel through xud.

On xud start is fine I think.

We could attempt a cooperative close. If peer offline -> attempt force as fallback when --force was provided?

This makes sense, if we can't cooperatively close (on the lnd level) then we force close (again on the lnd level) but either way we don't rely on the xud peer to be online to get this information.

Sounds about right!

I think the closechannel command would stay the same, where we just specify an xud peer, the difference being we'd use the channel information from our local database instead of from the xud peer.

Yep, that's what I had in mind too.

Thanks for the input @sangaman and it's valuable to have figured out how to implement this, but since this is not too trivial to implement without an immediate use case or situation where we ran into this, I'd like to lower prio on this one and will take some time to point you at higher prio issues today. That alright? @rsercano

[rsercano]

Yes of course Kilian @kilrau I completely agree with you