lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.64k stars 2.07k forks source link

[feature]: Include ParentFee when doing a CPFP using bumpFee RPC #7652

Open ziggie1984 opened 1 year ago

ziggie1984 commented 1 year ago

Currently when we try to bump the fee of a transaction via CPFP lnd's sweeper engine it is not accounting for the parent unconfirmed transaction:

https://github.com/lightningnetwork/lnd/blob/master/lnrpc/walletrpc/walletkit_server.go#L926

I was trying to give it a shot while implementing the kweight feature (#7454) but I am not quite sure how I can fetch the inputs of the transaction here:

utxo, err := w.cfg.Wallet.FetchInputInfo(op)
    if err != nil {
        return nil, err
    }

So we have the full transaction saved in the utxo.PrevTx, but to get the fee, I need to basically do Outputs - Inputs, but I do not know how to fetch the Prevout Amounts being in the walletkit_server ? Any ideas how to do that ?

ziggie1984 commented 1 year ago

This feature would be useful in high-fee-scenarios, and a user would not have to do all this Effective FeeCalculation but just supply the feerate for the bumpfee, and the right effective fee would be calculated.

positiveblue commented 1 year ago

Agree. This is one of the project ideas for the summer of bitcoin 2023

ziggie1984 commented 1 year ago

Ohh I see it says hard, aaargh nice challenge.

ziggie1984 commented 1 year ago

Not only should we take the parent into account when sweeping but also when opening channels with zeroconf funds.

ziggie1984 commented 1 year ago

I think this problem is pretty easy to solve for the bitcoind backend, before using a zeroconf output, query the mempool for its size via the rpc : bitcoin-cli getmempoolentry there all the important size caluclation is done.

kaloudis commented 1 year ago

Would love to see this. A lot of Zeus users struggle with calculating the effective fee rate on their own

Chinwendu20 commented 7 months ago

Hello @ziggie1984 is this issue still active, I can see you made changes to the bumpfee function

Chinwendu20 commented 7 months ago

Hello @ziggie1984 is this issue still active, I can see you made changes to the bumpfee function

Nevermind, I can see that it is for a different issue

yyforyongyu commented 7 months ago

I think this will be fixed by the new sweeper?

kaloudis commented 7 months ago

Seems like parents are being accounting for now, at least when bumping via cli

Roasbeef commented 7 months ago

I think this will be fixed by the new sweeper?

Assuming it is aware of the entire input package.

ziggie1984 commented 6 months ago

Seems like parents are being accounting for now, at least when bumping via cli

I think you talk about the bumpclosefee rpc, which takes the parent commitment fee into account but for the normal lncli wallet bumpfee we still do not take the parent feerate into account when doing a CPFP. @kaloudis Or which RPC cmd did you try ?

I think this will be fixed by the new sweeper?

So I looked at the final version of https://github.com/lightningnetwork/lnd/pull/8514, it I don't think this is covered. For the CPFP case we still don't ask our backend to provide us with the detailed ancestor size.

Roasbeef commented 5 months ago

For the CPFP case we still don't ask our backend to provide us with the detailed ancestor size.

Correct, we don't try to walk the transaction graph to get the fee of that unconfirmed component.

yyforyongyu commented 5 months ago

I think this is no longer relevant given the new behavior of our sweeper - when you do CPFP, a wallet utxo is grabbed and a sweeping request will be made. Then, inside the sweeper, the fee bumper will make an initial broadcast which guarantees the first attempt will succeed (or fail if the budget cannot cover the fees).

On the other hand, you can specify the max fees via Budget field, which gives you more control over the fees to be paid.

yyforyongyu commented 5 months ago

actually what @ziggie1984 is suggesting is, when creating the fee bumping tx, we should make sure the new tx always pays a higher fee, otherwise this new tx has no utility, which is not covered by the sweeper series.