Rename instances of is_public to is_announced

[tnull]

Referring to announced/unannounced channels as 'public'/'private' regularly leads to confusion on what they are and when which should be used. To avoid perpetuating this confusion, we should avoid referring to announced channels as 'public' in our API.

Here we rename the recently introduced field in OpenChannelRequest (which doesn't break released API), and align the pre-existing instances of is_public in the following commit (which breaks API).

Tagging this 0.0.124 as this is just a minor change and doing it before the changes from #3019 ship would be nice.

[codecov]

Codecov Report

Attention: Patch coverage is 95.94595% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.92%. Comparing base (bbfa15e) to head (5928063).
Report is 71 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	91.66%	1 Missing ⚠️
lightning/src/ln/invoice_utils.rs	85.71%	0 Missing and 1 partial ⚠️
lightning/src/routing/router.rs	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3257      +/-   ##
==========================================
+ Coverage   89.82%   90.92%   +1.10%     
==========================================
  Files         125      126       +1     
  Lines      102867   112889   +10022     
  Branches   102867   112889   +10022     
==========================================
+ Hits        92398   102646   +10248     
+ Misses       7753     7725      -28     
+ Partials     2716     2518     -198     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TheBlueMatt]

Maybe is_routing? I think its really more about the fact that the channel routes for others, rather than that its announced.

[tnull]

Maybe is_routing? I think its really more about the fact that the channel routes for others, rather than that its announced.

Mh, true, but it's referred to announced channels in the spec and in many other places, so introducing yet another terminology might also lead to more confusion? FWIW, if we want to go this direction is_forwarding might be even more explicitly communicating the fact that the distinction is really between the willingness to forward payments or not over the given channel.

[TheBlueMatt]

I feel like "public channels" is much more common terminology (outside the spec, maybe) than "announced channels", but we shouldn't worry too much about the common name here, IMO, though we should note the other names in documentation.

[arik-so]

would differently named accessor methods help that simply return the same variable? Though it might either make using the API simpler, or more confusing.

[tnull]

I feel like "public channels" is much more common terminology (outside the spec, maybe) than "announced channels", but we shouldn't worry too much about the common name here, IMO, though we should note the other names in documentation.

Alright. I think I'd prefer is_announced or is_forwarding, but also happy to go with is_routing if you prefer. Let me know in which direction I should update this PR.

would differently named accessor methods help that simply return the same variable? Though it might either make using the API simpler, or more confusing.

I don't think so: we try to improve clarity by reducing the use of misleading terminology. Just allowing all ways just makes it more ambiguous/confusing, IMO.

[tnull]

Rebased on main to resolve minor conflict.

[jkczyz]

IMO, is_announced would be preferred as it indicates the channel is gossiped, and we also use "announced channels" in our docs already (along with "public channels"). I don't think I've heard anyone say a "routing channel" or a "forwarding channel". That seems to be true of all channels, so those names don't really imply "for others". The terms "routing" or "forwarding" are more suitably applied to nodes not channels.

[TheBlueMatt]

Alright. I think I'd prefer is_announced or is_forwarding, but also happy to go with is_routing if you prefer. Let me know in which direction I should update this PR.

Yea, is_forwarding sounds fine to me too.

IMO, is_announced would be preferred as it indicates the channel is gossiped

Right, but the fact that its gossipsed is a side-effect of the fact that the channel intends to route payments for others, not the other way around.

we also use "announced channels" in our docs already (along with "public channels")

Sounds like we should unify around one term and use it consistently everywhere :)

I don't think I've heard anyone say a "routing channel" or a "forwarding channel".

Hmm, not sure either, but pretty sure I've heard "public channel" and "public node" most commonly, so if we want to go based on "common parlance" we should probably stick to the way it is now. That said, I'm not sure we should go based on "common parlance" as long as its understandable to people who are used to lightning terminology. We should generally prefer the clearest terms, IMO.

That seems to be true of all channels, so those names don't really imply "for others".

Hmm, but if you're "forwarding" a payment, you're definitely not doing it for yourself? You're clearly forwarding for someone else.

The terms "routing" or "forwarding" are more suitably applied to nodes not channels.

I disagree with this - you can have a node that has "forwarding" channels and non-"forwarding" channels on the same node (though its not particularly common). "Public"/"announced" could describe a node (i.e. it has more than zero channels which are gossiped, making the node publicly visible), too.

