Skip to content

Different FeeRates for different Input Types in a Force Close Transaction#7535

Closed
ziggie1984 wants to merge 6 commits into
lightningnetwork:masterfrom
ziggie1984:sweep-rates-outputs-commitmenttx
Closed

Different FeeRates for different Input Types in a Force Close Transaction#7535
ziggie1984 wants to merge 6 commits into
lightningnetwork:masterfrom
ziggie1984:sweep-rates-outputs-commitmenttx

Conversation

@ziggie1984
Copy link
Copy Markdown
Collaborator

@ziggie1984 ziggie1984 commented Mar 23, 2023

This is far from ready because I did not write any tests, I just want to get early feedback if we wanna go in this direction rather then now spending time writing all the accompanied tests.

This solves #7026

Change Description

The Basic Idea:

When a channel is unilaterally closed lnd sweeps the different outputs in different stages back to the lnd wallet. This is done because all output are locked to WitnessScripts (P2WSH, or P2WKH in rare cases(to_remote output in a STATIC_REMOTE key channel) and we want to sweep them back to lnd's default wallet. This makes the recover of funds a smooth process because we can just do it by using the seed. If we would not sweep them into the default wallet, we would need to reconstruct the WitnessScript the outputs where locked to which can be cumbersome and definitely need the static_backup_file to get all the necessary information. For HTLCs on our Commitment Transaction they are also kept in so called second-level transactions (Bolt03) for these outputs we cannot recover the outputs without collaboration with our peer, and they are time sensitive in the first place so its a no-brainer to sweep those output of a commitment.

Because of the architecture of the lightning network transaction we can however introduce different feerate levels on different outputs of the commitment transaction. Some of the outputs as already mentioned above are highly time sensitive meaning that we need to sweep them by a specific locktime otherwise we run into the possibility of losing money. Others however are not and it makes sense to give a node runner a fine granular control of this different output sweep fee rates so that everybody can decide by himself how much he wanna pay.
This PR also introduces the ability to limit the fee rate of time sensitive outputs if the users wants to (opinions are welcome here)

In the following I describe the 4 different fee settings of the sweeper config this PR introduces to give everybody a detailed understanding what they are affecting.

max-anchor-feerate: This settings relates to the FeeBumping of the Commitment Transaction soley relevant for AnchorType Channels, as the name applies anyways^^. Lnd currently uses the anchor and tries to bump the fee if we have outgoing HTLCs or incoming HTLCs we know the preimage of and the initial Commitment Feerate is too low. Whats important, this setting relates to the effective feerate (taking the commitment tx into account). This Fee Rate is time sensitive so setting this value too low, could lead to your Commitment Tx not confirming in time, getting you into some risk scenarios where you could lose money. Though with this PR it is still possible to bump the fee of the Commitment transaction manually using lncli bumpfee.
(Current Confirmation Target in lnd: 1 block if any timeout passed for relevant HTLCs on the commitment, or 144 blocks)

max-firstlevel-htlcresolve-feerate: This setting relates to the firstlevel of sweeps when HTLC outputs are swept. There are some subtle differences here whether the commitment transaction is ours or was broadcasted by the peer. But both of them are time sensitive. We wanna make sure we have a feerate so the Output confirms just in time otherwise we risk losing money (in case the htlc was part of a forward not a payment). So it makes sense to set this fee rate very high or not touch it at all. Though for small htlc amounts it might be not worth it and the node runner wants to take the risk.
(Current Confirmation Target: 6 Blocks)

  • Our Commitment Tx is broadcasted: Then this setting will only take effect when the ChannelType is an Anchor Channel because only there we can attach an arbitrary amount of fees to the second-level Timeout/Success Tx. In case of a non-anchor channel, fees for the Timeout/Success transaction are the same as for the Commitment Transaction and are negotiated beforehand, we cannot change this feerate when the force close already happended.

  • Peer Commitment Tx is broadcasted: In this case the Timeout/Success Output is directly on the Commitment Tx (Bolt03) and we can sweep this transaction attaching an arbitrary feerate.

max-secondlevel-htlcresolve-feerate": This setting refers to second-level sweep transactions of HTLCs which were on our Commitment Transaction (we force closed the channel) and we had to broadcast the secondlevel timeout/success transaction. This sweep is not time sensitive it sweeps the CSV locked output back to the lnd default wallet. So makes sense to make this fee rate customizable. And Because lnd has the ability to spend 0-confirmation outputs, they are usable as soon as they are swept even at low feerates.
(Current Confimation Target: 6 Blocks)

max-commitmentresolve-feerate: This setting relates to the to_local output on our commitment transaction or the to_remote on the remote commitment transaction . They are not time sensitive but the user might want them to confirm faster because the mostly have a high amount compared to the HTLC amounts. (But one could generalize this setting with the max-secondlevel-htlcresolve-feerate which would leave the user with less confusion maybe).
(Current Confirmation Target: 6 Blocks)

Currently the feerate of the different sweeps are configured with constant confirmation targets, and this PR does not change them but capps them as soon as the bitcoin-core/btcd fee estimate would overshoot this amount. A different solution would be to introduce confirmation targets for all those output types rather than capping the fee rate.

Happy to start a discussion and hear your thoughts :)

