[bug]: UpdateChannelPolicy changes fee rates for gossip but not for local channel in some cases

[LNBIG-COM]

Your environment

• version of lnd v0.17.3

Actual behaviour

I definitely noticed this situation: at some point in time (T), I change the fee rate and base fee (from N>0 to N=0) for the channel via UpdateChannelPolicy through gRPC. After a long time (for example, after a few hours: T + H). HTLC payments start coming from the network through the channel, where the senders have clearly set a zero price for forwarding the payment, but my node swears that the fee is zero, but it should be N (that is, by all indications, the LND for the channel has inside an old non-zero fee):

2024-01-17 05:35:07.124 [WRN] HSWC: ChannelLink(8701ee9262f8483c398d9418385571a8e098aa439f157f5fbb4e3a9e683737c4:1): outgoing htlc(dbf28c05b3ca2bdXXXXXXXXXXXXXX08b4a21) has insufficient fee: expected 321003, got 0
...
2024-01-17 05:35:10.207 [WRN] HSWC: ChannelLink(8701ee9262f8483c398d9418385571a8e098aa439f157f5fbb4e3a9e683737c4:1): outgoing htlc(760b2856bXXXXXXXXXXXXXXXXX79de3a1019) has insufficient fee: expected 161003, got 0
...

And so on.

Looking through the sources of the LND, I get the feeling that LND performs two steps with the UpdateChannelPolicy command - sends new fees to the network and then updates the policy locally, but (!) only for active links. That is, if I understand correctly - for those when the other side of the channel is online and when there are no problems.

If this is the case, then I can assume that when I called the UpdateChannelPolicy gRPC command at time T, the ChannelLink was inactive for some reason. At that moment, new policies for the channel went online through gossip. But locally, the policy has not been changed. Then, apparently, after some time, the link became working again, but with the old policy. In which way the policy is then updated locally in this case, I can guess that only through gossip, when LND receives its own policy for the channel through gossip from third peers. By all indications, it could have taken me many, many hours, if at all, to receive new policy data from gossip. All this time, such forwarding payments are "failed" as with an incorrect fee.

If this is the case, then from my point of view it is a bad implementation. It is logical to change the policy locally immediately, for an active or not channel, as soon as the UpdateChannelPolicy API command is called. Because as soon as the channel becomes active again, payments begin to go through it, but they all fail, but they all fail (This is noticeable on a loaded LND and only in cases when the fee changes downward).

Looking ahead, I observed that the sender does not receive a response error (InsufficientFeeUpdate error) onion in this case and the payment may hang for a long time as unfulfilled (I log all events from nodes through Router/SubscribeHtlcEvents and I did not find a return packet for a specific case with such an error about the fee error to the sender of the payment).

[Roasbeef]

Correct that we only update in-memory fee for active links. We do this as an inactive link means that nothing is actually in the switch, so can't be used for forwarding. When a peer connects again, we should load the new policy from disk for that link:

lnd/peer/brontide.go

Lines 922 to 965 in 54329a3

	// Before we register this new link with the HTLC Switch, we'll
	// need to fetch its current link-layer forwarding policy from
	// the database.
	graph := p.cfg.ChannelGraph
	info, p1, p2, err := graph.FetchChannelEdgesByOutpoint(chanPoint)
	if err != nil && err != channeldb.ErrEdgeNotFound {
	return nil, err
	}
	// We'll filter out our policy from the directional channel
	// edges based whom the edge connects to. If it doesn't connect
	// to us, then we know that we were the one that advertised the
	// policy.
	//
	// TODO(roasbeef): can add helper method to get policy for
	// particular channel.
	var selfPolicy *models.ChannelEdgePolicy
	if info != nil && bytes.Equal(info.NodeKey1Bytes[:],
	p.cfg.ServerPubKey[:]) {
	selfPolicy = p1
	} else {
	selfPolicy = p2
	}
	// If we don't yet have an advertised routing policy, then
	// we'll use the current default, otherwise we'll translate the
	// routing policy into a forwarding policy.
	var forwardingPolicy *models.ForwardingPolicy
	if selfPolicy != nil {
	forwardingPolicy = &models.ForwardingPolicy{
	MinHTLCOut: selfPolicy.MinHTLC,
	MaxHTLC: selfPolicy.MaxHTLC,
	BaseFee: selfPolicy.FeeBaseMSat,
	FeeRate: selfPolicy.FeeProportionalMillionths,
	TimeLockDelta: uint32(selfPolicy.TimeLockDelta),
	}
	} else {
	p.log.Warnf("Unable to find our forwarding policy "+
	"for channel %v, using default values",
	chanPoint)
	forwardingPolicy = &p.cfg.RoutingPolicy
	}

We have some itests that assert routine update flows in this area, but can look at them more closely to see if we can spot any gaps.

[ellemouton]

This might happen due to a race condition:

PropagateChanPolicyUpdate spins the processing and persistence of the update into a goroutine

So I think there is a chance that the link gets activated and loaded before the update gets persisted