I think one argument for public/announced over routing/forwarding for a channel is that you can disable routing/forwarding for a public/announced channel by setting the fees to infinite, though that's maybe also a reason why we shouldn't call it public/announced - we should discourage people from announcing channels that aren't going to forward.

[jkczyz]

Hmm, but if you're "forwarding" a payment, you're definitely not doing it for yourself? You're clearly forwarding for someone else.

How are you defining "forwarding"? Or more specifically, when is a channel not a forwarding channel? If a node has only one channel, and that channel is announced, is that a forwarding channel?

[TheBlueMatt]

How are you defining "forwarding"? Or more specifically, when is a channel not a forwarding channel?

In this case a channel is not a "forwarding" channel if we will not accept HTLCs to it. Though I guess its a bit more complicated based on accept_forwards_to_priv_channels, really in this context a "forwarding channel" is one that accepts HTLCs from rando peers (ie its "publicly forwardable").

Maybe public is in fact the right term and not announced or forwarding/routing 🤷‍♂️

If a node has only one channel, and that channel is announced, is that a forwarding channel?

You can happily forward over a single channel, not sure why you would, but I do see HTLCs occasionally come into my node from one peer and go straight back out the same channel.

[tnull]

Alright, excuse the delay here.

I personally would prefer to change is_public as it invites a bunch of misunderstandings, which often lead to users defaulting to it 'to play it safe', which however ends up a) being bad for the network as a whole and b) counterintuitively often the more unreliable choice (e.g., as route hints are much more reliable routing information than gossiped node announcements for many no-always-on nodes).

we should discourage people from announcing channels that aren't going to forward.

Yes, I strongly agree with this, and this PR is one step towards discouraging it further. I think we really want to make it clear that you have no business announcing channels unless you intend to run a 24/7 LSP or forwarding node and that doing so could have a series of repercussions that aren't immediately clear.

So, with some time passed, how do we conclude @TheBlueMatt, @jkczyz? Leave it is_announced, change it to is_forwarding? Something else?

[dunxen]

My 2 cents is that is_announced seems clearest even with public being commonplace. It's the least misleading. For dev users of LDK it might not cause as much confusion - I don't have anecdotal evidence here, but for end-users I've definitely seen the misunderstanding that "private" channels are somehow more secure from a cryptography point of view. So this change would hopefully encourage developers to start using this more accurate (IMO) terminology in their actual end-user-facing apps.

Either way, we need to just stick with one term throughout the project as Matt pointed out.

[TheBlueMatt]

Maybe is_announced_for_forwarding or is that too much of a mouthful?

[dunxen]

Maybe is_announced_for_forwarding or is that too much of a mouthful?

I'm ok with this too.

[Kixunil]

Right, but the fact that its gossipsed is a side-effect of the fact that the channel intends to route payments for others, not the other way around.

@TheBlueMatt FYI this is not entirely accurate since one can forward through an unannounced channel if the two nodes have another announced channel. (IIRC the name is non-strict forwarding.) Last time I checked (several years ago) it was supported by LND and Eclair.

discourage people from announcing channels that aren't going to forward.

There's currently incentive to announce channels even if you're doing shitty forwarding job to gain plausible deniability since then it's much harder for neighboring nodes to identify your payments. So if you want to discourage people from announcing them you need to figure out how to avoid leaking information by using unannounced channels.

[TheBlueMatt]

@TheBlueMatt FYI this is not entirely accurate since one can forward through an unannounced channel if the two nodes have another announced channel. (IIRC the name is non-strict forwarding.) Last time I checked (several years ago) it was supported by LND and Eclair.

Yes, I mentioned this above (in LDK its controlled by accept_forwards_to_priv_channels) :).

There's currently incentive to announce channels even if you're doing shitty forwarding job to gain plausible deniability since then it's much harder for neighboring nodes to identify your payments. So if you want to discourage people from announcing them you need to figure out how to avoid leaking information by using unannounced channels.

Sure, there may be routing nodes that are bad because they want to be a routing node for plausible deniability, but they're still routing, and the announcement is for the routing thing, not for the privacy. The privacy comes from the routing.

[Kixunil]

The privacy comes from the routing.

That's a good point that the better you forward the more privacy you gain. Still it sucks that this is required. I was thinking that wallets could open channels to multiple LSPs and use some kind of service where they occasionally interactively ask "privacy providers" to route through them by giving them routing hints to confuse the LSPs (assuming no two nodes are cooperating on breaking the privacy of the user).

[TheBlueMatt]