Steps to Test

Steps for reviewers to follow to test the change.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

@GordianLN
Copy link
Copy Markdown
Contributor

This all goes above my own skill and knowledge level, yet my unworthy 2 cents say that, when there is no time sensitivity, it's a very welcome, and incredibly still missing, feature to have, setting your own general policy for miner fees in a FC environment.
So you're doing god's work, son.
Most node runners have low time preference anyway (and I'm still waiting for feedback to my "sweep FCs to cold wallet" feature request made quite a long time ago).
Being very risk averted, I wouldn't play with lowering the fees for time sensitive sweeps, but I'd still use as small a fee as possible for everything else, especially if there was a very easy way to speed up the reclaiming of funds sooner where the need arises at a later time.

Idea: being onchain fees not amount-dependent, have the "time preference" be set like in lightning, for example "I'm willing to pay up to 500ppm to get my funds confirmed on my wallet" and determine the fee based on the amount to sweep.

@feelancer21
Copy link
Copy Markdown
Contributor

Hey Ziggie,

great that someone is currently looking at reducing the cost of force closures. These costs are currently the biggest pain for many routing node operators.

Now here are my thoughts. I'm a bit skeptical that absolute limit is the way to go when confirmation is time sensitive. People may forget what they set, or not know what they are doing, or may not be at the node to bump. But imho in the cases of secondlevel-htlcresolve, commitmentresolve and anchor without HTLCs, limiting the fees is the way to go.

This leaves the cases of anchor with HTLCs and firstlevel-resolve. The goal in these cases should be to push the funds safely into the wallet without the user having to intervene manually, e.g. by bumps or parameter changes when the mempool is full again. But sweeping should not be as conservative as it is now. It only becomes critical if the transactions are not confirmed by the time the HTLC downstream is due. So my idea is a dynamic confirmation target that is updated by lnd with each block_height and successively shortens until it eventually arrives at 1.

The due date of the downstream HTLC is to be called expiry_downstream. The dynamic confirmation target shall be calculated by max(expiry_downstream - confirmation_buffer_secondlevel - current_block_height;1).
Confirmation_buffer_secondlevel specifies how many blocks before the downstream HTLC is due the transaction must be confirmed . As the block height increases, the target would then decrease and lnd would bump more and more aggressively. However, one has the chance to get a confirmation at more favorable fees than in the status quo.
The whole thing would also have a positive side effect. It could move users to set higher TimeLockDeltas, because they can reduce the FC costs.

So far my thoughts.

@ziggie1984
Copy link
Copy Markdown
Collaborator Author

ziggie1984 commented Mar 29, 2023

Now here are my thoughts. I'm a bit skeptical that absolute limit is the way to go when confirmation is time sensitive. People may forget what they set, or not know what they are doing, or may not be at the node to bump. But imho in the cases of secondlevel-htlcresolve, commitmentresolve and anchor without HTLCs, limiting the fees is the way to go.

I am in favour of your argumentation and also think that we should remove the possiblity to tamper with the time-senstive feerates and rather introduce something else, like for example your dynamic bumping approach. So I will rework this PR to only give the user the possiblity for the non-timesenstive feerates and remove everything else.

@feelancer21
Copy link
Copy Markdown
Contributor

Good idea to focus on the non-timesensitive cases first. That would be already a win for node runners.

@ziggie1984
Copy link
Copy Markdown
Collaborator Author

ziggie1984 commented Apr 11, 2023

I removed all the time sensitive fee settings and just went for the non critical one (max-non-time-sensitive-feerate). As soon we have agreement that we want to go with this change, I will write some additional itests for this new logic. Currently only verified manually with polar and it works fine.

@ziggie1984 ziggie1984 force-pushed the sweep-rates-outputs-commitmenttx branch 3 times, most recently from 65aafa9 to 3ba2aa3 Compare April 13, 2023 11:31
@ziggie1984 ziggie1984 changed the base branch from master to 0-16-1-staging April 13, 2023 11:34
@ziggie1984
Copy link
Copy Markdown
Collaborator Author

ziggie1984 commented Apr 13, 2023

Looking for an approach ACK then I will add itests and potentially unit-tests coverage

@ziggie1984 ziggie1984 marked this pull request as ready for review April 13, 2023 11:35
Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

Very cool pr! This overlaps with #7549 a bit, or maybe 7549 depends on this change. I'm also looking into the sweeper recently, and think specifying max fee or max fee ratio is the way to go. Currently, when an input is swept, it's grouped with other inputs to make the final sweeping tx. The grouping uses fee buckets, which put inputs with similar fee rate into one tx, and the final fee rate is the mean of the group.
This means when we set a max fee rate, it can only be approximated. On the other hand, if we could set max fee ratio to specify what portion of the input amount can be used as fee, we can have a finer control of the cost. This also enables the fee bumper to control the fee bumping process accurately.
Meanwhile I think we should have a config for each input type with a default value. This way the user can set the max fee for a specifc input type at will.

@feelancer21
Copy link
Copy Markdown
Contributor

On the other hand, if we could set max fee ratio to specify what portion of the input amount can be used as fee, we can have a finer control of the cost.

How would you denominate the ratio? In ppm of the input amount?

@ziggie1984
Copy link
Copy Markdown
Collaborator Author

ziggie1984 commented Apr 14, 2023

Thanks for you feedback @yyforyongyu . I haven't thought about the average feerate of a bucket but thats right feerate can really only be approximated in case we have more transactions in one bucket. So totally interested in the ratio approach.

With what I am a bit struggling is, how would we group different inputs with different ratios.

Lets say we have a 100k input (200vbyte) and 100k (100vbyte) now both have a ratio of 1% for both. Leading maxfee for both inputs of 1000sats each. Leading to a max feerate for input 1 of 5sat/vbyte and of input 2 of 10sat/vbyte. So we still group them in buckets of feerate, leading to both inputs being in the 10sats/vbyte bucket. Now when estimating the feerate for inputs in the same bucket (the complete transaction) we use 2000 sats => leading to 2000/300 => 6.666 bytes.

But now the bigger the cluster becomes the lower the complete feerate is pressed down. So I think we also need to limit the amount of inputs into one transactions then ?

Or is my thinking in regards of ratio and the grouping off?

@RocketNodeLN
Copy link
Copy Markdown

I was able to run this PR live on my node to sweep a matured FC. Used sweeper.max-non-time-sensitive-feerate=2 in my config file. For reference, bitcoin-cli estimatesmartfee 6 'ECONOMICAL' was returning 20+ sats/vB at the time. Here are the log file entries for SWPR while running this PR.

2023-04-14 20:56:20.632 [INF] SWPR: Sweeper starting
2023-04-14 20:56:25.087 [INF] SWPR: Sweep request received: out_point=d051f6af95842ebb2ac6c2a7aa4fdd1ca105c033dc1b9ffb6c0333bd0a843ea4:2, witness_type=CommitmentTimeLock, relative_time_lock=480, absolute_time_lock=0, amount=0.00055439 BTC, params=(fee=6 blocks, force=false, exclusive_group=<nil>)
2023-04-14 20:56:55.047 [INF] SWPR: Candidate sweep set of size=1 (+0 wallet inputs), has yield=0.00055196 BTC, weight=486
2023-04-14 20:56:55.055 [INF] SWPR: Creating sweep transaction caf44a047fd583e432bf4b9ddbcdbe00f0b0209c44ba300b2a6369e5298ee14f for 1 inputs (d051f6af95842ebb2ac6c2a7aa4fdd1ca105c033dc1b9ffb6c0333bd0a843ea4:2 (CommitmentTimeLock)) using 500 sat/kw, tx_weight=534, tx_fee=0.00000267 BTC, parents_count=0, parents_fee=0 BTC, parents_weight=0
2023-04-14 20:57:34.827 [INF] SWPR: Sweeper shutting down

The sweep was successfully broadcasted at ~2 sats/vB. The only other suggestion for this PR is to update the logger to include the feerate parameter replacing the default fee=6 blocks.

@ziggie1984
Copy link
Copy Markdown
Collaborator Author

Thanks for testing @RocketNodeLN, lets see where we are heading in regards of fee-ratio or fee-camp and then bring this into lnd :)

