Add getchannelfees command

[0e319b6]

Motivation

Currently there is no good way to view channel fees, especially all in one condensed view. I have a hacky bash alias for this: lightning-cli summary | awk '{print $2}' | egrep [0-9]{5}x[0-9]+x[0-9]); do echo $scid: ; lightning-cli listchannels $scid | grep fee; done but that's obviously suboptimal.

Proposed command format

getchannelfees [id]

The optional argument id is either a peerid pubkey of one of our channel peers, a channel id, or a short channel ID. If no id is provided, fee data for all of our node's channels will be returned.

Proposed RPC format

I propose this command returns an object with a field named channel_fees that is a list of objects of the following format:

{
    "destination": <peer id pubkey>,
    "short_channel_id": <scid>,
    "from_destination_base_fee_millisatoshi": <int>,
    "from_destination_fee_per_millionth": <int>,
    "to_destination_base_fee_millisatoshi": <int>,
    "to_destination_fee_per_millionth": <int>
}

For each such object listed, if our node participates in the corresponding channel, destination shall be our corresponding channel peer's peerid. In the alternative, destination may be the peerid of either of the channel peers; a potential source field can be omitted as it is implied by destination and short_channel_id. The other fields should hopefully be self-explanatory.

Each channel shall be represented by one such object with fee data for both directions.

In case of an error, the channel_fees list shall be empty.

Optional extensions

We could also allow id to represent a peerid of a node who is not one of our node's channel peers. In this case, we can run Dijkstra's algorithm on the channel graph and return the lowest cumulative fees between our node and id.

A further extension would additionally allow an optional [amount] argument that, if id represents a peerid to which we are not peered, represents a minimum channel size for the above Dijkstra's algorithm. During the operation of that search, any channel with a capacity less than amount would be ignored.

[ZmnSCPxj]

We could also allow id to represent a peerid of a node who is not one of our node's channel peers. In this case, we can run Dijkstra's algorithm on the channel graph and return the lowest cumulative fees between our node and id.

Because of the proportional fee, this requires that you provide a reference amount to be sent to the designated peer. We cannot "just" accumulate the fees along the way due to how the math works out.

And if you want a Dijkstra algorithm, that already exists in getroute with fuzzpercent=0 (default is 5). You need to provide a reference amount, but the total fees would be the amount in the first hop minus the reference amount.

[m-schmoock]

@0e319b6 @ZmnSCPxj

My input on this:

• We should just return local fees and not rely on the remote fees from_destination_... as they are from gossip. In this case you can rename to_destination_base_fee_millisatoshi to just base_fee_millisatoshi and ppm accordingly.
• I would not include routing, as this gets really complex.
• We can also add the fees info to the listpeers output. It actually makes sense to have both (at least to me). Using gossip is currently the only option to get the local fees is not reliable because of the update delays and such. Using listpeers returns too much data for some plugins. But adding it also to listpeers is 'correct' as the fees are distinct local channel configs (non gossip values).

Regarding

If no id is provided, fee data for all of our node's channels will be returned.

We could do that, but if we do we should have an array as output format, even when only a single channel is requested. The reason why I prefer an array here is that the alternative (mapping) would require some key and if a plugin uses this RPC command it needs to know what to look for. If the plugin requested a single channel it can safely do node.rpc.getchannelfees(<scid>)[0] as the command should fail if a single requested channel was not found.

output format suggestion

{
    "peer": <pubkey>,
    "short_channel_id": <scid>,
    "direction": <1/0>,
    "base_fee_millisatoshi": <int>,
    "fee_per_millionth": <int>
}

[ZmnSCPxj]

We should just return local fees and not rely on the remote fees

So you mean only from us->peer direction? Since the symmetric command is setchannelfees, this makes sense to me, as you can only setchannelfees the us->peer direction.

We could do that, but if we do we should have an array as output format, even when only a single channel is requested.

Yes, this is indeed what we do on a lot of our interfaces (and which we should do for all, not that I remember any interface that does not do this).

Though we tend to use the terminology listfoo rather than getfoo for such arrayed interfaces.

[m-schmoock]

So you mean only from us->peer direction? Since the symmetric command is setchannelfees, this makes sense to me, as you can only setchannelfees the us->peer direction.

Yes, that was my intention, I just don't like to query gossip for local config data, which the fee settings are. What do you think about also adding this to the listpeers ?

Yes, this is indeed what we do on a lot of our interfaces (and which we should do for all, not that I remember any interface that does not do this).

The setchannelfee also allows having scid == "all" to set fees for all channels. So that would make sense. As a counterpart of setchannelfee naturally getchannelfee (also for a list) seems intuitive but I could also live with listchannelfees. Not my call.

[m-schmoock]

@0e319b6 @ZmnSCPxj
Can we update the proposed RPC format and details?

• just half channel. no 'from_destination...' , as this is the fee we control, other is gossip
• remove the routing stuff
• maybe also add base_fee_millisatoshi and fee_per_millionth to listpeers channels

[0e319b6]

• I am OK with removing remote fee details. Although my main motivation was to have a central place to view (both local and remote) fees on my channels, I recognize the complication of relying partially on data from gossip. I would still like a good solution for this, though...
• I am ambivalent about adding stuff to listpeers. Your call.
• I am cool with using the name listchannelfees for consistency.

New command format

listchannelfees [id]

Not specifying id is the same as using 'all', which returns objects for all channels in which we participate. Otherwise id is a channel id or an scid

New RPC format

{
    "channel_fees": [
        {
            "peer": "<peer id pubkey>",
            "short_channel_id": "<scid>",
            "base_fee_millisatoshi": <int>,
            "fee_per_millionth": <int>
        }
    ]
}

[m-schmoock]

@0e319b6

