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

Add "receive payment" snippets for channel opening fees #122

Merged
merged 39 commits into from
Apr 30, 2024

Conversation

ok300
Copy link
Contributor

@ok300 ok300 commented Jan 12, 2024

This PR documents the channel opening fee and provides sample code snippets to show how to retrieve it after receive_payment.

Fixes #109

Copy link
Contributor

@dangeross dangeross left a comment

Choose a reason for hiding this comment

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

Looks good

Base automatically changed from ok300-retrieve-invoice-receive-payment to main January 13, 2024 12:15
@JssDWt
Copy link
Contributor

JssDWt commented Jan 19, 2024

@ok300 Let's try to communicate to the developers exactly how they should show the fees involved to the user. Maybe even in a separate section called 'Communicating fees'?

What about something like this?

## Communicating fees
In the LSP model, fees are involved when the user wants to receive a payment, but doesn't have sufficient receivable amount. This section provides recommendations on how to communicate these fees to a user.

### Before receiving a payment
When the user wants to receive a payment, a setup fee is paid when the resulting invoice would exceed the receivable amount.
The setup fee is made up of two parts:
- A minimum fee
- A proportional fee based on the amount

Before creating an invoice, the amount the user will want to receive is yet unknown. It is recommended to show the user a message consisting of the following information:
`A setup fee of x% with a minimum of y sats will be applied for receiving more than z sats.`

Below code sample constructs this message.
(TODO: code sample)

### When an invoice is created
After calling receive_payment, you would typically show the recipient a screen containing a QR code with the invoice that the sender can scan.  

This is another point to show the user the opening fees applied to the invoice. At this point the amount the user wants to receive is known, so the message can be more concise:

`A setup fee of x sats is applied to this invoice.`

The fiat amount can be included. In case of a mobile app it is recommended to communicate to the user that the app has to be run in the foreground in order to be able to receive the payment.

Below code sample constructs this message.
(TODO: code sample)

### Receive onchain
For receiving onchain, there is a minimum and a maximum amount the user can receive. The fees are made up of the same components as receiving a lightning payment.

The user gets a onchain address from receive_onchain. There is no way to know ahead of time exactly the amount that will be received on this address, so it is recommended to show the user the receivable boundaries and the fees involved:

`Send more than v sats and up to w sats to this address. A setup fee of x% with a minimum of y sats will be applied for sending more than z sats. This address can only be used once.`

Below code sample constructs this message.
(TODO: code sample)

@ok300
Copy link
Contributor Author

ok300 commented Jan 20, 2024

@JssDWt is the Rust snippet in b6427b4 close to what you had in mind?

@JssDWt
Copy link
Contributor

JssDWt commented Jan 20, 2024

@JssDWt is the Rust snippet in b6427b4 close to what you had in mind?

Yes that is excellent.

@dangeross
Copy link
Contributor

I think you can simplify the example and text using the open_channel_fee SDK method. It can be used to communicate if there are fees and the fee params involved during the pre-invoice phase when the user is entering an amount.

@ok300
Copy link
Contributor Author

ok300 commented Jan 21, 2024

Thanks @dangeross , that's exactly what I was looking for!

@ok300
Copy link
Contributor Author

ok300 commented Jan 21, 2024

Looking closer, open_channel_fee needs an amount_msat argument, so it cannot be used in the pre-invoice phase.

Setting it to 0 will return no fees and no fee param. Setting it to a small amount like 1, could return no fee param if the user already has a channel.

Maybe we need a similar SDK method to prepare all fee thresholds for the pre-invoice phase?

pub async fn receive_payment_fee_info(&self) -> SdkResult<ReceivePaymentFeeInfo> {
    let lsp_info = self.lsp_info().await?;
    let opening_fees = lsp_info.opening_fee_params_list.get_cheapest_opening_fee_params()?;

    Ok(ReceivePaymentFeeInfo {
        fee_percentage: opening_fees.proportional * 100_f64 / 1_000_000_f64,
        min_fee_sat: opening_fees.min_msat / 1_000,
        inbound_liquidity_sat: self.node_info()?.inbound_liquidity_msats / 1000,
    })
}


/// The values needed for the app developer to show a message like
/// "A setup fee of X% with a minimum of Y sats will be applied for receiving more than Z sats."
pub struct ReceivePaymentFeeInfo {
    pub fee_percentage: f32, // X
    pub min_fee_sat: u64, // Y
    pub inbound_liquidity_sat: u64 // Z
}

@ok300
Copy link
Contributor Author

ok300 commented Jan 21, 2024

I made the snippet more compact in 0ab6370 .

I'm in favor of an SDK method, what do you think?

Alternatively, we could use this (compacted) snippet, although unless people copy-paste it, those multiplications and divisions are pretty error-prone.

@dangeross
Copy link
Contributor

The docs snippet is good, but only works in rust as get_cheapest_opening_fee_params is not available otherwise. I would prefer improving open_channel_fee over adding another SDK method.

@JssDWt
Copy link
Contributor

JssDWt commented Jan 21, 2024

I think you can simplify the example and text using the open_channel_fee SDK method. It can be used to communicate if there are fees and the fee params involved during the pre-invoice phase when the user is entering an amount.

That's true, that's simpler. I took a look at c-breez. There we do what @ok300 wrote in the example. But open_channel_fee is simpler, because then we don't have to explain the opening_fee_params exactly.

@ok300
Copy link
Contributor Author

