BOLT 07: delay announcement_signatures by max(6, min_depth) confs

[cfromknecht]

This PR modifies the sending requirements of announcement_signatures to wait max(6, min_depth) blocks before sending. The current reading says to wait for the funding_locked exchange and 6 confirmations, so this modification doesn't change the height at which the announcement_signatures should first be sent, but removes the stricter ordering requirements wrt funding_locked and (indirectly) channel_reestablish.

Note that the current wording is conflicting, as BOLT 2 say that BOLT 7 message retransmission is independent of BOLT 2 retransmission, though the current announcement_signatures retransmission has ordering requirements wrt to funding_locked.

The proposed fix here is to relax the ordering constraints on announcement_signatures, while still ensuring that announcement_signatures aren't transmitted grossly before the funding_locked message (e.g., the case where min_depth >> 6). This change is aimed at making the BOLTs more isolated, and reduce the coordination requirements between gossip and peer messages.

This change makes it easier for implementations to isolate critical forwarding functionality from non-critical processing of gossip traffic via isolated daemons or subsystems.

See prior discussion on #620

[renepickhardt]

As far as I understand the process for channel opening this PR is not specific enough. Only the fundee sets a min_depth in their channel_accept message. When suggesting to wait for min(6,min_depth) it is unclear whose min_depth value to use.

Let's say A funds a channel with min=15 to B who has mdin_depth=9. Now B would send the announcement message at 9? It is not aware of A's 15. What about A? Do they use their own min_depth or the one from B? I assume A's.

On a side note: while I understand your request one thing bothers me. If for some reason the funding_locked did not arrive in our non reliable communication layer we might end up announcing a public channel on gossip for which we don't have the next per_commit_point which makes the channel unusable for the network. We might have to add a rule about that with the update_add_htlc messages.

[cfromknecht]

Thanks for the feedback @renepickhardt

As far as I understand the process for channel opening this PR is not specific enough. Only the fundee sets a min_depth in their channel_accept message. When suggesting to wait for min(6,min_depth) it is unclear whose min_depth value to use.

Let's say A funds a channel with min=15 to B who has mdin_depth=9. Now B would send the announcement message at 9? It is not aware of A's 15. What about A? Do they use their own min_depth or the one from B? I assume A's.

I can see how that could be confusing, I've updated the wording to be more specific. I was under the assumption that since the both parties use the same fundee-proposed minimum_depth that this would carry over.

On a side note: while I understand your request one thing bothers me. If for some reason the funding_locked did not arrive in our non reliable communication layer we might end up announcing a public channel on gossip for which we don't have the next per_commit_point which makes the channel unusable for the network.

A conservative node may still choose to send its announcement_signatures only after receiving the remote party's funding_locked, which would yield the same thing as the current BOLT 7 wording. Given the propagation delay of gossip traffic, I don't think it'd be much of an issue if they are loosely coupled, even if they were spaced out by a few seconds. If the transport between two nodes is so unreliable that they can send one but not the other, they're going to have trouble being a useful channel.

I'm more concerned with the case where a node maliciously withholds funding_locked after receiving announcement_signatures, but as discussed in the other thread, this is really no different from the a malicious peer becoming unresponsive even if funding_locked had been exchanged.

We might have to add a rule about that with the update_add_htlc messages.

What rule do you have in mind?

[TheBlueMatt]

This effectively reverts #495 without (IMO) sufficient justification. Practically, #495 was important as it allows a node to avoid storing their peers' announcement_signatures prior to announcement (which should wait on having sent and received funding_locked). If you want them to be separate from channel_reestablish, I don't have a huge disagreement there (though it really doesn't seem right - announcement_signatures really makes little sense as a BOLT 7 message - its not about gossip, its about peers exchanging information which they can use to gossip their own channels), but as-is this breaks real things.

[rustyrussell]

I disagree with this.

If you said you want to wait until 100 confirms before accepting my channel, it makes no sense for me to send an announcement signature (which depends on the short-channel-id) before that, as you consider it still indeterminate.

[cfromknecht]

Sorry, went camping over the holidays and just getting back to this!

@TheBlueMatt Thanks for pointing me to #495, I wasn't aware of that merge.

Practically, #495 was important as it allows a node to avoid storing their peers' announcement_signatures prior to announcement (which should wait on having sent and received funding_locked).

IMO not having to temporarily store 168 bytes in memory is a pretty negligible win in the grand scheme of things. Personally I place a much higher importance on being able to properly isolate critical forwarding actions from non-critical gossip actions, and overall keeping the BOLTs as separate as possible.

