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

blip-0034: the recommended_feerates message #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t-bast
Copy link
Contributor

@t-bast t-bast commented Jul 2, 2024

Many lightning messages include feerates for on-chain transactions. Nodes receiving those messages are supposed to reject them if they are considered too low or too high. However, there is no mechanism to let peers know which feerates we currently find acceptable, which leads to frequent failures, especially when the mempool feerates fluctuate quickly.

We introduce an optional message to tell our peers the feerates we'd like to use.

Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

I mean, sure, but I'm not really sure how I'd go about implementing this. Pre-Bitcoin Core 28 if I think the fee rate needed for confirmation/mempool acceptance is higher than the channel currently is, I need to update. Whether my peer said they think the fee rates are higher than I think or not I'm probably still gonna use the one I think I need? I guess a peer could tell me they require a fee rate a few % higher and I'd accept that too but the peer should at least accept an increase even if it's not a fee rate they think they need anyway?

More generally, the fix really should be Bitcoin Core 28, anchor channels, and no feerate enforcement.

2. data:
* [`chain_hash`:`chain_hash`]
* [`u32`:`funding_feerate_per_kw`]
* [`u32`:`commitment_feerate_per_kw`]
Copy link
Contributor

Choose a reason for hiding this comment

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

As a recipient I don't really care what value you would set, I care what value(s) you would reject.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is useful when combined with blip 33, where the non-opener is paying the commit fees: they can tell the opener what feerate they will accept in open_channel2.

@t-bast
Copy link
Contributor Author

t-bast commented Jul 3, 2024

More generally, the fix really should be Bitcoin Core 28, anchor channels, and no feerate enforcement.

I don't get what you mean here...sure, for the commitment feerate, this isn't very important once we have anchor channels. But for opening channels and creating splice transactions (funding feerate), you will have a specific confirmation target (especially when combined with 0-conf, which makes RBF impossible).

In the LSP / mobile wallet case, the mobile wallet generally doesn't have access to its own Bitcoin Core instance, and doesn't know what confirmation target the LSP is using. So the LSP just tells the mobile wallet what feerate it will use when it initiates channel-open or splice, and the mobile wallet uses the same values when it is the initiator (e.g. swap-in or on-the-fly funding), which ensures that it will generally be accepted by the LSP.

EDIT: reading your comment again, this probably comes from the fact that I mentioned update_fee? And it's true that for update_fee, it probably doesn't make much sense, because you wouldn't really know how to handle that information. I'm currently using this only for open_channel2 and splice_init, I probably should remove the mention of update_fee and clarify that?

@TheBlueMatt
Copy link
Contributor

Ah, okay, I understood this to be about update_fee, indeed, rather than channel open and splicing. Indeed, IMO we should drop the reference to update_fee. However, I still don't think this is as useful as it should be - you say "nodes receiving those messages are supposed to reject them if they are considered too low or too high" but the spec here doesn't provide the range which a peer will accept. Why not send the min/max you'll accept explicitly here?

@t-bast
Copy link
Contributor Author

t-bast commented Jul 8, 2024

Why not send the min/max you'll accept explicitly here?

I hesitated, mostly because the way I'm currently using this, it's not a real guarantee that the recommended feerates will be accepted (depending on how long you wait before initiating an on-chain funding based on those feerates). I felt that having a min/max may give a false assurance that it will be accepted so I decided to stick with the simplest one.

But you're right, it's probably useful to change this to min/max/preferred and keep accepting them until the next recommended_feerates is issued, this would be more thorough. I'll change my implementation and will update this bLIP.

@t-bast
Copy link
Contributor Author

t-bast commented Jul 8, 2024

Done and rebased, I hope it's clearer now!

Many lightning messages include feerates for on-chain transactions.
Nodes receiving those messages are supposed to reject them if they
are considered too low or too high. However, there is no mechanism
to let peers know which feerates we currently find acceptable, which
leads to frequent failures, especially when the mempool feerates
fluctuate quickly.

We introduce an optional message to advertise to our peers the feerates
we'd like to use when funding new or existing channels.
Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Without having implemented any of this, makes sense to me.


Many lightning messages include feerates for on-chain transactions. Nodes
receiving those messages are supposed to reject them if they are considered
too low or too high. However, there is no mechanism to let peers know which
Copy link
Contributor

Choose a reason for hiding this comment

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

. Nodes receiving those messages are supposed to reject them if they are considered
too low or too high.

We currently don't have any way to reject the fee updates though, other than force closing, which then locks you into a potentially undesirable fee (pre and post anchors, before package relay).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but what does that have to do with this proposal? This introduction simply states that the BOLTs tell you to reject messages with offending feerates (where we don't really care what "reject" is) but doesn't give you tools to avoid creating a message with an offending feerate, which motivates this bLIP.

1. type: 39409 (`recommended_feerates`)
2. data:
* [`chain_hash`:`chain_hash`]
* [`u32`:`funding_feerate_per_kw`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be a min+max similar to the recommended_feerates_tlvs field below?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I wonder if it's useful for the desired use case if this is always communicating as a mapping from conf target to the fee rate. Otherwise, it's unclear what type of confirmation delay the sender is comfortable with when they send their funding_feerate_per_kw field.

Copy link
Contributor Author

@t-bast t-bast Jul 15, 2024

Choose a reason for hiding this comment

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

Should this also be a min+max similar to the recommended_feerates_tlvs field below?

I don't understand what you mean...there is a min/max range defined (funding_feerate_range).

Also I wonder if it's useful for the desired use case if this is always communicating as a mapping from conf target to the fee rate.

Good question. I don't think it's useful here, since the only thing you want to know is whether your peer will accept the feerate you set, and BOLT messages don't use a conf target but rather feerates.

But it could be informative to have an optional TLV containing the confirmation target the sending node is using, I could add that if you think that's really useful (I'm not sure it is). Or we could add it later if we find it useful.

blip-0034.md Outdated

The receiver:

* SHOULD not send `open_channel`, `open_channel2`, `splice_init`, `tx_init_rbf`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should add update_fee as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 4cabcbf: the commit message explains why update_fee is a bit more complex, because it can involve funds safety.

It's a bit murky what to do for `update_fee`: if you think you have to
update the feerate outside of the range your peer allows, you should
still do it, because you're the one paying for it. Your peer is free
to force-close if they're unhappy with that.
@dzdidi
Copy link

dzdidi commented Jul 22, 2024

FYI, @vincenzopalazzo

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.

4 participants