@yyforyongyu
Copy link
Copy Markdown
Member

How would you denominate the ratio? In ppm of the input amount?

I think to make it simple we can use percentage.

With what I am a bit struggling is, how would we group different inputs with different ratios.

That's a very good question and I'm still thinking about it. The idea of having a max fee is to cap the cost, but not spend it all at once. So ideally we'd start with a much lower fee rate, and gradually increase it when the deadline is approaching. This means different inputs will have different deadlines, and we will group them by deadlines, which is equivalent to grouping by fee rates since we estimate fee rates based on the conf target(deadline - current height).

On the other hand, the general assumption is the higher the fee rate, the more quickly it gets confirmed. But I think it should be the higher the fee, the more likely the miners prefer it. Need to double check tho.

Overall, the direction is to make the sweeper only care less about the fee rate, and leave it to the fee bumper to instruct new fee rates to be used. This also means we gonna rely less on bitcoind's fee estimator and calculate the fee rates based on configuration/specific needs.

@feelancer21
Copy link
Copy Markdown
Contributor

How would you denominate the ratio? In ppm of the input amount?

I think to make it simple we can use percentage.

I think a configuration in percantage has no use at the end. Usually the sweeps we are talking are about some million sats. A config in percentage would lead to fee caps and steps of ten thousands of sats.
Imho we should configure it in ppm. This shouldn't increase complexity because it is only a question of the denominator.