I would still like a good solution for this, though...

Check out lightning-cli listchannels ... which gives you remote (and local) fees for all channels using gossip. The problem using gossip is when doing local stuff (changing fees with API or plugins) that gossip is slow and unreliable for pure local information.

About the listpeers addition, we can do this in another issue...

Also ( @ZmnSCPxj and @0e319b6 ) : I think we should remove the outer object (the one containing channel_fees) and directly return the inner array as response, as I don't see that it adds anything.

I agree choosing getchannelfees over listchannelfees. But I just noticed we have a glitch with the naming of the setchannelfee function (Note: It's singular, without s at the end). Since we won't rename the already existing function to plural, this would give us a singular setter (which can even set multiple channels using 'all') and a plural getter. The problem here is really more with the existing setter -.- ... All this is, however, not my call ;)

[ZmnSCPxj]

directly return the inner array as response, as I don't see that it adds anything.

It allows us to add more keys in the future, just in case. Note that all existing listfoo have the same pattern of having a wrapping object with a key containing an array (listnodes, listchannels, listforwards, listfunds, listinvoices, listpays, listpeers, listtransactions). The outer object lets us do stuff like add a "warning" field, or adding other information or comments in the future. It is basically our RPC policy to always return an object, and for cases where we return an array, to wrap the array in an object containing a single key with that array.

[m-schmoock]

Ack on the outer object.

@ZmnSCPxj See #4247 on the naming in the RPC output. I'm not sure if we should use more pregnant names (fee_base (msat string), fee_ppm(u32 number)) or be more inline with the current proposal as in listchannels (base_fee_millisatoshi , fee_per_millionth) output, which I find hard to remember. To make things even worse, the current setchannelfee rpc command uses just base and ppm in its response as confirmation when one did change these values for a local half-channel.

[m-schmoock]

After thinking about this we should recycle the same variable names as for the setchannelfee result which is just base and ppm. Because get(list)channelfee(s?) reflects the getter for that setter function so we should reuse this syntax and also since we already know that the result is 'fee' related there no point in adding the prefix fee_... again for this.

Using the gossip style (base_fee_millisatoshi and fee_per_millionth) is not a good idea as the variable names are hard to remember and also this function is not gossip related.

[kristapsk]

New command format

listchannelfees [id]

Not specifying id is the same as using 'all', which returns objects for all channels in which we participate. Otherwise id is a channel id or an scid

New RPC format

{
    "channel_fees": [
        {
            "peer": "<peer id pubkey>",
            "short_channel_id": "<scid>",
            "base_fee_millisatoshi": <int>,
            "fee_per_millionth": <int>
        }
    ]
}

You can get most of this information using my feereport plugin (although it will show that for all channels, not specific one; but as a bonus you also get summary of routing fee earnings in a day, week and month)

[0e319b6]

@m-schmoock Yup, that's how I view this information currently. See my bash one-liner in the issue "Motivation" section.
@kristapsk Looks cool, thanks for pointing it out. I like the earnings breakdown by day/week/month. I think there is still demand, though, for a get/listchannelfee(s) to complement setchannelfee.

I strongly prefer to have 'msat' in the name of a field when the value it describes is in msat. I also think 'fee' should be in there, because although this is in the RPC response to a "fee" command, there are other fields in the object and we should be descriptive. I like how lnd calls it base_fee_msat. However, I do not like lnd's fee per millionth name, fee_per_mil...

New new proposed RPC format

{
    "channel_fees": [
        {
            "peer": "<peer id pubkey>",
            "short_channel_id": "<scid>",
            "base_fee_msat": <int>,
            "fee_ppm": <int>
        }
    ]
}

[0e319b6]

Actually, after thinking about it some more, I prefer just ppm over fee_ppm. This will not only match setchannelfee but ppm is a specific term only ever used in the context of channel fees, so its meaning should be unambiguous to anyone reasonably familiar with the command/RPC format.

[m-schmoock]

@0e319b6
See Rustys comment about the naming within the listpeers output: #4247 (comment)

So two things:

• the type of "base_fee_msat" will be a string (not int) that will be auto converted to Millisatoshi using the pyln client ;)
• Regarding "fee_ppm" Rusty said "we prefer to use spec names for fields". I personally would prefer "fee_ppm" as its easier to remember and type. But this would mean we should go for e.g. "fee_proportional_millionths". In this particular case we should also reconsider renaming "base_fee_msat" to "fee_base_msat".

[0e319b6]

Oh, thanks for pointing that comment out. Agreed on fee_base_msat. And I also prefer just ppm.

[m-schmoock]

@0e319b6

Oh, thanks for pointing that comment out. Agreed on fee_base_msat. And I also prefer just ppm.

I didn't meant ppm without the _fee prefix. However, I suggest reading through rustys comment regarding the name of that variable #4247 (comment)

[0e319b6]

So, I'm unclear. Would you prefer fee_ppm or fee_proportional_millionths? Or I can choose and you're OK with either one?

[m-schmoock]

Go for fee_base_msat (value then Millisatoshi e.g. "1000msat") and fee_proportional_millionths numeric value e.g. 10
This is the same I used to add it to listpeers in a recent PR

[carboncls]

I would like to add that it would be very useful to set fees on multiple but select channels. When you use setchannelfee all then all channel fees are set. Else, setchannelfee can only change the fee of individual channels. Usually, when a channel has a lot of outbound liquidity, you want to lower the fees and rebalance the channel that way. It would be very handy if you could do that for multiple but select channels in one command.

[cdecker]

Not sure I understand what the advantage over looping over channels and setting them individually is. We'd be doing that internally anyway if we provide such a batch rpc method. The Problem mostly is that these batch RPCs are very specialized and require the user to build valid json payloads, which not everyone can do.