[bug]: anchor sweep fee rate not adjusted in state StateCommitmentBroadcasted

[C-Otto]

Background

My node force-closed an anchor channel while outgoing HTLCs (which were initiated by my node, i.e. not due to forwarding) were pending. The sweeper was instructed accordingly, picking a very low rate as the deadline (refundTimeout) was well in the future at the time of closing the channel:

calculated deadline: 643, using deadlineMinHeight=832898, heightHint=832255
pre-confirmation sweep of anchor of local commit tx xxx:0, force=false
Sweep request received: out_point=xxx:0, witness_type=CommitmentAnchor, relative_time_lock=0, absolute_time_lock=0, amount=0.00000330 BTC, parent=(fee=0.00006491 BTC, weight=2836), params=(fee=643 blocks, force=false, exclusive_group=xxx)

While the channel was in state StateCommitmentBroadcasted (the transaction did not confirm due to fee pressure in the mempool), lnd did NOT adjust the fee rate.

Only after the deadline passed and after I restarted lnd, the sweeper was instructed to use a more aggressive fee rate (deadline 1, force=true):

deadline is passed with deadlineMinHeight=832898, heightHint=833085
calculated deadline: 1, using deadlineMinHeight=832898, heightHint=833085
pre-confirmation sweep of anchor of local commit tx xxx:0, force=true
Sweep request received: out_point=xxx:0, witness_type=CommitmentAnchor, relative_time_lock=0, absolute_time_lock=0, amount=0.00000330 BTC, parent=(fee=0.00006491 BTC, weight=2836), params=(fee=1 blocks, force=true, exclusive_group=xxx)

Your environment

• version of lnd: v0.17.4-beta
• which operating system (uname -a on *Nix): Linux server 6.1.0-11-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.38-4 (2023-08-08) x86_64 GNU/Linux
• version of btcd, bitcoind, or other backend: v25.1.0

Steps to reproduce

• have outgoing HTLCs (in this case: originated from my node, i.e. not forwarded)
• have peer respond with some weird message (a bug on their end), causing the local node to close the channel
• wait

Expected behaviour

The fee rate of the anchor CPFP transaction should gradually increase, so that it confirms before the first HTLC times out.

Actual behaviour

The fee rate was not adjusted. Based on my logs, the advanceState function was not invoked (which logs attempting state step with ..., see https://github.com/lightningnetwork/lnd/blob/v0.17.4-beta/contractcourt/channel_arbitrator.go#L1539).

To me it seems the issue is in channelAttendant, which skips channels in state StateCommitmentBroadcasted so that the (correct?) fee update code triggered in advanceState is not even invoked:

lnd/contractcourt/channel_arbitrator.go

Lines 2688 to 2693 in 8bba792

	// If we're not in the default state, then we can
	// ignore this signal as we're waiting for contract
	// resolution.
	if c.state != StateDefault {
	continue
	}

[morehouse]

I think your analysis of the problem is exactly correct. I've also noticed several places in sweeper/contractcourt where fees are not properly bumped as deadlines approach.

A simple fix should be quickly mergeable for this case, but I think the ongoing work on #8042 is important to address these cases more generally.

[C-Otto]

I think this is another issue:

lnd/contractcourt/channel_arbitrator.go

Lines 1316 to 1324 in 8bba792

	// Check the deadline against the default value. If it's less
	// than the default value of 144, it means there is a deadline
	// and we will perform a CPFP for this commitment tx.
	if deadline < anchorSweepConfTarget {
	// Signal that this is a force sweep, so that the
	// anchor will be swept even if it isn't economical
	// purely based on the anchor value.
	force = true
	}

What if the deadline is more than the default (currently set to 144 blocks), as it happened to me? What if the deadline is identical to the default, for whatever reason? I think it's better to return "deadline_is_default" (or the negation of that) in addition to the computed deadline to determine the use of "force".

[morehouse]

@C-Otto I think as long as sweepAnchors is called every block, force will flip to true once the deadline is approaching, so no changes there are needed.

But the force flag is pretty hacky and @yyforyongyu is trying to replace it with a better interface soon, which should completely eliminate that piece of code. See #8514,