@ziggie1984 ziggie1984 changed the base branch from 0-16-1-staging to master April 23, 2023 20:50
@lightninglabs-deploy
Copy link
Copy Markdown
Collaborator

@ziggie1984, remember to re-request review from reviewers when ready

@ziggie1984
Copy link
Copy Markdown
Collaborator Author

!lightninglabs-deploy mute

@ziggie1984 ziggie1984 force-pushed the sweep-rates-outputs-commitmenttx branch 2 times, most recently from dcc54d9 to bd00595 Compare May 2, 2023 12:43
@RocketNodeLN
Copy link
Copy Markdown

2nd success with the most recent PR. By combining with sweeper.batchwindowduration, I was able to get 2 FC as well as two expired htlc from one of the FC into 1 utxo at the configured max feerate.

@ziggie1984 ziggie1984 force-pushed the sweep-rates-outputs-commitmenttx branch from bd00595 to f320924 Compare May 7, 2023 07:26
@ziggie1984 ziggie1984 force-pushed the sweep-rates-outputs-commitmenttx branch 2 times, most recently from e30f7c0 to 12d8d35 Compare May 14, 2023 21:11
@ziggie1984 ziggie1984 force-pushed the sweep-rates-outputs-commitmenttx branch 6 times, most recently from 85263ab to f8c7f1d Compare October 31, 2023 13:20
A new fee rate option is introduced which allows for more fine
granular control of different sweep transactions in regards of
unilateral channel closures which are non time sensitive.
For different input types which are not time sensitive a fee
limit is introduced.
@ziggie1984
Copy link
Copy Markdown
Collaborator Author

Obsolete with the new sweeper subsystem.

@ziggie1984 ziggie1984 closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants