Bolt7: add channel update flag for scid alias

[t-bast]

Since #910, nodes are allowed to use aliases instead of real scids. It would be quite helpful to be explicit about it and set a flag in channel_update when an alias is used instead of a real scid for a few reasons:

• the receiving peer knows that this channel_update is meant to be kept private and can internally store it separately from "real" channel updates without requiring a lookup into an internal mapping table
• the sender knows whether they're using an alias or not, so it's trivial to set that bit

We also make the htlc_maximum_msat field mandatory: every node on the network currently sets it, so we can simplify message parsing. This makes this PR an alternative to #996

@cdecker @TheBlueMatt @Crypt-iQ is that message flag helpful for your implementation's router logic?

DAMN I could have gotten PR number 1000, I'm always off-by-one (unless there was a PR number 0)

[TheBlueMatt]

the receiving peer knows that they shouldn't wait for a corresponding channel_announcement to be validated against the blockchain

What does this mean? You should never "wait" for a channel_announcement - if you receive a channel_update first you should drop it on the floor and ignore it (unless its your own channel/direct peer).

the receiving peer knows that this channel_update is meant to be kept private and can internally store it separately from "real" channel updates

If you have a channel_announcement first and the channel_update matches the announcement and what's on-chain, then you're done. Not sure why this matters.

[Crypt-iQ]

I don't think this helps the lnd implementation any since we know which SCID aliases we are using for a given channel

[t-bast]

What does this mean? You should never "wait" for a channel_announcement - if you receive a channel_update first you should drop it on the floor and ignore it (unless its your own channel/direct peer).

Sorry, this was poorly phrased. We validate channel_announcements asynchronously (since this requires calls to bitcoind), so we can receive a valid channel_update while the channel_announcement is being validated, in which case we don't want to ignore that channel_update, we want to stash it to validate it once the channel_announcement has been validated. Does that make more sense?

Mostly what we'd like is to be able to properly type channel_update at decoding time. Right now when we receive a channel_update from the wire, we need to look at several internal maps to choose what processing flow we should apply to the channel_update and where to store it.

Being able to properly type channel_updates would ensure that it's impossible to have a channel_update for a private channel using aliases end up being sent to the network because of a bug in internal maps / processing flow. We can do that proper typing later in the flow, once we've identified what kind of updates it is, but we figured that it would be much better to be explicit directly in the message since it's trivial to do and would simplify the flow for some implementations (even though I think every implementation could benefit from it, because it should remove the need for a lookup in an internal map).

It's trivial for senders to set this bit, it remove some ambiguity in how the message is interpreted, so it feels like a useful change?

[t-bast]

I don't think this helps the lnd implementation any since we know which SCID aliases we are using for a given channel

But as a receiver, when you receive a channel_update on a connection, you'd have to do a lookup on a mapping table to know that this update is for a local private channel, right?

[Crypt-iQ]

I don't think this helps the lnd implementation any since we know which SCID aliases we are using for a given channel

But as a receiver, when you receive a channel_update on a connection, you'd have to do a lookup on a mapping table to know that this update is for a local private channel, right?

Yes, we have to lookup that we know of the alias, but I think this patch doesn't get rid of that except if the update designates that the SCID is not an alias -- If I receive an update saying this is for an alias, I still need to check my maps to check that the alias exists?

[TheBlueMatt]

Sorry, this was poorly phrased. We validate channel_announcements asynchronously (since this requires calls to bitcoind), so we can receive a valid channel_update while the channel_announcement is being validated, in which case we don't want to ignore that channel_update, we want to stash it to validate it once the channel_announcement has been validated. Does that make more sense?

Okay, I still don't see how this proposal materially helps you? If you haven't received/aren't processing the channel_announcement you can still drop the channel_update. Even if you don't want to reach ahead to implement that, presumably you have some anti-DoS mechanism to limit the queue size, can you not simply let the queue fill with these updates and drop them as new updates come in?

Mostly what we'd like is to be able to properly type channel_update at decoding time. Right now when we receive a channel_update from the wire, we need to look at several internal maps to choose what processing flow we should apply to the channel_update and where to store it.

But the answer is usually "both" - yes, it may be a channel update for a local channel, but that doesn't mean you don't have to also handle it as gossip, in general you need to handle all gossip for your public channels in both ways.

[Crypt-iQ]

Lnd uses a range of [16_000_000, 16_250_000] and if we receive a value in that range, we know it's an alias so it serves the same purpose as this SCID bit

[t-bast]

Okay, I still don't see how this proposal materially helps you? If you haven't received/aren't processing the channel_announcement you can still drop the channel_update. Even if you don't want to reach ahead to implement that, presumably you have some anti-DoS mechanism to limit the queue size, can you not simply let the queue fill with these updates and drop them as new updates come in?

Yes, that's true. But having the bit set lets us save one lookup in an internal table for one of the cases (which exact lookup you save depends on what you check first when you receive a channel_update: an ID-to-public channel table, an ID-to-private channel table, or an ID-to-pending-announcement table - I guess every implementations has something that roughly looks like that in the channel_update processing path).

Granted, this is something we can live without. It's not so much about the optimization of one call to a lookup table, but rather the ability of strongly typing channel_update at decoding time, to ensure that private channel_updates never end up processed by a component that may broadcast them to the network. A side-effect of that is the ability to better separate/architect the processing code for channel_updates.

But the answer is usually "both" - yes, it may be a channel update for a local channel, but that doesn't mean you don't have to also handle it as gossip, in general you need to handle all gossip for your public channels in both ways.

But when you have a lot of private channels (which is the case for LSPs), you receive many private channel_updates that aren't part of gossip, so it's starting to make sense to optimize a bit for that case, right?

[t-bast]

Lnd uses a range of [16_000_000, 16_250_000] and if we receive a value in that range, we know it's an alias so it serves the same purpose as this SCID bit

What?!? But you can't expect other implementations to use exactly that range (and we expect our node to be able to handle more than 250k channels with wallets, so we'd need a bigger range than that)! A specified bit that everybody agrees on is widely better than a heuristic that will work only for lnd...

[Crypt-iQ]

Lnd uses a range of [16_000_000, 16_250_000] and if we receive a value in that range, we know it's an alias so it serves the same purpose as this SCID bit

What?!? But you can't expect other implementations to use exactly that range (and we expect our node to be able to handle more than 250k channels with wallets, so we'd need a bigger range than that)! A specified bit that everybody agrees on is widely better than a heuristic that will work only for lnd...

It's like 2^58 possible SCIDs, that's just the block range. Since the counter-party only sends my aliases during gossip, this is fine - lnd stores counter-party aliases in a separate table and JIT inserts the alias in the gossip message if necessary? Rust-lightning was able to handle our SCID range just fine

EDIT: I see your point, you may not use an obvious range, so you always have to query your mapping to determine if it's an alias

[TheBlueMatt]

Granted, this is something we can live without. It's not so much about the optimization of one call to a lookup table, but rather the ability of strongly typing channel_update at decoding time, to ensure that private channel_updates never end up processed by a component that may broadcast them to the network. A side-effect of that is the ability to better separate/architect the processing code for channel_updates.

Right, I guess my reaction here is more that its not strongly-typed as a result, you've taken one of many different cases (public-our-channel, public-not-our-channel, nonpublic-our-channel, nonpublic-alias-our-channel) and flagged it, but you haven't flagged the other cases, which have similar handling issues/concerns. If we want to strongly-type this we should flag more than just this type.

[t-bast]

It's like 2^58 possible SCIDs, that's just the block range.

Ok, I didn't understand that this was the range of the block_height part of the scid (for aliases we're not treating them as block_height/tx_index/output_index triplets at all in eclair, just as a 64-bit value).

You're also completely right that we only care about the range we use for our own aliases, because the only channel_updates you'll receive that use an alias use your local alias, that's a good point. But you may have a backwards-compat' issue if someday you change the range you're using (and you update your node after it has generated a local alias that doesn't fit in the new range), that's why I think a bit in the message would be cleaner.

[TheBlueMatt]

Lnd uses a range of [16_000_000, 16_250_000] and if we receive a value in that range, we know it's an alias so it serves the same purpose as this SCID bit

Bleh, why not use realistic values? Especially post-taproot-in-use its a nice privacy win to do so.

[t-bast]

