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

[bug]: InboundFees are overwritten if they were not explicitly specified in lncli #8614

Open
feelancer21 opened this issue Apr 2, 2024 · 6 comments · May be fixed by #8758
Open

[bug]: InboundFees are overwritten if they were not explicitly specified in lncli #8614

feelancer21 opened this issue Apr 2, 2024 · 6 comments · May be fixed by #8758
Labels
bug Unintended code behaviour good first issue Issues suitable for first time contributors to LND P2 should be fixed if one has time policy

Comments

@feelancer21
Copy link
Contributor

Background

I run the current master on testnet and tested the setting of inbound fees with lncli updatechanpolicy. But the inbound fees are not preserved if I run the command without the command line flags

Your environment

  • testnet
  • version of lnd
    fn/v1.0.5-129-gb1175514f

Steps to reproduce

  1. set inbound fees with
    lncli updatechanpolicy --base_fee_msat 0 --fee_rate_ppm 200 --time_lock_delta 144 --inbound_fee_rate_ppm -100 --inbound_base_fee_msat -1 --chan_point 9952586a20fecd4d99c3f400c44f10e17c674f1050f773656a42acbf7b80901c:0

  2. check the inbound fees

lncli getchaninfo 2579374014416486400
{
    "channel_id":  "2579374014416486400",
    "chan_point":  "9952586a20fecd4d99c3f400c44f10e17c674f1050f773656a42acbf7b80901c:0",
    "last_update":  1712092954,
    "node1_pub":  "024e97ccd1382cb736f86694e0d1312cb53fdcbffb93f13f2626702c3d74cabf99",
    "node2_pub":  "038214ce28b285a6233300fc114184c1e1da7bab146d705679339c304e118330a1",
    "capacity":  "20000",
    "node1_policy":  {
        "time_lock_delta":  144,
        "min_htlc":  "2100000",
        "fee_base_msat":  "0",
        "fee_rate_milli_msat":  "200",
        "disabled":  false,
        "max_htlc_msat":  "19800000",
        "last_update":  1712092954,
        "custom_records":  {
            "55555":  "ffffffffffffff9c"
        },
        "inbound_fee_base_msat":  -1,
        "inbound_fee_rate_milli_msat":  -100
    },
    "node2_policy":  {
        "time_lock_delta":  144,
        "min_htlc":  "2100000",
        "fee_base_msat":  "0",
        "fee_rate_milli_msat":  "250",
        "disabled":  false,
        "max_htlc_msat":  "7858400",
        "last_update":  1712092021,
        "custom_records":  {},
        "inbound_fee_base_msat":  0,
        "inbound_fee_rate_milli_msat":  0
    },
    "custom_records":  {}
}
  1. Set the policy again without inbound fees
    lncli updatechanpolicy --base_fee_msat 0 --fee_rate_ppm 300 --time_lock_delta 144 --chan_point 9952586a20fecd4d99c3f400c44f10e17c674f1050f773656a42acbf7b80901c:0

  2. Check the inbound fees again

lncli getchaninfo 2579374014416486400
{
    "channel_id":  "2579374014416486400",
    "chan_point":  "9952586a20fecd4d99c3f400c44f10e17c674f1050f773656a42acbf7b80901c:0",
    "last_update":  1712093293,
    "node1_pub":  "024e97ccd1382cb736f86694e0d1312cb53fdcbffb93f13f2626702c3d74cabf99",
    "node2_pub":  "038214ce28b285a6233300fc114184c1e1da7bab146d705679339c304e118330a1",
    "capacity":  "20000",
    "node1_policy":  {
        "time_lock_delta":  144,
        "min_htlc":  "2100000",
        "fee_base_msat":  "0",
        "fee_rate_milli_msat":  "300",
        "disabled":  false,
        "max_htlc_msat":  "19800000",
        "last_update":  1712093293,
        "custom_records":  {
            "55555":  "0000000000000000"
        },
        "inbound_fee_base_msat":  0,
        "inbound_fee_rate_milli_msat":  0
    },
    "node2_policy":  {
        "time_lock_delta":  144,
        "min_htlc":  "2100000",
        "fee_base_msat":  "0",
        "fee_rate_milli_msat":  "250",
        "disabled":  false,
        "max_htlc_msat":  "7858400",
        "last_update":  1712092021,
        "custom_records":  {},
        "inbound_fee_base_msat":  0,
        "inbound_fee_rate_milli_msat":  0
    },
    "custom_records":  {}
}

Expected behaviour

Inbound Fees have to be preserved.

@feelancer21 feelancer21 added bug Unintended code behaviour needs triage labels Apr 2, 2024
@Roasbeef
Copy link
Member

Roasbeef commented Apr 2, 2024

I think it's an existing bug with the way updatechanpolicy works in that you need to set everything otherwise it doesn't know what was a default vs just setting that value to nothing (eg: zero fees).

@saubyk saubyk added P2 should be fixed if one has time policy and removed needs triage labels Apr 3, 2024
@feelancer21
Copy link
Contributor Author

Looked in the code because I wondered why max_htlc_msat and min_htlc_msat are preseverd. max_htlc_msat is replaced by the current value if a value of zero is submitted. And min_htlc_msat is set only if min_htlc_msat_specified is set.

I think we need also a boolean like inbound_fees_specified to make UpdateChanPolicy backwards compatible. If it is set then both inbound_base_fee_msat and inbound_fee_rate_ppm are updated.

@yyforyongyu yyforyongyu added the good first issue Issues suitable for first time contributors to LND label Apr 24, 2024
@SulaimanAminuBarkindo
Copy link

I'll jump on this. If there are any more relevant details that i might need, please share them. Thanks

@feelancer21
Copy link
Contributor Author

I'll jump on this. If there are any more relevant details that i might need, please share them. Thanks

Pretty cool. Had a short look on this some days ago. But haven't really started. So let's go.

@feelancer21
Copy link
Contributor Author

Started working on this. PR soon (tm)

https://github.com/feelancer21/lnd/tree/preserve-inbound-fees

@SulaimanAminuBarkindo
Copy link

Started working on this. PR soon (tm)

https://github.com/feelancer21/lnd/tree/preserve-inbound-fees

Awesome I am caught up with something. Looking forward to the PR

@feelancer21 feelancer21 linked a pull request May 14, 2024 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unintended code behaviour good first issue Issues suitable for first time contributors to LND P2 should be fixed if one has time policy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants