multi: specify retrieving active, inactive, public, and private channels from listchannels rpc #834
Conversation
lnrpc/rpc.proto
Outdated
| /// Whether this channel is active or not | ||
| bool active = 1 [json_name = "active"]; | ||
|
|
||
| /// Whether this channel is advertised to the network or not | ||
| bool public = 2 [json_name = "public"]; |
There was a problem hiding this comment.
in OpenChannelRequest we are using the private flag. Maybe use that here as well, to be consistent? :)
rpcserver.go
Outdated
| // Next, we'll determine whether we should add this channel to | ||
| // our list depending on the type of channels requested to us. | ||
| isActive := peerOnline && linkActive | ||
| isPublic := dbChannel.ChannelFlags&lnwire.FFAnnounceChannel == |
There was a problem hiding this comment.
This should be isPublic := dbChannel.ChannelFlags&lnwire.FFAnnounceChannel != 0
rpcserver.go
Outdated
| lnwire.FFAnnounceChannel | ||
|
|
||
| switch { | ||
| case in.Active && in.Inactive || !in.Active && !in.Inactive: |
There was a problem hiding this comment.
I would recommend just making the "in params" active_only, inactive_only etc, then just filter out channels that don't match the criteria (and you could return error early if two incpompatible flags are both set). Should simplify this code significantly!
There was a problem hiding this comment.
Great feature, thanks @wilmerpaulino! I like the new behavior that you and @halseth worked out for the flags. Just one small request for fixing the updates to the proto file
lnrpc/rpc.proto
Outdated
| /// Whether this channel is active or not | ||
| bool active = 1 [json_name = "active"]; | ||
|
|
||
| /// Whether this channel is advertised to the network or not | ||
| bool private = 2 [json_name = "private"]; |
There was a problem hiding this comment.
in order to maintain backwards compatibility for proto files, sequence numbers for existing field should never change. this field should be added to the end of the definition, and the indexes of prior fields should have their indexes reverted
There was a problem hiding this comment.
Good point. I believe since we don’t actually store the serialized data of the invoice proto anywhere, it should be fine. I could be mistaken though. I’ll apply your suggestion for consistency regardless.
There was a problem hiding this comment.
You are right, for our use case it's probably okay. But that doesn't preclude others from storing/continuing to use the proto definitions ;)
There was a problem hiding this comment.
Definitely! Will fix.
|
@wilmerpaulino thank you for updating the proto files, LGTM! can you rebase/fixup? |
|
Done! |
| @@ -1472,6 +1472,16 @@ func (r *rpcServer) PendingChannels(ctx context.Context, | |||
| func (r *rpcServer) ListChannels(ctx context.Context, | |||
| in *lnrpc.ListChannelsRequest) (*lnrpc.ListChannelsResponse, error) { | |||
|
|
|||
| if in.ActiveOnly && in.InactiveOnly { | |||
| return nil, fmt.Errorf("either `active_only` or " + | |||
There was a problem hiding this comment.
Not exactly right, you should be able to not set any of them :)
rpcserver.go
Outdated
| } | ||
|
|
||
| if in.PublicOnly && in.PrivateOnly { | ||
| return nil, fmt.Errorf("either `public_only` or " + |
|
Ready for merge! LGTM 📺 |
This PR adds parameters to the
listchannelsRPC in order to specify retrieving active, inactive, public, and private channels.Also fixes #810.