Right, I guess my reaction here is more that its not strongly-typed as a result, you've taken one of many different cases (public-our-channel, public-not-our-channel, nonpublic-our-channel, nonpublic-alias-our-channel) and flagged it, but you haven't flagged the other cases, which have similar handling issues/concerns. If we want to strongly-type this we should flag more than just this type.

It's true, it isn't perfect, but it's already better than nothing, because it lets you immediately identify channel_updates for which you'll never need to validate a channel_announcement (the channel is either private or public with 0-conf) and which you shouldn't hand to your staggered rebroadcast component. For such a trivial change on the sender side, I personally think it's worth it, but if others disagree we'll work without it!

[TheBlueMatt]

It's true, it isn't perfect, but it's already better than nothing, because it lets you immediately identify channel_updates for which you'll never need to validate a channel_announcement

No? If you receive a channel_update for a private channel today with a real SCID you have the same issue. I'm not sure how SCID aliases changed anything, but I'd be happy to see the flag repurposed to indicate more so that this could be implemented.

[Crypt-iQ]

Lnd uses a range of [16_000_000, 16_250_000] and if we receive a value in that range, we know it's an alias so it serves the same purpose as this SCID bit

Bleh, why not use realistic values? Especially post-taproot-in-use its a nice privacy win to do so.

Less chance of me messing something up in a tricky part of the codebase

for which you'll never need to validate a channel_announcement (the channel is either private or public with 0-conf

is it not necessary to validate the announcement for public 0-conf after 6 confs?

[t-bast]

is it not necessary to validate the announcement for public 0-conf after 6 confs?

Yes you should, but after 6-conf the channel_updates should start using the real scid (for a public channel) and thus won't set the alias bit, so these updates will automatically go in the component that handles on-chain validation.

No? If you receive a channel_update for a private channel today with a real SCID you have the same issue. I'm not sure how SCID aliases changed anything, but I'd be happy to see the flag repurposed to indicate more so that this could be implemented.

Do you mean that we should add the following message flags to make this more useful?

• uses_alias: the scid in that channel_update is an alias
• unannounced_channel: this channel will not be announced once confirmed

If so I'd be happy to add them. I should experiment a bit more with the code to see if those are the only ones we need, does this look correct to you?

[TheBlueMatt]

Do you mean that we should add the following message flags to make this more useful?

Yes, that's my alternative proposal here.

[t-bast]

Thinking about this more, I'm not sure we need more flags. Correct me if I'm missing something, but I believe the only thing we want to know is: "is this channel update supposed to stay private between me and my peer?". There are two cases where that is true:

• unannounced channels with scid_alias
• unannounced channels without scid_alias

The uses_alias field covers the first case. The second case would need a new flag, but if we want to be able to actually rely on it, we'd need all implementations to support this. Given that all implementations are currently working on shipping scid_alias, does it make sense to require another change? Since we'll be waiting for our direct peers to update, we simply need them to update to support scid_alias and at that point require that private channels use scid_alias, which makes the second case obsolete?

[TheBlueMatt]

Correct me if I'm missing something, but I believe the only thing we want to know is: "is this channel update supposed to stay private between me and my peer?". There are two cases where that is true:

Yes, agreed, I just figured the extra flag distinction may be useful to someone so might as well, but I don't feel strongly about that.

The uses_alias field covers the first case. The second case would need a new flag, but if we want to be able to actually rely on it, we'd need all implementations to support this. Given that all implementations are currently working on shipping scid_alias, does it make sense to require another change? Since we'll be waiting for our direct peers to update, we simply need them to update to support scid_alias and at that point require that private channels use scid_alias, which makes the second case obsolete?

Hmm, maybe? That's a good point I hadn't thought about that. I'm maybe a bit dubious that we should add a flag for something that people rely on meaning X, when the protocol allows it to mean Y, even though it usually means X. That seems like a great way to mis-implement it.

[t-bast]

The issue is that an additional isPrivate flag cannot be backwards-compatible with existing nodes (who will set it to 0 even for private channels), while a uses_alias flag can be fully backwards-compatible since existing nodes don't use aliases on mainnet yet and currently set that bit to 0.

I'm maybe a bit dubious that we should add a flag for something that people rely on meaning X, when the protocol allows it to mean Y, even though it usually means X. That seems like a great way to mis-implement it.

Are you talking about the uses_alias flag here? The meaning of that one should be completely straight-forward, it just adds more information about the short_channel_id included in that channel_update. Or am I missing a different way people may understand this?

I think I'd rather add only this flag to the spec, wait for 6 months until enough nodes on the network advertise support for option_scid_alias then make it mandatory on eclair. At that point we'll know for sure that:

• a channel_update that sets uses_alias must never be broadcast to the network and is only useful for invoice generation
• a channel_update that does not set uses_alias is for a public channel and must be broadcast to the network

WDYT?

[TheBlueMatt]

The issue is that an additional isPrivate flag cannot be backwards-compatible with existing nodes (who will set it to 0 even for private channels), while a uses_alias flag can be fully backwards-compatible since existing nodes don't use aliases on mainnet yet and currently set that bit to 0.

I mean this isn't entirely true, scid aliases have been deployed in LDK for some time now, but, sure, its not super broad and those users upgrade quickly currently. Still, I think the broader point is that this flag is really a hint to the recipient to skip the processing of it as if it were about a public channel, which doesn't really need backwards compatibility?

Are you talking about the uses_alias flag here? The meaning of that one should be completely straight-forward, it just adds more information about the short_channel_id included in that channel_update. Or am I missing a different way people may understand this?

Well, no, you're talking about using it for Y here, where Y is "skip the processing of this message as if it were a public channel". Its defined around the SCID being an alias, but the change in handling is presumably only on the public-graph part of the codebase?

I think I'd rather add only this flag to the spec, wait for 6 months until enough nodes on the network advertise support for option_scid_alias then make it mandatory on eclair. At that point we'll know for sure that:

Sure, but you can do the same whether the bit(s) are defined as uses-alias as you can do if the bits are defined more generally.

[t-bast]

I mean this isn't entirely true, scid aliases have been deployed in LDK for some time now, but, sure, its not super broad and those users upgrade quickly currently.

I didn't know it had been deployed so soon after the spec PR was merged, it indeed means we'll need those early adopters to upgrade quickly.

Still, I think the broader point is that this flag is really a hint to the recipient to skip the processing of it as if it were about a public channel, which doesn't really need backwards compatibility?
Sure, but you can do the same whether the bit(s) are defined as uses-alias as you can do if the bits are defined more generally.

My point is that with this exact flag + making scid_alias mandatory on your node, this hint becomes more than just a hint: if it's not set, you're 100% sure this is a public channel_update meant for rebroadcast. I believe other uses of message_flags would only be hints, which means implementations will still need to handle the case where the condition is true but the hint isn't set, which means you can't simplify your codebase to rely entirely on those flags.

[TheBlueMatt]

Seems like the meeting conclusion here was to either use a flag to mean "not public channel update" or have two flags as I'd proposed. Is that what you took away form the conversation?

[rustyrussell]

Ack, this seems good. Closes 996...

[Roasbeef]

LGTM 🍬

[t-bast]

This is implemented in ACINQ/eclair#2362, let me know if you want me to do cross-compat tests between eclair and another implementation!

[harding]

After the merge of this PR, the spec no longer describes what htlc_maximum_msat is for. E.g. none of these is informative,

$ git grep htlc_maximum_msat
07-routing-gossip.md:    * [`u64`:`htlc_maximum_msat`]
07-routing-gossip.md:  - if `htlc_maximum_msat` is greater than channel capacity:
07-routing-gossip.md:    - SHOULD consider the `htlc_maximum_msat` when routing.
07-routing-gossip.md:the presence of the `htlc_maximum_msat` field. This field must now always

In particular, I think it would be helpful to restore the following two lines to somewhere in the spec:

-    - MUST set `htlc_maximum_msat` to the maximum value it will send through this channel for a single HTLC.
-      - MUST set this to less than or equal to the channel capacity.

It looks to me like the PR that this PR superseded did that: #996

FWIW, I noticed this when trying to confirm my understanding of htlc_maximum_msat after reading the recent mailing list discussion about using it as a flow control device: https://lists.linuxfoundation.org/pipermail/lightning-dev/2022-September/003686.html

Alternatively, maybe the term should be added to the glossary in BOLT0

[t-bast]

Thanks @harding, you're right this is my mistake! I'll correct that with a follow-up PR.

[t-bast]

@harding done in #1029, thanks for the report!