Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fees on listpeers rpc output #4247

Merged
merged 3 commits into from
Dec 7, 2020

Conversation

m-schmoock
Copy link
Collaborator

@m-schmoock m-schmoock commented Dec 2, 2020

As mentioned in the last developer meeting and additional to #4221 we want to have the local half-channel fees (the ones that are applied when a payment is routed though our node) as part of the listpeers output. Motivation is that this data is non-gossip and currently only available via gossip listchannels.

Example

{
    ...
    "fee_base_msat" : "100msat",
    "fee_proportinal_millionths" : 1000,
    ...
}

This PR adds feature, test and doc.

Copy link
Collaborator

@ZmnSCPxj ZmnSCPxj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 8dbe6f1

Minor quibble about docs but not enough to block this.

doc/lightning-listpeers.7.md Outdated Show resolved Hide resolved
doc/lightning-listpeers.7.md Outdated Show resolved Hide resolved
@rustyrussell
Copy link
Contributor

OK, so minor feedback:

  1. We prefer to use spec names for fields (unless we've already misuse elsewhere). That makes it fee_base_msat and fee_proportional_millionths.
  2. msat field names should end in _msat so that the Python JSON decoder can automagically turn them into Millisatoshi amounts.

This will add `fee_base` (msat) and `fee_ppm` (u32 num) to the RPC
`listpeers` output.

Changelog-Added: fee_base and fee_ppm to listpeers
@m-schmoock
Copy link
Collaborator Author

@rustyrussel
Thx for the feedback. I changed the names.

I figured that in the current listchannels output base_fee_millisatoshi is currently not having a _msat pendant. Should we add this in another PR? If we want to eventually get rid of non-msat fields we need to add this.

@m-schmoock
Copy link
Collaborator Author

@rustyrussell

  1. We prefer to use spec names for fields (unless we've already misuse elsewhere). That makes it fee_base_msat and fee_proportional_millionths.

Note: Just checked, listchannels gives me fee_per_millionth ...

@cdecker
Copy link
Member

cdecker commented Dec 7, 2020

ACK 54af4c9

@cdecker cdecker merged commit 4507203 into ElementsProject:master Dec 7, 2020
@m-schmoock m-schmoock deleted the feat/listpeers_fees branch December 12, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants