Configurable Gateway Fees #2246

okjodom · 2023-04-18T08:34:30Z

Enable configurable gateway fees for routing outgoing payments.
Fees are declared using known RoutingFees structure from lightning crate, and can be configured when setting up the gateway.

Example: export FM_GATEWAY_FEES= "10,2000" means the gateway will run declare a 10 msat base fee and a 2% liquidity based fee to federations served

By default, gatewayd configures a base fee of 0 msat and a marginal fee of 1%, which is on par with current behavior.

Next, we need to introduce a gateway API for configuring routing fees

Longer term, we need more comprehensive definition for gateway fees, as tracked by Discussion #2392

fedimint-client-legacy/src/ln/mod.rs

modules/fedimint-ln-server/src/lib.rs

m1sterc001guy · 2023-04-18T14:55:52Z

Is the next step making an API so that the fees are configurable?

okjodom · 2023-04-18T15:11:57Z

Is the next step making an API so that the fees are configurable?

Yes. Left that out because I believe we have opportunity to introduce better configurable fee declaration before we introduce the api to configure the fees.

Additionally, we might want to make design decisions like supporting dynamic fee configuration per federation, vs some global fee configuration, vs automated fee configuration (based on route hints?)

codecov · 2023-04-18T20:17:08Z

Codecov Report

Patch coverage: 64.16% and project coverage change: +0.03 🎉

Comparison is base (ca46c3a) 61.18% compared to head (c7c5e05) 61.22%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2246      +/-   ##
==========================================
+ Coverage   61.18%   61.22%   +0.03%     
==========================================
  Files         180      180              
  Lines       36185    36286     +101     
==========================================
+ Hits        22141    22217      +76     
- Misses      14044    14069      +25

Impacted Files	Coverage Δ
fedimint-testing/src/gateway.rs	`93.10% <ø> (ø)`
gateway/ln-gateway/src/bin/gatewayd.rs	`0.99% <0.00%> (-0.28%)`	⬇️
gateway/ln-gateway/src/rpc/mod.rs	`81.81% <ø> (ø)`
modules/fedimint-ln-client/src/lib.rs	`9.31% <0.00%> (-0.07%)`	⬇️
modules/fedimint-ln-server/src/lib.rs	`70.33% <0.00%> (-0.35%)`	⬇️
gateway/ln-gateway/src/lib.rs	`56.77% <87.50%> (+0.97%)`	⬆️
modules/fedimint-ln-common/src/lib.rs	`76.47% <92.85%> (+7.13%)`	⬆️
fedimint-client-legacy/src/ln/mod.rs	`71.81% <94.44%> (+0.86%)`	⬆️
fedimint-client-legacy/src/lib.rs	`74.95% <100.00%> (+0.02%)`	⬆️
fedimint-core/src/encoding/mod.rs	`87.94% <100.00%> (+0.47%)`	⬆️
... and 1 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

fedimint-client-legacy/src/lib.rs

gateway/ln-gateway/src/lib.rs

dpc · 2023-04-20T18:24:35Z

fedimint-core/src/encoding/mod.rs

@@ -379,6 +379,29 @@ impl Encodable for lightning_invoice::Invoice {
    }
 }

+impl Encodable for lightning::routing::gossip::RoutingFees {


Not sure if it"s a good idea to depend for our own encodings on data-structures from external crates (that possibly could change).

Also how we are going to implement gateway fees and LN gateway fees, are probably two different things.

Could have fees paid in different currencies supported by federation, or maybe "no gateway fee tuesdays", or some other things? If not, then whatever, let"s just go with this, seems reasonable. If possibly yes, maybe we should go with an enum. But the main variant of that enum should probably be this thing. Not sure if we should copy&paste or use directly.

RE: Also how we are going to implement gateway fees and LN gateway fees, are probably two different things.

Could have fees paid in different currencies supported by federation, or maybe "no gateway fee tuesdays", or some other things? If not, then whatever, let"s just go with this, seems reasonable. If possibly yes, maybe we should go with an enum. But the main variant of that enum should probably be this thing. Not sure if we should copy&paste or use directly.

Fantastic idea IMO. If I adopt it as a proposal, we"d have

#[non_exhaustive] pub enum FeeStructure { LnRouting { fees: RoutingFees } }

To open up future possibility of:

#[non_exhaustive] pub enum FeeStructure { LnRouting { fees: RoutingFees }, Discount { ... } }

Because "Free Gateway Tuesdays" is a fantastic thing :)

RE: Not sure if it"s a good idea to depend for our own encodings on data-structures from external crates (that possibly could change).

We are also guilty in these other accounts, and I"m not keen on maintaining a variant of RoutingFees because we already have a dependency on the crate and type in other scenarios.

Can we fully rely on version control to alert breaks in encodings? Is maintaining our own types the only way to avoid encoding extern types?

Proposing flexible fee structure as a quick follow up to the current PR: #2287

gateway/ln-gateway/Cargo.toml

modules/fedimint-ln-common/src/lib.rs

justinmoon · 2023-04-23T02:34:25Z

Thinking about this a little more, I think we need to achieve the following:

Clients are presented a fee that will be charged for the payment before creating outgoing / incoming contract . They can use this fee to choose a gateway, display this fee to the user, and calculate the amount needed for the outgoing / incoming contracts.
For outgoing payments, gateways can set a fee they will charge for routing payments a given payment, net of routing fees they pay. For incoming payments, gateways can set a separate fee to receive a payment for the user compensating them for locking up capital in channel capacity and ecash balance.

So the first conclusion is that I think we need separate fees for incoming and outgoing payments. They fundamentally represent costs of different resources: outgoing lightning liquidity, and incoming lightning liquidity + cost and risk of holding ecash.

Another conclusion is that no matter how we express these fees in the outgoing case, we need to take them into consideration when the gateway pays the actual invoice. We’ll need to pass these fee calculations on to PayInvoiceRequest.max_fee_percent field in our gRPC spec. Without doing this, the gateway only controls the fees they charge the end-user, but could still lose money routing the payment if routing fees exceed fees charged to the user.

I’m also not sure if lightning::routing::gossip::RoutingFees is the best way to encode our fees, because it seems like it’s intended to express the cost for forwarding a payment across a single channel. The gateway’s job is different: completing the entire payment no matter how many hops it takes. And if we do use RoutingFees to express gateway fees, we"ll need another parameter expressing how much of these fees the gateway will reserve for itself when paying the invoice. For example, if RoutingFees translates to a fee of x for a given invoice, the gateway might want to use a value less than x when calculating PayInvoiceRequest.max_fee_percent to guarantee that they make a profit.

Edit: Client currently hard-codes a routing fee of 0 for the gateway for incoming payments. RoutingFees could be nice to express this fee. Additionally, we would need to have the gateway validate this when it receives an HTLC.

modules/fedimint-ln-common/src/lib.rs

m1sterc001guy · 2023-05-15T14:02:46Z

gateway/ln-gateway/src/bin/gatewayd.rs

@@ -47,6 +49,11 @@ pub struct GatewayOpts {
    /// Gateway webserver authentication password
    #[arg(long = "password", env = "FM_GATEWAY_PASSWORD")]
    pub password: String,
+
+    /// Configured gateway routing fees
+    /// Format: <base_msat>,<proportional_millionths>


Nit: For me I know I would be confused by this format, would probably just prefer separate parameters

Actually I would want us to remove this gateway config soon as we have an API from #2271

m1sterc001guy · 2023-05-15T14:04:40Z

modules/fedimint-ln-client/src/lib.rs

+        let fees = gateway.fees;
+        let base_fee = fees.base_msat as u64;
+        let margin_fee: u64 = if fees.proportional_millionths > 0 {
+            let fee_percent = 1000000 / fees.proportional_millionths as u64;


What does 1000000 represent? Would prefer constant

huh. it"s just the number 1 million used to compute percentage here :)

m1sterc001guy

LGTM, let"s iterate on it

okjodom · 2023-05-15T15:40:16Z

LGTM, let"s iterate on it

thanks for review.

@justinmoon, take a look if you still need this change asap

okjodom mentioned this pull request Apr 18, 2023

Dynamic fees for lightning gateways #524

Closed

okjodom force-pushed the configurable-gw-fees branch from ad96585 to 8ee869f Compare April 18, 2023 10:06

m1sterc001guy reviewed Apr 18, 2023

View reviewed changes

fedimint-client-legacy/src/ln/mod.rs Outdated Show resolved Hide resolved

m1sterc001guy reviewed Apr 18, 2023

View reviewed changes

modules/fedimint-ln-server/src/lib.rs Outdated Show resolved Hide resolved

okjodom force-pushed the configurable-gw-fees branch from b133005 to b2faeaa Compare April 18, 2023 20:11

okjodom marked this pull request as ready for review April 18, 2023 20:27

okjodom requested review from a team as code owners April 18, 2023 20:27

m1sterc001guy previously approved these changes Apr 19, 2023

View reviewed changes

dpc reviewed Apr 19, 2023

View reviewed changes

fedimint-client-legacy/src/lib.rs Outdated Show resolved Hide resolved

dpc reviewed Apr 19, 2023

View reviewed changes

gateway/ln-gateway/src/lib.rs Outdated Show resolved Hide resolved

okjodom marked this pull request as draft April 20, 2023 06:14

okjodom dismissed m1sterc001guy’s stale review via 544ff96 April 20, 2023 12:02

okjodom force-pushed the configurable-gw-fees branch from b2faeaa to 544ff96 Compare April 20, 2023 12:02

okjodom mentioned this pull request Apr 20, 2023

API for configuring Gateway fees #2271

Closed

okjodom force-pushed the configurable-gw-fees branch 2 times, most recently from b833c5f to b9906aa Compare April 20, 2023 13:07

okjodom marked this pull request as ready for review April 20, 2023 13:20

dpc reviewed Apr 20, 2023

View reviewed changes

gateway/ln-gateway/Cargo.toml Show resolved Hide resolved

okjodom marked this pull request as draft April 21, 2023 06:07

okjodom mentioned this pull request Apr 21, 2023

Gateway Fee Structure #2287

Closed

okjodom marked this pull request as ready for review April 21, 2023 09:03

okjodom added the waiting-on-review PR needs review label Apr 21, 2023

douglaz reviewed Apr 21, 2023

View reviewed changes

modules/fedimint-ln-common/src/lib.rs Outdated Show resolved Hide resolved

garyray-k reviewed Apr 30, 2023

View reviewed changes

modules/fedimint-ln-common/src/lib.rs Outdated Show resolved Hide resolved

okjodom marked this pull request as draft May 3, 2023 15:40

okjodom mentioned this pull request May 3, 2023

Gateway Fee Structure okjodom/fedimint#127

Closed

okjodom removed the waiting-on-review PR needs review label May 4, 2023

okjodom added 6 commits May 12, 2023 13:16

feat: support serialization of lightning::routing::gossip::RoutingFees

48f7d91

feat: support encoding of lightning::routing::gossip::RoutingFees

09c4224

feat: require configurable gateway fees

298027f

feat: configure default gateway fees

36c77e7

feat: use default fees in gateway lightning tests

9644331

feat: announce gateway configured fees to fedimint clients

62d020b

okjodom force-pushed the configurable-gw-fees branch from b9906aa to dab8069 Compare May 12, 2023 21:50

okjodom added 2 commits May 12, 2023 16:14

feat: clients use gateway configured fees in outgoing contracts

97174c8

breaking(ln): bump lightning module db to skip migrations

7a99ba0

okjodom force-pushed the configurable-gw-fees branch from dab8069 to 7a99ba0 Compare May 12, 2023 23:20

okjodom added 2 commits May 12, 2023 16:35

feat: support gateway fee configuration

df97a76

feat: show fees in gateway info

c7c5e05

okjodom marked this pull request as ready for review May 13, 2023 00:00

elsirion requested a review from m1sterc001guy May 14, 2023 14:22

m1sterc001guy reviewed May 15, 2023

View reviewed changes

m1sterc001guy approved these changes May 15, 2023

View reviewed changes

justinmoon merged commit 9be58bb into fedimint:master May 19, 2023

okjodom mentioned this pull request Jun 19, 2023

LND gateway implementation doesn"t specify max fee #2663

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configurable Gateway Fees #2246

Configurable Gateway Fees #2246

okjodom commented Apr 18, 2023 •

edited

Loading

m1sterc001guy commented Apr 18, 2023

okjodom commented Apr 18, 2023

codecov bot commented Apr 18, 2023 •

edited

Loading

dpc Apr 20, 2023 •

edited

Loading

okjodom Apr 21, 2023

okjodom Apr 21, 2023

okjodom Apr 21, 2023

justinmoon commented Apr 23, 2023 •

edited

Loading

m1sterc001guy May 15, 2023

okjodom May 15, 2023

m1sterc001guy May 15, 2023

okjodom May 15, 2023

m1sterc001guy left a comment

okjodom commented May 15, 2023

Configurable Gateway Fees #2246

Configurable Gateway Fees #2246

Conversation

okjodom commented Apr 18, 2023 • edited Loading

m1sterc001guy commented Apr 18, 2023

okjodom commented Apr 18, 2023

codecov bot commented Apr 18, 2023 • edited Loading

Codecov Report

dpc Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

okjodom Apr 21, 2023

Choose a reason for hiding this comment

okjodom Apr 21, 2023

Choose a reason for hiding this comment

okjodom Apr 21, 2023

Choose a reason for hiding this comment

justinmoon commented Apr 23, 2023 • edited Loading

m1sterc001guy May 15, 2023

Choose a reason for hiding this comment

okjodom May 15, 2023

Choose a reason for hiding this comment

m1sterc001guy May 15, 2023

Choose a reason for hiding this comment

okjodom May 15, 2023

Choose a reason for hiding this comment

m1sterc001guy left a comment

Choose a reason for hiding this comment

okjodom commented May 15, 2023

okjodom commented Apr 18, 2023 •

edited

Loading

codecov bot commented Apr 18, 2023 •

edited

Loading

dpc Apr 20, 2023 •

edited

Loading

justinmoon commented Apr 23, 2023 •

edited

Loading