but as-is this breaks real things.

I don't think it does?

From the sending side it's backwards compatible, if you wait for funding_locked then you've inherently satisfied the requirement.

From the receiving side, it's a little less clear, but only because it is already broken in the wild due to different interpretations of the requirements. Currently lnd will transmit after 6 confs regardless, so waiting min(6, minimum_depth) can only improve the situation AFAICT. Reordering of these messages is already being done in the wild, so I don't see how it can make things worse than the status quo...

I disagree with this.

If you said you want to wait until 100 confirms before accepting my channel, it makes no sense for me to send an announcement signature (which depends on the short-channel-id) before that, as you consider it still indeterminate.

I'm confused, which part do you disagree with? If both sides wait until max(6, minimum_depth) then by definition it's no longer indeterminate by either party.

Regardless of whether this change is accepted (which seems unlikely at this point 😂), @rustyrussell the main thing I want to ask you is if lnd were to add this extra delay, would it improve interop with CL? Is the primary issue you're seeing that lnd sends way too early (several blocks) or just jitter when reconnecting after minimum_depth has already been reached? If it's the former, I'm happy to make moves to get this patch out in the near term at least an initial stop gap.

Thanks for the replies!

[TheBlueMatt]

I don't think anyone is concerned with the memory cost of storing a few signatures, more about the logic cost of tracking them and digging them out when necessary instead of just converting them to announcements and relaying immediately. As to your point about wanting that logic separate from the channel state tracking, sure, but I'm kinda confused how this has any impact on it? Either way you're generating them and sending them at some block height based on data about the channel. If you want to expose that to your routing database tracker and do it all there, why not just expose the min_confs bit?

[lightning-developer]

I believe this proposal is outdated and the PR can be closed.

I looked at the major implementations and it seems as if everyone currently just waits for 6 confirmations before sending announcement signatures.

It would be nice if folks from implementations could confirm this as I might have overlooked something in the various code bases.

c-lightning

https://github.com/ElementsProject/lightning/blob/35bec51a97c8060602df82b03c74b59e90e870cb/common/gossip_constants.h#L39
sets ANNOUNCE_MIN_DEPTHto a constant value 6 and in https://github.com/ElementsProject/lightning/blob/d22fd599973175e3d1a484a7aa2efd9916116bc5/channeld/channeld.c#L3288 it is checked if the depth is reached and in https://github.com/ElementsProject/lightning/blob/d22fd599973175e3d1a484a7aa2efd9916116bc5/channeld/channeld.c#L614 the critical if tests if a node may send the announcement signature.

Thus c-lightning has still the old way of requiring that funding depth has to be at least 6 confirmations.

eclair

https://github.com/ACINQ/eclair/blob/3003289328c091527f248b118e9280657417481d/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala#L80
seta val ANNOUNCEMENTS_MINCONF = 6

Then in https://github.com/ACINQ/eclair/blob/3003289328c091527f248b118e9280657417481d/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala#L668

sends an event if the depth is reached. This event is also registered in channel reestablishment in the same file.

Thus eclair also has the old way of requiring that funding depth has to be at least 6 confirmations.

electrum

https://github.com/spesmilo/electrum/blob/56b03e2e8d6923c4d396c1272d3fafed57919a8f/electrum/lnpeer.py#L1109 indicates also the check is also for depth >= 6 however the next statement is a return statement so it seems as if electrum does not announce channels at all. I am a bit confused and will open an issue there and ask.

LDK

https://github.com/lightningdevkit/rust-lightning/blob/61518f97212fd6a4c40cdd62a3ee5ab7a15d1971/lightning/src/ln/channelmanager.rs#L3672 says in this comment

We only send a channel_update in the case where we are just now sending a
funding_locked and the channel is in a usable state. Further, we rely on the
normal announcement_signatures process to send a channel_update for public
channels

also https://github.com/lightningdevkit/rust-lightning/blob/e9774aeb2eaf27dccd2d3d4422b65040995bdc9b/lightning/src/util/config.rs#L55 implies they set minimum depth fixed to 6

LND

https://github.com/lightningnetwork/lnd/blob/6c66338b143fbac5941cba52d4244499a3804a8e/funding/manager.go#L2661

defines the function annAfterSixConfs which if the channel is to be public registers a Notification with the ChainNotifier which is to be send after 6 confirmations are reached.

Tagging @Roasbeef as I am not sure if @cfromknecht will follow this.

[TheBlueMatt]

It seems @cfromknecht no longer works on lightning. Without a champion to defend this PR, closing.