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

confusing unit descriptions for "fee_per_mil" and "fee_rate" #4155

Open
AndySchroder opened this issue Apr 6, 2020 · 4 comments
Open

confusing unit descriptions for "fee_per_mil" and "fee_rate" #4155

AndySchroder opened this issue Apr 6, 2020 · 4 comments
Labels
documentation Documentation changes that do not affect code behaviour fees Related to the fees paid for transactions (both LN and funding/commitment transactions) graph P3 might get fixed, nice to have v0.12

Comments

@AndySchroder
Copy link

For fee_per_mil we have a definition of "The amount charged per milli-satoshis transferred expressed in millionths of a satoshi.". I interpret the units for this to be (1sat/1milli-sat)*(10^6 micro-sat)/(1sat)=(10^6 micro-sat)/(1milli-sat) .

For fee_rate we have a definition of "The effective fee rate in milli-satoshis. Computed by dividing the fee_per_mil value by 1 million.". If I take the above assumption, then I get ((10^6 micro sat)/(1milli-sat))/(10^6 microsat/1sat)=1sat/1milli-sat , which is not in milli-sat.

Should the definition of fee_per_mil read "The milli-satoshi amount charged per milli-satoshis transferred expressed in micro-satoshi per milli-satoshi"`?

Should the definition of fee_rate be "The milli-satoshi amount charged per milli-satoshis transferred, which is equal to fee_per_mil*10^6"? If this is correct, fee_rate is unitless.

Is there a clearer way to present it?

@AndySchroder
Copy link
Author

Note, the above definitions came from ChannelFeeReport.

Note: lnd -h indicates:

bitcoin.feerate, The fee rate used when forwarding payments on our channels. The total fee charged is basefee + (amount * feerate / 1000000), where amount is the forwarded amount. (default: 1)

So, I'm even more confused at this point. If I look at a channel, using lncli feereport, fee_rate=0.000001 and fee_per_mil=1, which contradicts.

Also note, lncli help updatechanpolicy says "fee_rate, the fee rate that will be charged proportionally based on the value of each forwarded HTLC, the lowest possible rate is 0 with a granularity of 0.000001 (millionths)." This seems to be in agreement with ChannelFeeReport, but not lnd -h.

@Roasbeef Roasbeef added the documentation Documentation changes that do not affect code behaviour label Apr 6, 2020
@Roasbeef
Copy link
Member

Roasbeef commented Apr 6, 2020

Related to #1523

@Roasbeef Roasbeef added fees Related to the fees paid for transactions (both LN and funding/commitment transactions) graph labels Apr 6, 2020
@Roasbeef Roasbeef added this to the 0.12.0 milestone Jul 22, 2020
@Roasbeef Roasbeef added the v0.12 label Jul 22, 2020
@Roasbeef Roasbeef added this to To do in v0.12.0-beta Jul 22, 2020
@yyforyongyu
Copy link
Collaborator

I was looking into this issue.

A bit confused though.

My understanding of the feerate comes from this line in link.go and this test. To put it here, the expected fee is calculated using,

f.BaseFee + (htlcAmt*f.FeeRate)/1000000

Since both f.BaseFee and htlcAmt are in msat, reversing the equation, the f.FeeRate is then using a uint as a billionth of a satoshi per msat, which is 'nano-sat' per msat, or, nsat / msat, this is more clear in the equation,

msat + (msat *  nsat / msat) / 1e6

And my understanding of the fee_rate is, the amount to charge in nsat to route 1 msat. If that's correct, I'd suggest two changes.

  • The first is to fix the inaccuracies in the documentations, which is what this issue is about.
  • A further approach, is to introduce a new type, lnwire.NanoSatoshi, to address the speciality of the fee rate used in routing payments. Also we could refactor some of the variable names.

@jhoenicke
Copy link

I think stating it in nanosat per millisat confuses people even more. It's a fraction. Just say its in "per million" (and don't abbreviate it as per_mil, since that may be read as per mille, which is just per thousand). We are used to compute in percent and this should follow this notion only it is in "per million" not "per hundred". There is also ppm (parts per million) but I'm not sure if it should be used here, since there are no particles.

@Roasbeef Roasbeef removed this from To do in v0.12.0-beta Dec 2, 2020
@Roasbeef Roasbeef modified the milestones: 0.12.0, 0.13.0 Dec 12, 2020
@Roasbeef Roasbeef added the P3 might get fixed, nice to have label Jan 28, 2021
@Roasbeef Roasbeef added this to Blocked in v0.13.0-beta Jan 28, 2021
@Roasbeef Roasbeef removed this from the 0.13.0 milestone Mar 23, 2021
@Roasbeef Roasbeef removed this from Blocked in v0.13.0-beta Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation changes that do not affect code behaviour fees Related to the fees paid for transactions (both LN and funding/commitment transactions) graph P3 might get fixed, nice to have v0.12
Projects
None yet
Development

No branches or pull requests

4 participants