ok300 commented Jan 22, 2024

I think I found a way to avoid relying on get_cheapest_opening_fee_params in eb9a0b0. It's still verbose, but it's using the SDK method open_channel_fee as is.

Alternatively, I tweaked the SDK method to pre-calculate these fields: breez/breez-sdk-greenlight#737 . With this change, the snippet would make only 1 SDK call: sdk.open_channel_fee(amount = 0).

The 2nd way is better IMO, what do you think?

@ok300 ok300 force-pushed the ok300-add-channel-open-fees-receive-payment branch from eb9a0b0 to afdea04 Compare January 30, 2024 15:48
@ok300 ok300 marked this pull request as draft January 30, 2024 15:49
@ok300 ok300 force-pushed the ok300-add-channel-open-fees-receive-payment branch from 829c6fe to c7f078c Compare February 7, 2024 05:51
@ok300
Copy link
Contributor Author

ok300 commented Feb 7, 2024

Snippet for fee info before receiving a LN payment:

let inbound_liquidity_msat = sdk.node_info()?.inbound_liquidity_msats;
let inbound_liquidity_sat = inbound_liquidity_msat / 1000;
let opening_fee_response = sdk
.open_channel_fee(OpenChannelFeeRequest {
amount_msat: inbound_liquidity_msat + 1,
expiry: None,
})
.await?;
if let Some(opening_fees) = opening_fee_response.used_fee_params {
let fee_percentage = (opening_fees.proportional * 100) as f64 / 1_000_000_f64;
let min_fee_sat = opening_fees.min_msat / 1_000;
if inbound_liquidity_sat == 0 {
info!("A setup fee of {fee_percentage}% with a minimum of {min_fee_sat} sats will be applied.");
} else {
info!("A setup fee of {fee_percentage}% with a minimum of {min_fee_sat} sats will be applied for receiving more than {inbound_liquidity_sat} sats.");
}
}

Snippet for fee info before swaps:

let swap_info = sdk
.receive_onchain(ReceiveOnchainRequest::default())
.await?;
let min_deposit_sat = swap_info.min_allowed_deposit;
let max_deposit_sat = swap_info.max_allowed_deposit;
let inbound_liquidity_sat = sdk.node_info()?.inbound_liquidity_msats / 1000;
if let Some(swap_opening_fees) = swap_info.channel_opening_fees {
let fee_percentage = (swap_opening_fees.proportional * 100) as f64 / 1_000_000_f64;
let min_fee_sat = swap_opening_fees.min_msat / 1_000;
info!("Send more than {min_deposit_sat} sats and up to {max_deposit_sat} sats to this address. \
A setup fee of {fee_percentage}% with a minimum of {min_fee_sat} sats will be applied \
for sending more than {inbound_liquidity_sat} sats. This address can only be used once.");
}

@JssDWt @dangeross is there a simpler way I'm missing? Or would you say this is good enough?

@dangeross
Copy link
Contributor

@JssDWt @dangeross is there a simpler way I'm missing? Or would you say this is good enough?

I think it looks pretty good. Didn't you update open_channel_fee() to always return fee_params? So you don't need to do the inbound_liquidity_msat + 1

@ok300
Copy link
Contributor Author

ok300 commented Feb 7, 2024

Didn't you update open_channel_fee() to always return fee_params? So you don't need to do the inbound_liquidity_msat + 1

Good point, yes that's true, thanks.

I'll have to wait until 0.2.16 though, as that change is not yet part of a release. The docs are now referencing 0.2.15, which is why I was coding against the older version.

@ok300 ok300 force-pushed the ok300-add-channel-open-fees-receive-payment branch from 7deb280 to e3f7eee Compare April 17, 2024 16:19
@ok300 ok300 changed the base branch from main to pre-release April 17, 2024 16:19
@ok300 ok300 marked this pull request as ready for review April 25, 2024 17:22
@ok300 ok300 requested a review from dangeross April 25, 2024 17:27
Copy link
Contributor

@dangeross dangeross left a comment

Choose a reason for hiding this comment

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

Looking good, just a couple of comments


func getFeeInfoBeforeInvoiceCreated() {
// ANCHOR: get-fee-info-before-receiving-payment
if nodeInfo, err := sdk.NodeInfo(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if nodeInfo, err := sdk.NodeInfo(); err != nil {
if nodeInfo, err := sdk.NodeInfo(); err == nil {

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe coding pattern-wise its better to do

if nodeInfo, err := sdk.NodeInfo(); err != nil {
        return err
}

Copy link
Contributor Author

@ok300 ok300 Apr 29, 2024

Choose a reason for hiding this comment

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

@JssDWt indeed looks better IMO, snippets will be more readable. How about I do that in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect.

Then @dangeross this is addressed for now in e084005

ok300 and others added 8 commits April 29, 2024 10:56
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
Co-authored-by: Ross Savage <551697+dangeross@users.noreply.github.com>
@ok300 ok300 requested a review from dangeross April 29, 2024 13:00
Copy link
Contributor

@dangeross dangeross left a comment

Choose a reason for hiding this comment

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

LGTM!

@ok300 ok300 merged commit d5d0be2 into pre-release Apr 30, 2024
68 of 69 checks passed
@ok300 ok300 deleted the ok300-add-channel-open-fees-receive-payment branch September 8, 2024 07:27
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.

Missing documentation on channel open fees when receiving a payment
3 participants