Add fee and initiator to Pending Channels RPC #4111
Conversation
lnrpc/rpc.proto
Outdated
| @@ -2096,8 +2096,7 @@ message PendingChannelsResponse { | |||
| /// Channels pending opening | |||
| repeated PendingOpenChannel pending_open_channels = 2; | |||
|
|
|||
| /// Channels pending closing | |||
| repeated ClosedChannel pending_closing_channels = 3; | |||
| reserved 3; | |||
There was a problem hiding this comment.
Not sure if we should first set it to pending_closing_channels = 3 [deprecated = true]; first and just not fill it any more? Then remove it in the next version? Not sure what the reserved does for clients that use a previous RPC client?
There was a problem hiding this comment.
Agree, I too think deprecating first is the better way to do this. (Can even leave it deprecated).
lnrpc/rpc.proto
Outdated
| @@ -2096,8 +2096,7 @@ message PendingChannelsResponse { | |||
| /// Channels pending opening | |||
| repeated PendingOpenChannel pending_open_channels = 2; | |||
|
|
|||
| /// Channels pending closing | |||
| repeated ClosedChannel pending_closing_channels = 3; | |||
| reserved 3; | |||
There was a problem hiding this comment.
Agree, I too think deprecating first is the better way to do this. (Can even leave it deprecated).
The pending closing channels field was present to cover an edge case where coperatively closed channels were closed before upgrade and confirmed after the upgrade. This commit deprecates the field and adds a warning log to cover these edge cases.
carlaKC
force-pushed
the
4067-pendingchannelsrpcadditions
branch
from
1e875fa
to
9e1cdb7
Compare
| case channeldb.ErrChannelNotFound: | ||
|
|
||
| case nil: | ||
| channel.Initiator = historical.IsInitiator |
There was a problem hiding this comment.
Shouldn't the field distinguish between non-initiator and a legacy channel for which no history is present?
There was a problem hiding this comment.
hm yes it should, at the time I thought we wouldn't have legacy channels in pending, but there is of course the old upgrade path option of a channel closing before upgrade, and list pending after. This should use the Initiator enum.
This PR adds channel initiator and fee fields the the pending channels messages where they are missing. It also deprecates the
pending_close_channelsfield, which was kept around to cover the case where somebody upgraded their node after cooperatively closing their channel, but before the close tx confirmed. The change was made in 2018, and pending close is only a temporary state, so it is acceptable to deprecate it. Adding channels to the deprecated field is replaced with a warning log, in case somebody with an old node upgrades and hits this edge case.Fixes: #4067