I was thinking that wallets could open channels to multiple LSPs and use some kind of service where they occasionally interactively ask "privacy providers" to route through them by giving them routing hints to confuse the LSPs (assuming no two nodes are cooperating on breaking the privacy of the user).

If you're just looking for plausible deniability you could just announce and be a "normal" routing node but fail usually....but also please please don't do that. Flooding the network with garbage "routing" nodes is really bad for the network.

[Kixunil]

but also please please don't do that. Flooding the network with garbage "routing" nodes is really bad for the network.

Exactly, and that's why I don't want people to do it, so I was trying to come up with other solutions. (Also ideally LN should be robust against this kind of griefing but I understand it's hard.)

[TheBlueMatt]

Lightning nodes with proper graph-scoring datasets are robust against it, but its still noise that probers have to ignore and more crap....anyway, all the more reason I think the naming here should focus on "routing" not public/announced :)

[Kixunil]

naming here should focus on "routing" not public/announced

Maybe "spamming" ;) But more seriously "gossiped"?

[TheBlueMatt]

The point of gossip is for routing...if you're not routing, you're spamming, and our documentation should nudge users towards not doing that :)

[jkczyz]

So, with some time passed, how do we conclude @TheBlueMatt, @jkczyz? Leave it is_announced, change it to is_forwarding? Something else?

I'm in favor of keeping is_announced.

Maybe is_announced_for_forwarding or is that too much of a mouthful?

I'm not sure introducing new terminology helps, TBH. The "for forwarding" clarification can just be included in the docs, IMO. When helping a user debug an issue, I'm going to ask if they have an "announced channel", not a "channel announced for forwarding."

[TheBlueMatt]

I'm not sure introducing new terminology helps, TBH. The "for forwarding" clarification can just be included in the docs, IMO. When helping a user debug an issue, I'm going to ask if they have an "announced channel", not a "channel announced for forwarding."

After the interaction with @Kixunil above I feel even more confident that we shouldn't just call it announced. Documentation or not, people read based on what the variable names are, and calling these things "announced" is a misnomer that gives the wrong impression of what the thing is for.

I don't think common terminology is the right way to decide naming, but rather whether it is the clearest name that correctly communicates the intent and won't confuse people (eg by being toooo far from common terminology).

[Kixunil]

The misleading thing about other names is that !announced() doesn't imply !forwarding. Maybe something along the lines of publicly_marked_for_forwarding where the publicly suggests there's a non-public way to mark a channel for forwarding.

[jkczyz]

and calling these things "announced" is a misnomer that gives the wrong impression of what the thing is for.

But what exactly is the wrong impression that is given?

[TheBlueMatt]

The misleading thing about other names is that !announced() doesn't imply !forwarding

It implies a specific kind of !forwarding - !forwarding for a third party (though it may still forward to or from the channel, but only "for" that channel). I dunno how to properly communicate that, though...

But what exactly is the wrong impression that is given?

That the intent here is to announce the channel and that you might want to announce the channel for various reasons. The intent of a channel announcement, and the only reason you should ever announce a channel, is to forward payments for unrelated third parties over that channel. Once you've decided to forward payments over the channel, announcement is just a byproduct.

[jkczyz]

That the intent here is to announce the channel and that you might want to announce the channel for various reasons. The intent of a channel announcement, and the only reason you should ever announce a channel, is to forward payments for unrelated third parties over that channel. Once you've decided to forward payments over the channel, announcement is just a byproduct.

The name is_announced_for_forwarding reads differently to me. While it conveys that the channel was announced for the purpose of forwarding, it does not preclude the fact that there may be other reasons for the channel being announced that are not covered by the variable (i.e., this value may be false but the channel nonetheless may have been announced for another reason). With is_announced there is no ambiguity.

[TheBlueMatt]

Hmm, well I guess in theory there could be some other kind of announcement, but currently there isnt :). As for is_announced not being ambiguous, I strongly disagree, not only can it be ambiguous, but we've seen confusion, immediately in this PR (but also I've seen it elsewhere) where people want to do announced channels for reasons other than forwarding, which I'd call a result of ambiguity giving people incorrect conclusions.

[Kixunil]

!forwarding for a third party

Non-strict forwarding is forwarding for a third party though.

Though I now realized why there's confusion. I think is_announced is the correct name for a getter if you don't know the reason why a channel was announced but publicly_enable_forwarding is better name for a setter for your own channels. And if you statically know a channel is yours then is_forwarding_publicly_enabled would make sense.

