Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Explicitly allow funding_locked early, and support alias scids (feat 46/47/50/51) #910

Merged
merged 6 commits into from
May 30, 2022
Merged

Explicitly allow funding_locked early, and support alias scids (feat 46/47/50/51) #910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d41cc1e
Explicitly allow funding_locked early, and support alias scids.
rustyrussell Aug 12, 2021
faa6c41
BOLT 2: Restore minimum_depth requirement, but explicitly allow 0.
rustyrussell Oct 19, 2021
7aa76b6
BOLT 2: add channel_type for option_scid_alias
rustyrussell Oct 25, 2021
f8e5c92
channel_update: make sure we use alias scids correctly.
rustyrussell Nov 9, 2021
7a812cf
Make zeroconf a channel type, and acceptance indicates trust.
rustyrussell Feb 15, 2022
34e9cd9
Rename `funding_locked` to `channel_ready`.
rustyrussell Feb 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 86 additions & 29 deletions 02-peer-protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ operation, and closing.
* [The `accept_channel` Message](#the-accept_channel-message)
* [The `funding_created` Message](#the-funding_created-message)
* [The `funding_signed` Message](#the-funding_signed-message)
* [The `funding_locked` Message](#the-funding_locked-message)
* [The `channel_ready` Message](#the-channel_ready-message)
* [Channel Close](#channel-close)
* [Closing Initiation: `shutdown`](#closing-initiation-shutdown)
* [Closing Negotiation: `closing_signed`](#closing-negotiation-closing_signed)
Expand Down Expand Up @@ -70,8 +70,8 @@ must broadcast the funding transaction to the Bitcoin network. After
the `funding_signed` message is sent/received, both sides should wait
for the funding transaction to enter the blockchain and reach the
specified depth (number of confirmations). After both sides have sent
the `funding_locked` message, the channel is established and can begin
normal operation. The `funding_locked` message includes information
the `channel_ready` message, the channel is established and can begin
normal operation. The `channel_ready` message includes information
that will be used to construct channel authentication proofs.


Expand All @@ -82,8 +82,8 @@ that will be used to construct channel authentication proofs.
| A |--(3)-- funding_created --->| B |
| |<-(4)-- funding_signed -----| |
| | | |
| |--(5)--- funding_locked ---->| |
| |<-(6)--- funding_locked -----| |
| |--(5)--- channel_ready ---->| |
| |<-(6)--- channel_ready -----| |
+-------+ +-------+

- where node A is 'funder' and node B is 'fundee'
Expand Down Expand Up @@ -207,12 +207,16 @@ definitions they reuse even feature bits, but they are not an
arbitrary combination (they represent the persistent features which
affect the channel operation).

The currently defined types are:
The currently defined basic types are:
- no features (no bits set)
- `option_static_remotekey` (bit 12)
- `option_anchor_outputs` and `option_static_remotekey` (bits 20 and 12)
- `option_anchors_zero_fee_htlc_tx` and `option_static_remotekey` (bits 22 and 12)

Each basic type has the following variations allowed:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these mutually exclusive

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're saying we should be explicit in the text to say that they are not mutually exclusive? You obviously want to support both.

- `option_scid_alias` (bit 46)
- `option_zeroconf` (bit 50)

#### Requirements

The sending node:
Expand All @@ -239,6 +243,8 @@ The sending node:
- MUST set it to a defined type representing the type it wants.
- MUST use the smallest bitmap possible to represent the channel type.
- SHOULD NOT set it to a type containing a feature which was not negotiated.
- if `announce_channel` is `true` (not `0`):
- MUST NOT send `channel_type` with the `option_scid_alias` bit set.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs some kind of feature gate - prior to this PR sending a channel_ready after the channel has advanced state` is not allowed (and, at least, we would barf on it).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we just only allow this with the SCID alias channel type bit? There's not much reason to do it otherwise.

Copy link
Contributor

@Crypt-iQ Crypt-iQ Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the spec as-is doesn't specify what to do if funding_locked is received multiple times. Since an older node complying with an earlier version of the spec may fail on this, it seems to me that having it packaged with the feature bit is better. It would still be possible to send aliases for existing channels, but only if both nodes have the feature bit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we just only allow this with the SCID alias channel type bit

looks like this is covered below by the clause on line 517?

- if `channel_type` has `option_scid_alias` set:
   - MAY send multiple `channel_ready` messages to the same peer with different `alias` values.

But perhaps it should be gated by the option_scid_alias feature bit instead of the channel type? cause surely we may want to send multiple channel_ready in the case of a public (ie, not scid_alias channel) zero conf channel too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that seems reasonable, the gating here should be on the feature bit, not the channel type.


The sending node SHOULD:
- set `to_self_delay` sufficient to ensure the sender can irreversibly spend a commitment transaction output, in case of misbehavior by the receiver.
Expand Down Expand Up @@ -276,7 +282,9 @@ are not valid secp256k1 pubkeys in compressed format.
- the funder's amount for the initial commitment transaction is not sufficient for full [fee payment](03-transactions.md#fee-payment).
- both `to_local` and `to_remote` amounts for the initial commitment transaction are less than or equal to `channel_reserve_satoshis` (see [BOLT 3](03-transactions.md#commitment-transaction-outputs)).
- `funding_satoshis` is greater than or equal to 2^24 and the receiver does not support `option_support_large_channel`.
- It supports `channel_type`, `channel_type` was set, and the `type` is not suitable.
- It supports `channel_type` and `channel_type` was set:
- if `type` is not suitable.
- if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel.

The receiving node MUST NOT:
- consider funds received, using `push_msat`, to be received until the funding transaction has reached sufficient depth.
Expand Down Expand Up @@ -344,8 +352,10 @@ The `temporary_channel_id` MUST be the same as the `temporary_channel_id` in
the `open_channel` message.

The sender:
- SHOULD set `minimum_depth` to a number of blocks it considers reasonable to
avoid double-spending of the funding transaction.
- if `channel_type` includes `option_zeroconf`:
- MUST set `minimum_depth` to zero.
- otherwise:
- SHOULD set `minimum_depth` to a number of blocks it considers reasonable to avoid double-spending of the funding transaction.
- MUST set `channel_reserve_satoshis` greater than or equal to `dust_limit_satoshis` from the `open_channel` message.
- MUST set `dust_limit_satoshis` less than or equal to `channel_reserve_satoshis` from the `open_channel` message.
- if it sets `channel_type`:
Expand Down Expand Up @@ -461,31 +471,67 @@ use `option_static_remotekey`, `option_anchor_outputs` or
`option_static_remotekey`, and the superior one is favored if more than one
is negotiated.

### The `funding_locked` Message
### The `channel_ready` Message

This message (which used to be called `funding_locked`) indicates that the funding transaction has sufficient confirms for channel use. Once both nodes have sent this, the channel enters normal operating mode.

This message indicates that the funding transaction has reached the `minimum_depth` asked for in `accept_channel`. Once both nodes have sent this, the channel enters normal operating mode.
Note that the opener is free to send this message at any time (since it presumably trusts itself), but the
accepter would usually wait until the funding has reached the `minimum_depth` asked for in `accept_channel`.

1. type: 36 (`funding_locked`)
1. type: 36 (`channel_ready`)
2. data:
* [`channel_id`:`channel_id`]
* [`point`:`next_per_commitment_point`]
* [`point`:`second_per_commitment_point`]
* [`channel_ready_tlvs`:`tlvs`]

1. `tlv_stream`: `channel_ready_tlvs`
2. types:
1. type: 1 (`short_channel_id`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we use an odd type tlv here? surely we would only include this tlv if we negotiated option_scid_alias with our peer in which case we expect them to understand the tlv?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the feature bit doesn't indicate anything about the presence or lack of SCID alias field. You're also welcome to send an SCID alias without the feature bit, and the peer can interpret that or ignore it as they see fit. This greatly simplifies the logic in all clients (except apparently lnd, which has a different approach to features).

Note further that the scid_alias feature is somewhat of a misnomer, it's really "scid alias only" (in LDK we're calling it "scid privacy" instead), not "scid alias".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the spec allows sending a SCID alias without the feature bit (i.e. multiple funding_locked), then what was the reasoning behind this comment https://github.com/lightning/bolts/pull/910/files#r807436433

Copy link
Collaborator

@TheBlueMatt TheBlueMatt Mar 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that comment is on the wrong line, i was intending to talk generally about the ability to send multiple funding_locked messages, and you responded as such. I'm not sure what your question is here - you can use the feature bit (or some other gating) to limit whether you can send multiple funding_locked messages, but that doesn't impact whether the scid can be set or interpreted independent of the channel type in the funding_locked.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the comment I linked was about sending multiple funding_locked, which as I understand it should be gated on the feature bit. But you're saying here that you can send anything in the TLV (as usual) including this scid alias regardless of the feature bit. If I understand you correctly here, then I agree

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I believe we agree - I was only referring to the "send redundant funding_locked after the channel is locked in" part being something that needs feature gating. This does not apply the the presence of the scid alias (obviously), but also does not apply to using the scid alias - whether the feature bit is set or not doesn't matter to the fact that you can send an scid alias and the peer is free to use it.

2. data:
* [`short_channel_id`:`alias`]

#### Requirements

The sender MUST:
- NOT send `funding_locked` unless outpoint of given by `funding_txid` and
The sender:
pm47 marked this conversation as resolved.
Show resolved Hide resolved
- MUST NOT send `channel_ready` unless outpoint of given by `funding_txid` and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- MUST NOT send `channel_ready` unless outpoint of given by `funding_txid` and
- MUST NOT send `channel_ready` unless outpoint given by `funding_txid` and

Think either this of needs to be removed or a noun is missing.

`funding_output_index` in the `funding_created` message pays exactly `funding_satoshis` to the scriptpubkey specified in [BOLT #3](03-transactions.md#funding-transaction-output).
- wait until the funding transaction has reached `minimum_depth` before
sending this message.
- set `next_per_commitment_point` to the per-commitment point to be used
for the following commitment transaction, derived as specified in
- if it is not the node opening the channel:
- SHOULD wait until the funding transaction has reached `minimum_depth` before
sending this message.
- MUST set `second_per_commitment_point` to the per-commitment point to be used
for commitment transaction #1, derived as specified in
[BOLT #3](03-transactions.md#per-commitment-secret-requirements).
- if `option_scid_alias` was negotiated:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the channel_type or the feature bit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree it should be clarified but it means the feature bit. Providing an SCID alias for a channel that does not have the scid_alias feature set is an important supported behavior.

- MUST set `short_channel_id` `alias`.
- otherwise:
- MAY set `short_channel_id` `alias`.
- if it sets `alias`:
- if the `announce_channel` bit was set in `open_channel`:
- SHOULD initially set `alias` to value not related to the real `short_channel_id`.
- otherwise:
- MUST set `alias` to a value not related to the real `short_channel_id`.
- MUST NOT send the same `alias` for multiple peers or use an alias which
collides with a `short_channel_id` of a channel on the same node.
- MUST always recognize the `alias` as a `short_channel_id` for incoming HTLCs to this channel.
- if `channel_type` has `option_scid_alias` set:
- MUST NOT allow incoming HTLCs to this channel using the real `short_channel_id`
Copy link
Contributor

@Crypt-iQ Crypt-iQ Oct 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If option_scid_alias_only is negotiated and incoming HTLCs of the real short_channel_id are disallowed, does this break the spontaneous nature of key-send? The payer will need to know one of the alias short_channel_id which will be different from the publicly announced "real" one in the channel_announcement / channel_update messages. Additionally, regular routing may require a payer to know aliases of ALL option_scid_alias_only-negotiating hops along the way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scid aliases don't (currently) make sense for public channels - with the node ids public its obvious to the sender which channel is/isnt being used. They only really make sense to provide privacy to non-public nodes. Agreed this could be more clear here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also makes sense in the zero-conf case before the scid is known

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but that doesn't work for public channels - you have to announce all public channels with the "real" ID for anti-DoS reasons, this isn't unique to keysend. I think what you're really saying here is that option_scid_alias_only needs to disable announcements/imply/require the channel to be private.

Copy link
Contributor

@Crypt-iQ Crypt-iQ Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting confused between the option and the TLV segment. Let's say the option isn't negotiated and a zero-conf channel is opened that is public after minimum_depth 6 confs. Keysend should work once public since routing to the real short_channel_id is allowed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its already implied cause you're not allowed to set a public channel as scid_alias.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clause you're referencing only requires that if the option_scid_alias channel_type was negotiated rather than the feature bit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the current text of this PR, however, only adds the only-use-alias-when-forwarding requirement for the "channel_type has option_scid_alias set", which is correct, I believe. There is nothing implied by the feature bit here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For private zero-conf channels, this line seems to be ignored (the zero-conf and option-scid-alias channel types seem to be exclusive)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly entirely unsure what we're discussing here at this point - the above comments all seem to be on the same page - you can't set scid_alias on a public channel, and for private channels you can set it allowing the limiting of relay to the alias.

- MAY send multiple `channel_ready` messages to the same peer with different `alias` values.
- otherwise:
- MUST wait until the funding transaction has reached `minimum_depth` before sending this message.


The sender:

A non-funding node (fundee):
- SHOULD forget the channel if it does not see the correct funding
transaction after a timeout of 2016 blocks.
transaction after a timeout of 2016 blocks.

From the point of waiting for `funding_locked` onward, either node MAY
The receiver:
- MAY use any of the `alias` it received, in BOLT 11 `r` fields.
- if `channel_type` has `option_scid_alias` set:
- MUST NOT use the real `short_channel_id` in BOLT 11 `r` fields.

From the point of waiting for `channel_ready` onward, either node MAY
send an `error` and fail the channel if it does not receive a required response from the
other node after a reasonable timeout.

Expand All @@ -501,6 +547,16 @@ to broadcast the commitment transaction to get his funds back and open a new
channel. To avoid this, the funder should ensure the funding transaction
confirms in the next 2016 blocks.

The `alias` here is both required for routing (since there real
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `alias` here is both required for routing (since there real
The `alias` here is both required for routing (since the real

short_channel_id is unknown until confirmation), and useful to provide
one or more aliases, even once there is a real `short_channel_id` for
a public channel.

While a node can send multiple `alias`, it must remember all of the
ones it has sent so it can use them should they be requested by
incoming HTLCs. The recipient only need remember one, for use in
`r` route hints in BOLT 11 invoices.

## Channel Close

Nodes can negotiate a mutual close of the connection, which unlike a
Expand Down Expand Up @@ -541,7 +597,7 @@ along with the `scriptpubkey` it wants to be paid to.
A sending node:
- if it hasn't sent a `funding_created` (if it is a funder) or a `funding_signed` (if it is a fundee):
- MUST NOT send a `shutdown`
- MAY send a `shutdown` before a `funding_locked`, i.e. before the funding transaction has reached `minimum_depth`.
- MAY send a `shutdown` before a `channel_ready`, i.e. before the funding transaction has reached `minimum_depth`.
- if there are updates pending on the receiving node's commitment transaction:
- MUST NOT send a `shutdown`.
- MUST NOT send an `update_add_htlc` after a `shutdown`.
Expand All @@ -563,7 +619,7 @@ A receiving node:
- SHOULD send an `error` and fail the channel.
- if the `scriptpubkey` is not in one of the above forms:
- SHOULD send a `warning`.
- if it hasn't sent a `funding_locked` yet:
- if it hasn't sent a `channel_ready` yet:
- MAY reply to a `shutdown` message with a `shutdown`
- once there are no outstanding updates on the peer, UNLESS it has already sent a `shutdown`:
- MUST reply to a `shutdown` message with a `shutdown`
Expand Down Expand Up @@ -713,7 +769,7 @@ the closing transaction will likely never reach miners.

## Normal Operation

Once both nodes have exchanged `funding_locked` (and optionally [`announcement_signatures`](07-routing-gossip.md#the-announcement_signatures-message)), the channel can be used to make payments via Hashed Time Locked Contracts.
Once both nodes have exchanged `channel_ready` (and optionally [`announcement_signatures`](07-routing-gossip.md#the-announcement_signatures-message)), the channel can be used to make payments via Hashed Time Locked Contracts.

Changes are sent in batches: one or more `update_` messages are sent before a
`commitment_signed` message, as in the following diagram:
Expand Down Expand Up @@ -1367,11 +1423,12 @@ The sending node:
A node:
- if `next_commitment_number` is 1 in both the `channel_reestablish` it
sent and received:
- MUST retransmit `funding_locked`.
- MUST retransmit `channel_ready`.
- otherwise:
- MUST NOT retransmit `funding_locked`.
- MUST NOT retransmit `channel_ready`, but MAY send `channel_ready` with
a different `short_channel_id` `alias` field.
- upon reconnection:
- MUST ignore any redundant `funding_locked` it receives.
- MUST ignore any redundant `channel_ready` it receives.
- if `next_commitment_number` is equal to the commitment number of
the last `commitment_signed` message the receiving node has sent:
- MUST reuse the same commitment number for its next `commitment_signed`.
Expand Down Expand Up @@ -1439,7 +1496,7 @@ atomic: if it doesn't complete, it starts again. The only exception
is if the `funding_signed` message is sent but not received. In
this case, the funder will forget the channel, and presumably open
a new one upon reconnection; meanwhile, the other node will eventually forget
the original channel, due to never receiving `funding_locked` or seeing
the original channel, due to never receiving `channel_ready` or seeing
the funding transaction on-chain.

There's no acknowledgment for `error`, so if a reconnect occurs it's
Expand Down Expand Up @@ -1475,7 +1532,7 @@ commitment number 0 is created during opening.
`commitment_signed` for commitment number 1 is send and then
the revocation for commitment number 0 is received.

`funding_locked` is implicitly acknowledged by the start of normal
`channel_ready` is implicitly acknowledged by the start of normal
operation, which is known to have begun after a `commitment_signed` has been
received — hence, the test for a `next_commitment_number` greater
than 1.
Expand Down
9 changes: 9 additions & 0 deletions 04-onion-routing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,15 @@ An _intermediate hop_ MUST NOT, but the _final node_:
- if the `amt_to_forward` does NOT correspond with the `incoming_htlc_amt` from the
final node's HTLC:
- MUST return a `final_incorrect_htlc_amount` error.
- if it returns a `channel_update`:
- MUST set `short_channel_id` to the `short_channel_id` used by the incoming onion.

### Rationale

In the case of multiple short_channel_id aliases, the `channel_update`
`short_channel_id` should refer to the one the original sender is
expecting, to both avoid confusion and to avoid leaking information
about other aliases (or the real location of the channel UTXO).

## Receiving Failure Codes

Expand Down
12 changes: 7 additions & 5 deletions 07-routing-gossip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ The `announcement_signatures` message is created by constructing a `channel_anno
A node:
- if the `open_channel` message has the `announce_channel` bit set AND a `shutdown` message has not been sent:
- MUST send the `announcement_signatures` message.
- MUST NOT send `announcement_signatures` messages until `funding_locked`
- MUST NOT send `announcement_signatures` messages until `channel_ready`
has been sent and received AND the funding transaction has at least six confirmations.
- otherwise:
- MUST NOT send the `announcement_signatures` message.
Expand All @@ -104,8 +104,8 @@ A recipient node:
`error` and fail the channel.
- if it has sent AND received a valid `announcement_signatures` message:
- SHOULD queue the `channel_announcement` message for its peers.
- if it has not sent funding_locked:
- MAY defer handling the announcement_signatures until after it has sent funding_locked
- if it has not sent `channel_ready`:
- MAY defer handling the announcement_signatures until after it has sent `channel_ready`
- otherwise:
- MUST ignore it.

Expand Down Expand Up @@ -167,7 +167,7 @@ The origin node:
- for the _Bitcoin blockchain_:
- MUST set `chain_hash` value (encoded in hex) equal to `6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000`.
- MUST set `short_channel_id` to refer to the confirmed funding transaction,
as specified in [BOLT #2](02-peer-protocol.md#the-funding_locked-message).
as specified in [BOLT #2](02-peer-protocol.md#the-channel_ready-message).
- Note: the corresponding output MUST be a P2WSH, as described in [BOLT #3](03-transactions.md#funding-transaction-output).
- MUST set `node_id_1` and `node_id_2` to the public keys of the two nodes
operating the channel, such that `node_id_1` is the lexicographically-lesser of the
Expand Down Expand Up @@ -445,10 +445,12 @@ or `node_id_2` otherwise.
### Requirements

The origin node:
- MUST NOT send a created `channel_update` before `funding_locked` has been received.
- MUST NOT send a created `channel_update` before `channel_ready` has been received.
- MAY create a `channel_update` to communicate the channel parameters to the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once the announcement_signatures has been exchanged, each side should send the channel_update with the real SCID to each other since prior to this point, they might have been using alias SCIDs in the channel_update. Unless this is already implied since both sides will broadcast (to each other, the network) the "real" channel_update?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason not to add it, but we could also do it by just changing even though the channel has not yet been announced (...) to even if the channel has not yet been announced (i.e. the announce_channel bit was not set or the channel funding transaction has not yet reached six confirmation

channel peer, even though the channel has not yet been announced (i.e. the
`announce_channel` bit was not set).
- MUST set the `short_channel_id` to either an `alias` it has
received from the peer, or the real channel `short_channel_id`.
Comment on lines +452 to +453
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- MUST set the `short_channel_id` to either an `alias` it has
received from the peer, or the real channel `short_channel_id`.
- MUST set the `short_channel_id` to either an `alias` it has
sent to the peer, or the real channel `short_channel_id`.

Here is the rationale from @rustyrussell (#910 (comment)):

I believe that when you send a private channel_update, you should use the peers scid alias. Reasons are simple: they committed to remember it, and you don't actually ever send this to anyone anyway (they just use the info to populate route hints).

That's really confusing I think. In your example below: if B is the alias set by Bob for Bob->Alice, then B is what Bob should use in the channel_update for Bob->Alice. Otherwise the same alias points to one direction (in the routing hint) and the opposite direction (in a payment error)

If Alice calls the channel A, and Bob calls it B, and Alice uses B in a routehint for Carol, if Carol gets the fee wrong it's Bob who will be the one to respond with a channel_update, and he will use B in that naturally (it should specify that nodes use the scid used by the sender in this case: this is no longer clear once we have aliases!).

I don't see what the problem is when we have multiple aliases. Bob should return whichever alias that was used by C in the payment onion (he committed to remember all of them too).

Curious how you have implemented it @TheBlueMatt? Did you use the sender or receiver alias for channel_update?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's really confusing I think. In your example below: if B is the alias set by Bob for Bob->Alice, then B is what Bob should use in the channel_update for Bob->Alice. Otherwise the same alias points to one direction (in the routing hint) and the opposite direction (in a payment error)

Its confusing in that you have to generate channel updates differently based on the context, but you already likely have to do that to handle the different cases for onions now anyway - using real scids for public channels and refusing to do so for private ones.

Curious how you have implemented it @TheBlueMatt? Did you use the sender or receiver alias for channel_update?

The receiver's alias makes sense here - the receiver isn't otherwise required to understand the sender's alias at all, and moreso its an optional TLV so we have to assume they don't know how to interpret it at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its confusing in that you have to generate channel updates differently based on the context

No, it's confusing because the same identifier sometimes points to one direction of the channel, sometimes to the other direction of the same channel. Or am I missing something?

Here is my understanding of the spec:

    A                            B
    |                            |
    |  channel_ready(alias=aaa)  |
    |--------------------------->|
    |                            |
    |  channel_ready(alias=bbb)  |
    |<---------------------------|
    |                            |

Spec says:

  • Alice invoice uses bbb
  • Bob channel_update sent to Alice to help her fill her invoice uses aaa (*)
  • Bob channel_update sent to everyone else (to return errors when he can't forward payments to Alice) uses bbb
  • Bob invoice uses aaa
  • Alice channel_update sent to Bob to help him fill his invoice uses bbb
  • Alice channel_update sent to everyone else (to return errors when she can't forward payments to Bob) uses aaa (*)

(*) Note how aaa identifies channel_updates for both sides of the channel

I say:

  • Alice invoice uses bbb
  • Bob channel_update always uses bbb
  • Bob invoice uses aaa
  • Alice channel_update always uses aaa

the receiver isn't otherwise required to understand the sender's alias at all

But it does: the receiver uses the sender's alias in the route hints of their invoices. "While a node can send multiple alias, it must remember all of the ones it has sent so it can use them should they be requested by incoming HTLCs. The recipient only need remember one, for use in r route hints in BOLT 11 invoices." (BOLT 7)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the receiver doesn't have to use the alias in their invoices at all, if they don't want to (without the scid_privacy channel type set). This is, again, consistent with the SCID alias being an optional TLV, with no feature bit assigned to describing its presence.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we are optimizing for the case where both the sender and the receiver understand aliases, but neither of option_scid_privacy or option_zeroconf are enabled, and the alias isn't used for anything except in the channel_update, and in a way that creates the confusion I layed out above? I don't know why we would want that.

In my opinion we should instead:

  • if option_scid_privacy or option_zeroconf is enabled: have each side always use their own alias for their own channel_update
  • otherwise: use the real scid for channel_updates.

This too would be consistent with the alias being an optional TLV: in the latter case the sender is always free to set the alias, and the receiver may or may not use it in their invoice.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If option_scid_privacy or option_zeroconf is negotiated, you use the scid alias in the channel_update because you know that the peer supports it and they must remember it.

Or we could not add any branching based on feature flags, no complicated logic to figure out which one to use and just keep it as-is :). I'm not really sure why you're advocating this - yea, its a bit awkward from a theoretical perspective, but the way its written now leaves the code much, much simpler.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will be branching anyway: either based on feature flags (I'd argue they are built for that purpose), or based on context (depending on what the channel_update is built for). Anyway I think both positions are pretty clear let's see what others think 🤓.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, apologies, using only "option_scid_privacy or option_zeroconf" doesn't work - you're totally allowed to start using a channel at 0conf even without option_zeroconf. This is in fact an important use-case because I believe only lnd is gonna bother using option_zeroconf, others can/will simply send a channel_ready without any confirmations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're totally allowed to start using a channel at 0conf even without option_zeroconf

True, but even in that case, both sides need to exchange their channel_ready before the channel is used (BOLT 2), which allows each side to tell whether the other side understand aliases. So we should indeed branch on the presence of the alias tlv instead on branching on the feature bits to decide whether using the sender's scid alias or the real scid for channel_update. It is still strictly simpler than using the receiver's scid alias, which requires the same branching + makes the scid context-dependent for the channel_update.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which allows each side to tell whether the other side understand aliases

Not currently, you'd be adding an assumption that if a peer sent you an alias they also stored the alias you sent them, which is a new assumption that doesn't seem like it needs to make?

It is still strictly simpler than using the receiver's scid alias, which requires the same branching + makes the scid context-dependent for the channel_update.

This doesn't match up with our implementation experience at all. On the sending side we just do chan.get_short_channel_id().or(chan.latest_inbound_scid_alias()) which is nice and clean. On the receiving side, we use the same map we use for outbound relay - we already (obviously) have to have some listing of SCIDs that includes real and outbound-aliases in one place, we currently do not anywhere have a listing of real and inbound-alias SCIDs anywhere. Your proposed change would mandate we store a new hashmap for SCID aliases for the inbound side.

- MUST NOT forward such a `channel_update` to other peers, for privacy
reasons.
- Note: such a `channel_update`, one not preceded by a
Expand Down
3 changes: 3 additions & 0 deletions 09-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ The Context column decodes as follows:
| 22/23 | `option_anchors_zero_fee_htlc_tx` | Anchor commitment type with zero fee HTLC transactions | IN | `option_static_remotekey` | [BOLT #3][bolt03-htlc-tx], [lightning-dev][ml-sighash-single-harmful]|
| 26/27 | `option_shutdown_anysegwit` | Future segwit versions allowed in `shutdown` | IN | | [BOLT #2][bolt02-shutdown] |
| 44/45 | `option_channel_type` | Node supports the `channel_type` field in open/accept | IN | | [BOLT #2](02-peer-protocol.md#the-open_channel-message) |
| 46/47 | `option_scid_alias` | Supply channel aliases for routing | IN | | [BOLT #2][bolt02-channel-ready] |
| 48/49 | `option_payment_metadata` | Payment metadata in tlv record | 9 | | [BOLT #11](11-payment-encoding.md#tagged-fields)
| 50/51 | `option_zeroconf` | Understands zeroconf channel types | IN | `option_scid_alias` | [BOLT #2][bolt02-channel-ready] |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC we can have zero-conf public channels, so there is no dependency link between the features (even if they both rely on aliases under the hood)

Suggested change
| 50/51 | `option_zeroconf` | Understands zeroconf channel types | IN | `option_scid_alias` | [BOLT #2][bolt02-channel-ready] |
| 50/51 | `option_zeroconf` | Understands zeroconf channel types | IN | | [BOLT #2][bolt02-channel-ready] |

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no dependency link, how do you enforce aliases are used for zero-conf channels? Is this not the point of option-scid-alias feature bit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the purpose of option_scid_alias is to "insist that the alias always be used, never the real SCID, for privacy" as Rusty puts it.

That's why Matt proposed to rename it option_scid_privacy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency here only applies to the init and node_announcement contexts. There is no such dependency in the channel type contexts. Seems to me like it makes perfect sense to have an scid-alias dependency in those contexts.

Copy link
Collaborator

@pm47 pm47 Apr 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that we rename option_scid_alias to option_scid_privacy (as you suggested and I agree with you).

The two features introduced by this PR: option_scid_privacy and option_zeroconf, are very different and independent from each other. We can have one without the other. It just happens that they both depend on supporting scid alias, but scid alias is just an internal mechanism that doesn't need a feature bit, just like e.g. option_payment_metadata depends on supporting tlvs.

For example, we at eclair may implement option_zeroconf but go with blinded routes for privacy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, i guess I was interpreting the bit in different contexts differently - means one thing in init, a different thing as a channel type. If we want to assign it no meaning in the init (aside from supporting the channel type) then I agree with you. I don't believe we enforce the dependency currently so our current code will be happy either way.

Copy link
Collaborator

@cdecker cdecker Apr 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we ever announce a channel, if setting the option_scid_alias literally forces us to set the announce_channel flag in the open_channel message, and that flag is impossible to renegotiate once the negotiation is done? I think this literally makes public/announced channels and zeroconf mutually exclusive.

Oh I see, it can still be negotiated as a blank feature, not as part of the channel_type at all. Always re-read things twice before commenting 😊

https://github.com/lightning/bolts/pull/910/files#diff-ed04ca2c673fd6aabde69389511fa9ee60cb44d6b2ef6c88b549ffaa753d6afeR246-R247


## Definitions

Expand Down Expand Up @@ -90,6 +92,7 @@ This work is licensed under a [Creative Commons Attribution 4.0 International Li
[bolt02-open]: 02-peer-protocol.md#the-open_channel-message
[bolt03-htlc-tx]: 03-transactions.md#htlc-timeout-and-htlc-success-transactions
[bolt02-shutdown]: 02-peer-protocol.md#closing-initiation-shutdown
[bolt02-channel-ready]: 02-peer-protocol.md#the-channel_ready-message
[bolt04]: 04-onion-routing.md
[bolt07-sync]: 07-routing-gossip.md#initial-sync
[bolt07-query]: 07-routing-gossip.md#query-messages
Expand Down