alexbosworth / lightning

Lightning client methods
MIT License
126 stars 32 forks source link

add inbound fees fields to channel event #164

Closed bufo24 closed 3 months ago

bufo24 commented 4 months ago

Adds needed fields to be compatible with the new fields added in this PR:

https://github.com/lightningnetwork/lnd/pull/8723

Will change from draft to ready for review whenever the LND PR gets merged

alexbosworth commented 4 months ago

I was thinking about calling this source_discount rather than inbound_fee to give more clarity towards what it is typically doing, reducing the outbound fee rate based on the source

bufo24 commented 4 months ago

I know LND also calls it inbound fees, so it's nice to be consistent with the naming, so other devs directly understand that we're talking about the same thing, instead of having to figure out it source_discount and inbound fees are the same.

alexbosworth commented 4 months ago

I don't think it would be the same because I'd flip the sign on it, although I have to play with it to know

Consistency with naming isn't a goal of the arguments generally

alexbosworth commented 4 months ago

I'm playing around with this on the other APIs, just a heads up not 100% settled on naming but I can adjust it

alexbosworth commented 4 months ago

OK what I settled on is:

inbound_fee_base_msat should map to inbound_base_discount_mtokens but negative sign (unless 0, then 0), and a string rather than a number

inbound_fee_rate_milli_msat should map to inbound_rate_discount but negative sign (unless 0, then just 0)

bufo24 commented 4 months ago

LND already gives the negative values, do you want to make them positive?

bufo24 commented 4 months ago

And I tested this and I see that the inbound fees are coming in like numbers, not strings:

{
    custom_records: {
      '\x03Ù\x00\x00\x00\x00\x00\x00': <Buffer ff ff ff f6 ff ff ff ec>
    },
    time_lock_delta: 50,
    min_htlc: '1000',
    fee_base_msat: '10',
    fee_rate_milli_msat: '10000000',
    disabled: false,
    max_htlc_msat: '49500000',
    last_update: 0,
    inbound_fee_base_msat: -10,
    inbound_fee_rate_milli_msat: -20
  }
alexbosworth commented 4 months ago

Yeah I saw that too, I pushed some changes for the existing inbound fees that should show the transform