[TheBlueMatt]

Non-strict forwarding is forwarding for a third party though.

By "for a third party" I mean "neither the sender nor recipient of the payment are the channel counterparty".

[TheBlueMatt]

And if you statically know a channel is yours then is_forwarding_publicly_enabled would make sense.

In this context we always know the channel is ours. If the channel isn't ours, then its in our network graph and by definition publicly announced :)

[Kixunil]

"neither the sender nor recipient of the payment are the channel counterparty"

IIRC non-strict forwarding allows it.

If the channel isn't ours, then its in our network graph

Haven't looked into the code and I assumed the case when you know about a channel from routing hints exists.

[jkczyz]

Hmm, well I guess in theory there could be some other kind of announcement, but currently there isnt :). As for is_announced not being ambiguous, I strongly disagree, not only can it be ambiguous, but we've seen confusion, immediately in this PR (but also I've seen it elsewhere) where people want to do announced channels for reasons other than forwarding, which I'd call a result of ambiguity giving people incorrect conclusions.

I'm not referring to another type of announcement, just the reason for it. A value of false doesn't preclude that the node was announced for a different reason from the reader's perspective.

[TheBlueMatt]

IIRC non-strict forwarding allows it.

In theory, yes, in practice, its not ever done.

I'm not referring to another type of announcement, just the reason for it. A value of false doesn't preclude that the node was announced for a different reason from the reader's perspective.

Right, and that's precisely the misconception we should counter here. If they're doing it for some other reason, they're wrong, and they actually did it for forwarding :).

[jkczyz]

Right, and that's precisely the misconception we should counter here. If they're doing it for some other reason, they're wrong, and they actually did it for forwarding :).

But can you see how a value of false for ChannelDetails::is_announced_for_forwarding, say, doesn't convey that the channel hasn't been announced?

[TheBlueMatt]

Right, but if I were confused about it, I'd assume there would be some ChannelDetails::is_announced_for_other_reason present, which there is not.

[jkczyz]

I'm sympathetic to the argument of having is_announced used for getters (i.e., ChannelDetails) while using a more detailed name in setters (i.e., events and method parameters). But I don't feel strongly enough about this naming to argue much further. At very least, the shorter version for the former would seem to result in less of a wrong impression given it is always seen after the fact of the action that set it.

[tnull]

So, to summarize, we so far discussed the following alternatives and people voiced their support for:

keep is_public: nobody
change to is_announced: dunxen, jkczyz, Kixunil (?), tnull
change to is_routing: TheBlueMatt
change to is_forwarding: tnull
change to is_forwarding_publicly_enabled: Kixunil
change to is_announced_for_forwarding: dunxen, TheBlueMatt

Let me know if I misrepresented/forgot something, but at least by number of votes is_announced seems to come out on top current?

[vincenzopalazzo]

Every name is fine for me, I think we should just make more docs (if there are no already) about the field and maybe add a separate section that describes a little bit what is the "public/announced" definition.

From my point of view, I think in this case is hard to define what is confusing the user, because (sorry if am missing some previous message) we do not know what is the kind of user and if it has some previous familiarity with other lightning implementation.

e.g: if I am a core lightning user I am used to opening a private channel, not a not announced channel. So for this kind of user public should not be confused. However, I think that in this case, can we take as a reference the BOLT 2 where there is announce_channel definition? so in this case every user that wants to know what an announced (or is_announced_for_forwarding) channel is can take as a reference the bolt2 too

[jkczyz]

Let me know if I misrepresented/forgot something, but at least by number of votes is_announced seems to come out on top current?

Also use is_announced for "getters" (i.e., in ChannelDetails) and is_announced_for_forwarding (or similar but phrased appropriately) for "setters" (i.e., in ChannelHandshakeConfig) using those terms loosely. That at least makes the intent clear when performing an action, while the more general use covers the current state where the why seems a little out of place, IMO. At very least, tacking on _for_some_reason to other fields in ChannelDetails seems unnecessary even if a similar argument applies. Seems like a reasonable middle ground without inviting confusion.

[tnull]

As discussed offline, I now added a commit renaming ChannelHandshakeConfig::announced_channel to ChannelHandshakeConfig::is_announced_for_forwarding.

Didn't touch other fields (as for example force_announced_channel_preference). Let me know if I should and I will (but likely not anymore today).

[jkczyz]

LGTM. Please squash.

[tnull]

LGTM. Please squash.

Squashed without further changes.