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.

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

extension-bolt: taproot gossip (features 32/33) #1059

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ellemouton
Copy link
Contributor

Overview

The initial version of the Lightning Network gossip protocol as defined in
BOLT 7 was designed around P2WSH funding transactions. For these
channels, the channel_announcement message is used to advertise the
channel to the rest of the network. Nodes in the network use the content
of this message to prove that the channel is sufficiently bound to the
Lightning Network context and that it is owned by the nodes advertising
the channel. This proof and verification protocol is, however, not
compatible with SegWit V1 (P2TR) outputs and so cannot be used to
advertise the channels defined in the Simple Taproot Channel proposal.
This document thus aims to define an updated gossip protocol that will
allow nodes to both advertise and verify taproot channels. This part of the
update affects the announcement_signatures and
channel_announcement messages.

The opportunity is also taken to rework the node_announcement and
channel_update messages to take advantage of BIP-340 signatures and
TLV fields. Timestamp fields are also updated to be block heights instead of
Unix timestamps.

@ellemouton ellemouton marked this pull request as draft March 16, 2023 14:35
@ellemouton
Copy link
Contributor Author

Thanks to @rustyrussell , @joostjager & @GeorgeTsagk for the initial set of comments on the pre-draft PR! I will carry over some of the comments over to this PR so that a discussion can take place :)

Copy link
Contributor Author

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

Carry-over comments from the pre-draft PR

This document aims to update the gossip protocol defined in [BOLT 7][bolt-7] to
allow for advertisement and verification of taproot channels. An entirely new
set of gossip messages are defined that use [BIP-340][bip-340] signatures and
that use a purely TLV based schema.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Rusty:

The problem with TLV is that you now need everyone to check that compulsory fields are present. But some fields don't actually make sense as optional. We end up needing another version anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the simplicity of using tlv for everything might be more future proof & less complicated in the end. For example, if we decide on a compulsory field now but then later on add a new one, it will have to be TLV and then we have to have 2 ways of checking that compulsory fields are present vs something clean like tlvstream.MustHaveTypes(1, 2, 3, 4, ...)

We end up needing another version anyway.

This is true but by using TLV for everything, adding a new version due to the change of 1 compulsory field becomes easier to define in the spec I think. Instead of needing to define a whole new message, can just say "drop this tlv field and add this other tlv field".

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can't though, because that would not be backwards compatible :( So you actually do have compulsory fields, which cannot be removed, until everyone upgrades.

It's actually simpler to define a new type in that case!

Copy link
Collaborator

Choose a reason for hiding this comment

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

is that you now need everyone to check that compulsory fields are present. But some fields don't actually make sense as optional

Isn't this already the case for all the other TLV extensions we've added over time?

I think one scenario that optional fields can be useful is if say in the channel announcements, we allow the advertiser to include/omit information that enables verification of complete channel output binding (so the individual keys, and tweak -- which would just be the bip86 tweak if that was used).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this already the case for all the other TLV extensions we've added over time?

Yeah plus one for this question. If we later add compulsory fields, they would be via TLV and there would be some feature bit associated with understanding the version of the message that includes this field. So the benefit of doing only TLV is that all mandatory fields (ones added now and ones we add later on) are checked in the same way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like some of the lessons from protobufs: the message gets defined completely separately from the business logic (ie the validation checking if all the wanted fields are there) and then it is always possible to add more fields or remove any fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps worth a quick discussion at the spec meeting.

Comment on lines 136 to 145
For legacy channels, these last two proofs are made possible by including four
separate signatures in the `channel_announcement`. However, [BIP-340][bip-340]
signatures and the MuSig2 protocol allow us to now aggregate these four
signatures into a single one. Verifiers will then be able to aggregate the four
keys (`bitcoin_key_1`, `bitcoin_key_2`, `node_ID_1` and `node_ID_2`) using
MuSig2 key aggregation and then they can do a single signature verification
check instead of four individual checks.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Rusty:

Can we weaken this (and future proof it) a little, by publishing only the taproot_output_key? Sure, that means any taproot output can pretend to be a channel, but this is also a feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is some benefit to proving a binding to the LN context at least a little bit.

We could possibly just advertise the taproot_internal_key though (although I need to check the musig verification math for this). Kinda nice to, at least initially, to prove that the output cant be spent via the script path (via BIP86) as this at least gives spammers some deterrence... although perhaps not enough - interested to hear what people think re preventing spam of non-channels in the "only advertise the taproot_output_key" case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that perhaps the "only announce taproot_output_key" scenario only works in full Gossip 2.0 where you have to prove you have some bitcoin in order to announce channels. But in this proposal (gossip 1.5), I think that might not be the case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gossip v2 detached the proof-of-utxo from the channels in two ways:

  1. You could use any UTXO proofs
  2. The proofs were per-node, and not per-channel.

This is the former, without the latter. It allows for us to change to a different Taproot scheme in future if we wanted, without updating gossip, but still requires a proof-per-channel. Say, gossip 1.75?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It allows for us to change to a different Taproot scheme in future if we wanted

Could you perhaps give an example? Do you mean like: we could possibly use the same gossip for a future channel factory or something in which case the internal key itself would be derived in a different way? (ie, something other than a 2-of-2 multisig)

If that is the case, then how about we make the "binding level" optional. Ie, we make the bitcoin_key_1, bitcoin_key_2 and tapteak fields optional and then if they are present, nodes can choose to be more strict with their verification or not.

Comment on lines 463 to 467
- MUST set `chain_hash` to the 32-byte hash that uniquely identifies the chain
that the channel was opened within:
- for the _Bitcoin blockchain_:
- MUST set `chain_hash` value (encoded in hex) equal
to `6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Rusty:

I suggest the same trick we use for offers: omitting this field means it is 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000. And you must omit the field if it would be 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000 (remember: both sides need to agree on exactly what this message looks like!).

Note also: I like the use of the TLV here. Because if we want to later, we can adapt this for pre-taproot channels, which would require more / different sigs. Though maybe they'll all close by the time v1-apocolypse is here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool - yeah I like the idea.
I have heard a few oppositions to assuming defaults though so just gonna leave this thread open to leave room for people to discuss. But personally, I dont see why a default is that bad.

9. subtype: `dns_hostname`
10. data:
* [`...*utf8`:`hostname`]
* [`tu16`:`port`]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Rusty:

And if port is 0, say it's 9735?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here as for the above re defaults

2. data (tlv_stream):
1. type: 0 (`channel_id`)
2. data:
* [`channel_id`:`channel_id`]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Rusty:

I would definitely not use a TLV for channel_id. Every other message starts with channel_id, we should keep it fixed. The other two fields, well, they're OK, but it seems weird that they're optional.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, especially since this is not a gossip message, but a peer message which is much easier to upgrade.

verify taproot channels. This part of the update affects the
`announcement_signatures` and `channel_announcement` messages.

The opportunity is also taken to rework the `node_announcement` and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Joost:

Could you also not take this opportunity and keep using ecdsa signatures for those? That seems to reduce the delta of this proposal significantly, and maybe without loss of functionality for the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we could definitely do this. but:

  • getting to use schnorr sigs for them is quite beneficial for things like multi-key backed nodes
  • see the section on "Bootstrapping Taproot Gossip", it is not necessary for nodes to use the new announcement message straight away. They can use the old one for as long as they like.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah don't see a good reason to not just use schnorr everywhere. For node anns, this means you can actually create a composite node that's actually composed of multiple entities, either via musig2 or FROST (tho FORST integration at the channel level has some other implications re shachain).

signatures at the time of spending the output. This extra cost provided some
extra deterrence for attackers. This deterrence is not present with P2TR
transactions.
3. The owners of `bitcoin_key_1` and `bitcoin_key_2` agree to be associated with
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From George:

Since the public announcements of channels pretty much reveal what the output is being used for, would it be fair to mention that this mainly benefits private channels?

Different phrasing: Taproot outputs all do look the same regardless of being used for a channel or not, but any participant in LN will eventually receive a (public) channel announcement that marks some taproot UTXO as a channel, so effectively it's only the private channels that can remain "incognito"

To also prevent nodes from building up too large of a burst-buffer with which
they can spam the network and to give a limit to how low the block height on a
`node_announcement_2` can be: nodes should not be allowed to use a block height
smaller 1440 (~ a day worth of blocks) below the current block height.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this spam prevention works in all scenarios:

  • A has a block height of 800,144 and sends B a channel_update_2 with height 800,000
  • B has a block height of 800,145 and rejects the channel_update_2

Copy link

@GeorgeTsagk GeorgeTsagk Mar 20, 2023

Choose a reason for hiding this comment

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

That would be a big issue if implementations defaulted on using the lowest possible block in order to keep a high update budget.

Couldn't this be resolved on the implementation side? (i.e. use a height of -143 blocks instead of -144 to avoid this race condition)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point, a buffer of blocks could be used here to avoid this issue

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can't do this, since you can't tell if it's old or deliberately backdated. The two weeks comes from the fact that we expire gossip messages after that time currently, anyway. You're supposed to update rebroadcast say every 13 days, which is a compromise between spamminess and proof-of-life.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't do this

are you referring to the idea of the burst-buffer in general?

since you can't tell if it's old or deliberately backdated

does it matter? surely we can just say it is not old if it is the highest height that we have seen? (just like the current rules for "timestamp" used in current gossip)/

To also prevent nodes from building up too large of a burst-buffer with which
they can spam the network and to give a limit to how low the block height on a
`node_announcement_2` can be: nodes should not be allowed to use a block height
smaller 1440 (~ a day worth of blocks) below the current block height.

Choose a reason for hiding this comment

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

Suggested change
smaller 1440 (~ a day worth of blocks) below the current block height.
smaller 144 (~ a day worth of blocks) below the current block height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah - silly me. I actually meant to say 2 weeks worth of blocks (which would be 2016). i must have confused myself here 🙃

It is also semi arbitrary. We need to look at how fast gossip tends to spread

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

I don't think we should even consider redoing gossip unless we take the opportunity to build in the obvious trivial privacy improvements.


## Timestamp fields

In this document, the `timestamp` fields in messages are block heights instead
Copy link
Collaborator

Choose a reason for hiding this comment

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

Then let's call them block height?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure :)

13. type: 6 (`bitcoin_key_1`)
14. data:
* [`point`:`point`]
15. type: 7 (`bitcoin_key_2`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strong NACK on including both Bitcoin keys here - we have an opportunity to get some incredibly nontrivial privacy by letting folks being outside UTXOs for channel proofs here - setting up the verification end to support that by not including both Bitcoin keys and instead just including the aggregate that's already on chain is trivial and I don't really see why we should avoid it here. We'll have to add a protocol in the future where one node can offer an alternative UTXO and signature in the signature message, but we can do that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my response to Rusty's comment above. Basically: I think we can go full-ghost mode once we do Gossip 2.0 (overcommit stuff) but before that (gossip 1.5 aka this) we should have at least some binding to the LN context. The way the proposal is currently designed, nodes can prove that the output cant be spent along the script path. Yes, perhaps we can smush the 2 bitcoin keys into one (i would just need to check that the musig verification can still work)

Copy link
Collaborator

Choose a reason for hiding this comment

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

we should have at least some binding to the LN context

Why? Your response to Rusty's above comment doesn't seem to explain why you think we should try to tie this to lightning more, but I don't really get it - its trivial not to, and a super great outcome if we dont bind it! In general, I'd say it should be considered an explicit goal to not "bind to the lightning context", especially given there's no additional complexity to doing so, even better it (I think?) reduces the size of the gossip!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, perhaps we can smush the 2 bitcoin keys into one (i would just need to check that the musig verification can still work)

Each key is tweaked individually and then added together, so this would require providing the tweaks for the node keys and including the bitcoin keys as one with their individual tweaks pre-applied.

If we choose to go with the taproot output key, then we'll have to deal with recursive MuSig2, which I believe we're still waiting on a security proof for. I guess we'd still have to handle recursive MuSig2 if either of the node keys are also multi-sig.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically: I think we can go full-ghost mode once we do Gossip 2.0 (overcommit stuff) but before that (gossip 1.5 aka this) we should have at least some binding to the LN context. The way the proposal is currently designed, nodes can prove that the output cant be spent along the script path.

IIUC, it's important that node1 and node2 prove to each other that script path spends are impossible. But I don't see a reason we need to prove that to the rest of the network.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's important that node1 and node2 prove to each other that script path spends are impossible. But I don't see a reason we need to prove that to the rest of the network.

i don't think they need to prove this to eachother. They would not have generated the same output key & hence would not have produced the correct signatures if either node added a script path. They would not be able to hide such a path from the peer.

The reason for the "proving to the network" part is similar to today's reason for advertising the 2 keys: today we prove that the output can only be spent via a 2-of-2 multisig? this makes it a bit more difficult for an attacker to create fake spam since they are now limited in how they can spend the utxo since they would now have to pay more for spending from it (2 sigs). That disincentive is not available with taproot - so the idea here is that we at least show the network that we are limiting how the utxo can be spent in some way - by showing that we cant spend via a script path. But perhaps this is too "cosmetic" to consider sufficient in any case.

So i think the main thing we should iron out is: do we want to be able to prove (ie, do we care) that the utxo is created via a 2-of-2 musig2 or not. Perhaps a good topic for today's spec call

Copy link
Contributor

Choose a reason for hiding this comment

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

That disincentive is not available with taproot - so the idea here is that we at least show the network that we are limiting how the utxo can be spent in some way - by showing that we cant spend via a script path. But perhaps this is too "cosmetic" to consider sufficient in any case.

The "no-script-path" limitation seems insignificant to me. I suspect most P2TR outputs today don't have a script path anyway.

So i think the main thing we should iron out is: do we want to be able to prove (ie, do we care) that the utxo is created via a 2-of-2 musig2 or not.

In the taproot world there's no fee difference between musig and single sig UTXOs. So spam prevention doesn't seem a compelling reason to prove a UTXO is a musig.

It seems to me that proof-of-UTXO is a "good enough" spam prevention. In fact, we're talking about moving to overcommit in the future, which would be a weaker spam prevention than is being proposed right here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep - so I think the next version is gonna make this tweak optional. So by default, nodes will just use the output key found on chain & then if nodes want to include things like the 2 keys used in the musig2 & if they want to add a tweak then they can do so optionally.

* [`chain_hash`:`chain_hash`]
7. type: 3 (`short_channel_id`)
8. data:
* [`short_channel_id`:`short_channel_id`]
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should have an option for a channel to not include an SCID at all, rather allowing a node to overcommit their balance by simply saying "I already have channel(s) that have utxos, let me announce more without a proof up to multiplier N by not including a proof on this channel".

This would also imply that we should move the UTXO profs to the node announcements, but we don't strictly have to - it works fine without that.

We'll have to define a protocol to negotiate such an announcement, but that can happen later and is rather trivial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO we should have an option for a channel to not include an SCID at all

since it is TLV, it can always be made optional in future.

rather allowing a node to overcommit their balance

I think you are referring to the full on Gossip 2.0 proposal. This proposal is the more vanilla gossip 1.5 that was discussed in oakland.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just make it optional now? There's a bunch of complexity to adding the negotiation of building gossip with overcommit, so I wouldn't expect to actually build that now, but there's very little complexity to allowing overcommit now (with the exception of bikeshedding over the ratio 😂, but I'm open to someone rolling a die :)), so it seems like we should just do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's a bunch of complexity to adding the negotiation of building gossip with overcommit, so I wouldn't expect to actually build that now, but there's very little complexity to allowing overcommit now

Could you expand on the details a bit?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's two ways to do overcommit on the receiver's-side, one a trivial modification of this existing logic, the other more "clean", but a bit more work.

In the simplest version, we (a) make the bitcoin point optional, if its not provided (b) an extra bit has to be set somewhere indicating if this channel should count against the overcommit of node_1 or node_2, and (c) the short_channel_id should be greater than some number far, far into the future as an indication that its not a real channel (and to ensure receiving gossip always still works by making sure channels with utxo proofs come "before" channels without in scid-order).

Then we'd have to define some overcommit ratio (let's roll a dice or bikeshed for months), and a receiver of such a message must track the current amount of bitcoin in utxo-proven channels for each node in their graph, as well as the amount of channels which did not have a utxo proof, and if the utxo-proven channel amount is < amount of non-utxo-proven channels * the overcommit ratio the channels with the lowest (highest? whatever) fake scids are removed.

The alternative way to do this which is maybe cleaner but more of an overhaul would be to move the UTXO proofs entirely outside of the channel announcements and into the node announcements, relying exclusively on the overcommit format described above in channel announcements, and calculating against the total proven in the node announcement.

There's no need to describe the messages to actually negotiate this new format in announcement_signatures (its not complicated, we could, but if you're trying to reduce new things here its fine not to, IMO), but I think its important to actually get it on the validating/receiving end sooner rather than later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we'd have to define some overcommit ratio (let's roll a dice or bikeshed for months), and a receiver of such a message must track the current amount of bitcoin in utxo-proven channels for each node in their graph, as well as the amount of channels which did not have a utxo proof, and if the utxo-proven channel amount is < amount of non-utxo-proven channels * the overcommit ratio the channels with the lowest (highest? whatever) fake scids are removed.

Should the overcommit amount depend on bitcoin amount in proven channels, or on number of proven channels?

Chain fees scale with number of proven channels, but not necessarily with bitcoin amount in channels. So I think number of proven channels provides stronger spam protection.

(Sorry if this was discussed in Oakland or elsewhere, I'm new here.)

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should have an option for a channel to not include an SCID at all, rather allowing a node to overcommit their balance by simply saying "I already have channel(s) that have utxos, let me announce more without a proof up to multiplier N by not including a proof on this channel".

This is a somewhat unsettling proposition that I'm encountering for the first time. Has this been discussed anywhere as to why people think it's a good idea? I find it unusual that we are suggesting things that resemble leverage/fractional reserve at the protocol level in Bitcoin.


A node can be assumed to understand the new gossip v2 messages if:

- They advertise `option_taproot_gossip` in a legacy `node_announcement` message
Copy link
Contributor

Choose a reason for hiding this comment

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

Or in the init message

Comment on lines +284 to +291
These extra requirements only apply if the `option_taproot` channel type is set
in the `open_channel` message.
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure everyone supports option_channel_type these days, but if either node doesn't support it then we should rely on the features shared between both nodes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO channel type becomes even more important in this scenario as one may want to still preferentially open the old segwit v0 channel with certain peers. Assuming the advertised feature bits tell the complete story would implicltly mean that the taproot chans are the new "default", which likely won't be the case for some time (eg: zero fee anchors isn't yet the default for all impls).

Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming the advertised feature bits tell the complete story would implicltly mean that the taproot chans are the new "default", which likely won't be the case for some time

It's also less future proof. Even if we want TR channels to be assumed default which @Roasbeef already points out is unlikely to be the case for a while, in the future we may have a new default preferred channel type as the network continues to develop and mature.

Comment on lines 375 to 376
- MUST send the `announcement_signatures_2` message.
- MUST NOT send `announcement_signatures_2` messages until `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.

Suggested change
- MUST send the `announcement_signatures_2` message.
- MUST NOT send `announcement_signatures_2` messages until `channel_ready`
- MUST send the `announcement_signatures_2` message once `channel_ready`

13. type: 6 (`bitcoin_key_1`)
14. data:
* [`point`:`point`]
15. type: 7 (`bitcoin_key_2`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, perhaps we can smush the 2 bitcoin keys into one (i would just need to check that the musig verification can still work)

Each key is tweaked individually and then added together, so this would require providing the tweaks for the node keys and including the bitcoin keys as one with their individual tweaks pre-applied.

If we choose to go with the taproot output key, then we'll have to deal with recursive MuSig2, which I believe we're still waiting on a security proof for. I guess we'd still have to handle recursive MuSig2 if either of the node keys are also multi-sig.

bolt-taproot-gossip.md Outdated Show resolved Hide resolved
bolt-taproot-gossip.md Outdated Show resolved Hide resolved

## Query Messages

TODO: any updates that need to happen to this section?
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll likely want to extend query_channel_range and gossip_timestamp_filter with new bits to specify if we should only receive legacy, taproot, or both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true 👍 will work on that section once there is more consensus around the general gossip direction

bolt-taproot-gossip.md Outdated Show resolved Hide resolved
bolt-taproot-gossip.md Outdated Show resolved Hide resolved
* [`chain_hash`:`chain_hash`]
7. type: 3 (`short_channel_id`)
8. data:
* [`short_channel_id`:`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.

Then we'd have to define some overcommit ratio (let's roll a dice or bikeshed for months), and a receiver of such a message must track the current amount of bitcoin in utxo-proven channels for each node in their graph, as well as the amount of channels which did not have a utxo proof, and if the utxo-proven channel amount is < amount of non-utxo-proven channels * the overcommit ratio the channels with the lowest (highest? whatever) fake scids are removed.

Should the overcommit amount depend on bitcoin amount in proven channels, or on number of proven channels?

Chain fees scale with number of proven channels, but not necessarily with bitcoin amount in channels. So I think number of proven channels provides stronger spam protection.

(Sorry if this was discussed in Oakland or elsewhere, I'm new here.)

13. type: 6 (`bitcoin_key_1`)
14. data:
* [`point`:`point`]
15. type: 7 (`bitcoin_key_2`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically: I think we can go full-ghost mode once we do Gossip 2.0 (overcommit stuff) but before that (gossip 1.5 aka this) we should have at least some binding to the LN context. The way the proposal is currently designed, nodes can prove that the output cant be spent along the script path.

IIUC, it's important that node1 and node2 prove to each other that script path spends are impossible. But I don't see a reason we need to prove that to the rest of the network.

This document aims to update the gossip protocol defined in [BOLT 7][bolt-7] to
allow for advertisement and verification of taproot channels. An entirely new
set of gossip messages are defined that use [BIP-340][bip-340] signatures and
that use a purely TLV based schema.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that you now need everyone to check that compulsory fields are present. But some fields don't actually make sense as optional

Isn't this already the case for all the other TLV extensions we've added over time?

I think one scenario that optional fields can be useful is if say in the channel announcements, we allow the advertiser to include/omit information that enables verification of complete channel output binding (so the individual keys, and tweak -- which would just be the bip86 tweak if that was used).

[BOLT 7][bolt-7] was designed around P2WSH funding transactions. For these
channels, the `channel_announcement` message is used to advertise the channel to
the rest of the network. Nodes in the network use the content of this message to
prove that the channel is sufficiently bound to the Lightning Network context
Copy link
Collaborator

Choose a reason for hiding this comment

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

Explicitly mention that the LN context here meant a 2-of-2 p2wsh script? Then also the cross signing on the peer layer.

verify taproot channels. This part of the update affects the
`announcement_signatures` and `channel_announcement` messages.

The opportunity is also taken to rework the `node_announcement` and
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah don't see a good reason to not just use schnorr everywhere. For node anns, this means you can actually create a composite node that's actually composed of multiple entities, either via musig2 or FROST (tho FORST integration at the channel level has some other implications re shachain).

along with a [BIP86 tweak][bip86-tweak]. This provides a slightly weaker
binding to the LN context than legacy channels do but at least somewhat
limits how the output can be spent due to the script-path being disabled.
The context binding with legacy channels is greater because an attacker
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe more accurate to say faking a signal with the legacy protocol is more expensive due to additional on chain fees? Basically as a side effect of the keyspend path reducing on chain fees for cooperative contract paths.


### Rate Limiting

A block height based timestamp results in more natural rate limiting for gossip
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty cool how we can indirectly gain global rate limiting by using block hashes. I think this has a few implications for the ways channel_updates work in the network today:

  • Any attempts to use max_htlc to leak out the available bandwidth for a channel (so increase/decrease with each forward) can be rate limited at a global level.
  • Protocols that would directly require a node to trigger a specific channel update are now less attractive from an incentives PoV as: those updates would eat into the node's available set of global updates within a block interval.
  • Nodes can still do more rapid updates by sending their new channel_update in an onion error directly to the sender.

Also some implementations have extensions to the gossip query protocol to support timestamp channel update syncing, which can now be updated to use block height based syncing instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any attempts to use max_htlc to leak out the available bandwidth for a channel (so increase/decrease with each forward) can be rate limited at a global level

do you mean that some nodes explicitly try to leak their own available bandwidth via the use of a channel_update message?

Comment on lines +284 to +291
These extra requirements only apply if the `option_taproot` channel type is set
in the `open_channel` message.
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO channel type becomes even more important in this scenario as one may want to still preferentially open the old segwit v0 channel with certain peers. Assuming the advertised feature bits tell the complete story would implicltly mean that the taproot chans are the new "default", which likely won't be the case for some time (eg: zero fee anchors isn't yet the default for all impls).


1. `tlv_stream`: `channel_ready_tlvs`
2. types:
1. type: 0 (`announcement_node_pubnonce`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's possible to only send a single (pre aggregated nonce) here, so then the final step would be just combining 2 signatures instead of 4. IIUC, this would require nested musig2 though, which I don't think any one has actually implemented in full.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok cool I will need to look into nested musig2


### `channel_ready` Extensions

These extensions only apply if the `option_taproot` channel type was set in the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should specify that these values only need to be attached if an announcement signatures message hasn't already been full constructed.

We also need to think about the restart implications here as well. Eg: lnd today will sign then stash the ann_sig message to read from disk to send to the remote party. However if each time we reconnect a new set of nonces is exchanged, then we'll need to wait for chan_reest to be sent (channel_ready isn't always sent once a channel is fully operational and has had updates), then we'll need to be ready to regen+ sign again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should specify that these values only need to be attached if an announcement signatures message hasn't already been full constructed.

What do you mean? When would one be in a situation where the announcment sig message is fully constructed but you then need to send channel_ready again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then we'll need to wait for chan_reest to be sent (channel_ready isn't always sent once a channel is fully operational and has had updates), then we'll need to be ready to regen+ sign again.

Ok, so you are saying we might need to add these fields to chan_reest as-well?

two nonces. The `announcement_node_nonce` is for `node_ID_x` and the
`announcement_bitcoin_nonce` is for `bitcoin_key_x`.

Since the channel can only be announced once the `channel_ready` messages have
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment above: channel_ready isn't always sent on reconnect. So if a channel is used before the gossip advertisement is finished, then the nonces won't be available to advertise.

* [`point`:`node_id`]
9. type: 4 (`color`)
10. data:
* [`rgb_color`:`rgb_color`]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe remove the color field? Don't think it was ever seriously used....

On the other hand, if we go TLV everywhere, then it matters less as including it is optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've made it an odd number so it is optional :)

@ellemouton ellemouton force-pushed the bolt-taproot-gossip branch 2 times, most recently from 4d9883a to 534af57 Compare June 5, 2023 14:22
Copy link
Contributor Author

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

Thanks for the initial review everyone!

Made some of the more minor updates. I'm gonna wait until after the spec meeting in NYC before making any changes regarding the channel proofs.

Here is a small list of things that need to be answered:

  • do we want to go full TLV or does that just not make sense?
  • should we use recursive musig2 for the announcement sigs or leave it as a 4-or-4 for now with perhaps leaving enough wiggle room to upgrade to recursive musig2 in future.
  • are we ok with only announcing the SCID (and hence just the taproot-output-key) or do we want to at least give nodes the option of adding the bitcoin_key_1/2 and tap-tweak to the channel announcement.
  • can we assume dynamic commitments will be in place to upgrade legacy channels to taproot channels which will then be able to use the new gossip messages (in other words, does everyone agree that the new gossip messages should not support advertising legacy channels)?

This document aims to update the gossip protocol defined in [BOLT 7][bolt-7] to
allow for advertisement and verification of taproot channels. An entirely new
set of gossip messages are defined that use [BIP-340][bip-340] signatures and
that use a purely TLV based schema.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this already the case for all the other TLV extensions we've added over time?

Yeah plus one for this question. If we later add compulsory fields, they would be via TLV and there would be some feature bit associated with understanding the version of the message that includes this field. So the benefit of doing only TLV is that all mandatory fields (ones added now and ones we add later on) are checked in the same way.

Comment on lines 136 to 145
For legacy channels, these last two proofs are made possible by including four
separate signatures in the `channel_announcement`. However, [BIP-340][bip-340]
signatures and the MuSig2 protocol allow us to now aggregate these four
signatures into a single one. Verifiers will then be able to aggregate the four
keys (`bitcoin_key_1`, `bitcoin_key_2`, `node_ID_1` and `node_ID_2`) using
MuSig2 key aggregation and then they can do a single signature verification
check instead of four individual checks.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It allows for us to change to a different Taproot scheme in future if we wanted

Could you perhaps give an example? Do you mean like: we could possibly use the same gossip for a future channel factory or something in which case the internal key itself would be derived in a different way? (ie, something other than a 2-of-2 multisig)

If that is the case, then how about we make the "binding level" optional. Ie, we make the bitcoin_key_1, bitcoin_key_2 and tapteak fields optional and then if they are present, nodes can choose to be more strict with their verification or not.


### Rate Limiting

A block height based timestamp results in more natural rate limiting for gossip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any attempts to use max_htlc to leak out the available bandwidth for a channel (so increase/decrease with each forward) can be rate limited at a global level

do you mean that some nodes explicitly try to leak their own available bandwidth via the use of a channel_update message?

To also prevent nodes from building up too large of a burst-buffer with which
they can spam the network and to give a limit to how low the block height on a
`node_announcement_2` can be: nodes should not be allowed to use a block height
smaller 1440 (~ a day worth of blocks) below the current block height.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't do this

are you referring to the idea of the burst-buffer in general?

since you can't tell if it's old or deliberately backdated

does it matter? surely we can just say it is not old if it is the highest height that we have seen? (just like the current rules for "timestamp" used in current gossip)/

- Nodes are encouraged to actively connect to other nodes that advertise the
`option_taproot_gossip` feature bit as this is the only way in which they
will learn about taproot channel announcements and updates.
- Nodes should not use the new `node_announcement_2` message until they have
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they don't use node_announcement_2 at all, then none of their taproot channels will propagate.

Why not? they can still send out channel_announcement_2 . and their (legacy) node_announcement will have the feature bit in it that informs nodes that they support taproot chans which hopefully encourages those nodes to connect to them so that their channel_announcement_2 messages will propogate.

What's the rationale here?

The main reason was so that legacy nodes continue to get updates about the node while they have a channel (legacy) that mean something to the legacy node.


## Query Messages

TODO: any updates that need to happen to this section?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

true 👍 will work on that section once there is more consensus around the general gossip direction

* [`point`:`node_id`]
9. type: 4 (`color`)
10. data:
* [`rgb_color`:`rgb_color`]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've made it an odd number so it is optional :)

This document aims to update the gossip protocol defined in [BOLT 7][bolt-7] to
allow for advertisement and verification of taproot channels. An entirely new
set of gossip messages are defined that use [BIP-340][bip-340] signatures and
that use a purely TLV based schema.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like some of the lessons from protobufs: the message gets defined completely separately from the business logic (ie the validation checking if all the wanted fields are there) and then it is always possible to add more fields or remove any fields.

This document aims to update the gossip protocol defined in [BOLT 7][bolt-7] to
allow for advertisement and verification of taproot channels. An entirely new
set of gossip messages are defined that use [BIP-340][bip-340] signatures and
that use a purely TLV based schema.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps worth a quick discussion at the spec meeting.


1. `tlv_stream`: `channel_ready_tlvs`
2. types:
1. type: 0 (`announcement_node_pubnonce`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok cool I will need to look into nested musig2

@ellemouton
Copy link
Contributor Author

ellemouton commented Jun 28, 2023

note from @rustyrussell - would be good to support legacy updates in the new messages in order to take advantage of the new blockheight fields. This gives us better Spam prevention & plays better with minisketch set reconciliation

@ellemouton
Copy link
Contributor Author

ellemouton commented Jul 17, 2023

Hey everyone! Was planning on having this doc updated by today but got side tracked a bit. Will aim to have the updated changes pushed by the time of the next bi-monthly meeting.

In the meantime, here is a summary of what I plan to update as a result of the discussions from the NYC meeting. Let me know if something looks off.

1. Plan for a future where the script can be anything

Instead of requiring the channel peers to specify the two bitcoin keys in the channel_announcement_, we will instead only require them to provide a signature for the output key that is found on-chain. This means that once nested MuSig2 has a security proof and has been spec'd out, then the signature in the channel_announcement_2 can be for the key:

P_agg = MuSig2.KeyAgg(MuSig2.KeySort(`node_id_1`, `node_id_2`, `taproot_output_key`))

So to cater for this future, we can say that channel_annoucement_2 verifiers should do the following:

  • if there are no bitcoin_key_* fields in the announcement:

    • they can calculate the 3-of-3 P_agg as specified above and validate the sig accordingly.
  • else, bitcoin_key_* fields are in the announcements (tlv) along with an optional merkle_root in which case the P_agg is:

     P_agg = MuSig2.KeyAgg(MuSig2.KeySort(`node_id_1`, `node_id_2`, `bitcoin_key_1`, `bitcoin_key_2`))
    

    and the verifier also just checks that they can reconstruct what is found on-chain at the given SCID:

     P_btc_agg = MuSig2.KeyAgg(MuSig2.KeySort(`bitcoin_key_1`, `bitcoin_key_2`))
    
     P_output = P_btc_agg + tagged_hash("TapTweak", P_btc_agg || merkle_root_hash)
    
     P_output =? taproot_output
    

For the signers, this would mean the following:

  1. they can switch to using nested MuSig2 (or any other construction for that matter) at anytime as long as they can produce a valid partial sig for it.
  2. The default option for now would be to follow what is spec'ed in this doc which for now will be a flat 4-of-4 MuSig2 using node_id_1, node_id_2, bitcoin_key_1 and bitcoin_key_2.

The verifier will be able to verify both types from the get go.

2. Plan for a future where not every channel announcement contains UTXO proof

We want to allow the flexibility so that in future we dont need to tie every channel to a UTXO but instead use the rule of "here is a channel without a proof, please look at all the other open channels I have announced with a proof and use that to determine if I do infact have the allowance to open one without"

What I took away from the meeting is that to create a sort of carve-out for that future, what we will do today is to start putting the channel capacity in the channel_announcement and the verifier for now just checks that this amount is less than or equal to the output on-chain.

Let me know if I missed something here.

3. Message structures

New type definitions:

  • bip340_sig: a 64-byte bitcoin Elliptic Curve Schnorr signature as per
    [BIP-340][bip-340].
  • boolean_tlv: a zero length TLV record. If the TLV is present then true is
    implied, otherwise false is implied.
  • partial_signature: a 32-byte partial MuSig2 signature as defined
    in [BIP 327][bip-327].
  • public_nonce: a 66-byte public nonce as defined in [BIP 327][bip-327].
  • utf8: a byte as part of a UTF-8 string. A writer MUST ensure an array of
    these is a valid UTF-8 string, a reader MAY reject any messages containing an
    array of these which is not a valid UTF-8 string.

announcement_signatures_2 (no tlv here)

  1. type 260
  2. data:
    • [channel_id:channel_id]
    • [short_channel_id:short_channel_id]
    • [partial_signature:partial_signature]

channel_announcement_2

  1. type 267

  2. data:

    • [bip340_sig:bip340_sig]
    • [channel_announcement_2_tlvs:tlvs]
  3. type: 0 (chain_hash)

  4. data:

    • [chain_hash:chain_hash]
  5. type: 2 (features)

  6. data:

    • [...*byte: features]
  7. type: 4 (short_channel_id)

  8. data:

    • [short_channel_id:short_channel_id]
  9. type: 6 (node_id_1)

  10. data:

    • [point:point]
  11. type: 8 (node_id_2)

  12. data:

  • [point:point]
  1. type: 10 (bitcoin_key_1)
  2. data:
  • [point:point]
  1. type: 12 (bitcoin_key_2)
  2. data:
  • [point:point]
  1. type: 14 (merkle_root_hash)
  2. data:
    • [32*byte:hash]

node_announcement_2

  1. type 269

  2. data:

    • [bip340_sig:bip340_sig]
    • [node_announcement_2_tlvs:tlvs]
  3. tlv_stream: node_announcement_2_tlvs

  4. types:

    1. type: 0 (features)
    2. data:
      • [...*byte: features]
    3. type: 2 (block_height)
    4. data:
      • [u32: block_height]
    5. type: 4 (node_id)
    6. data:
      • [point:node_id]
    7. type: 1 (color)
    8. data:
      • [rgb_color:rgb_color]
    9. type: 3 (alias)
    10. data:
      • [...*utf8:alias]
    11. type: 5 (ipv4_addrs)
    12. data:
      • [...*ipv4_addr: ipv4_addresses]
    13. type: 7 (ipv6_addrs)
    14. data:
      • [...*ipv6_addr: ipv6_addresses]
    15. type: 9 (tor_v3_addrs)
    16. data:
      • [...*tor_v3_addr: tor_v3_addresses]
    17. type: 11 (dns_hostnames)
    18. data:
      • [...*dns_hostname: dns_hostnames]

The following subtypes are defined:

  1. subtype: rgb_color

  2. data:

    • [byte:red]
    • [byte:green]
    • [byte:blue]
  3. subtype: ipv4_addr

  4. data:

    • [u32:addr]
    • [u16:port]
  5. subtype: ipv6_addr

  6. data:

    • [16*byte:addr]
    • [u16:port]
  7. subtype: tor_v3_addr

  8. data:

    • [35*utf8:onion_addr]
    • [u16:port]
  9. subtype: dns_hostname

  10. data:

    • [u16:len]
    • [len*utf8:hostname]
    • [u16:port]

channel_update_2

  1. type 271

  2. data:

    • [bip340_sig:bip340_sig]
    • [channel_update_2_tlvs:tlvs]
  3. tlv_stream: channel_update_2_tlvs

  4. types:

    1. type: 0 (chain_hash)
    2. data:
      • [chain_hash:chain_hash]
    3. type: 2 (short_channel_id)
    4. data:
      • [short_channel_id:short_channel_id]
    5. type: 4 (block_height)
    6. data:
      • [u32:block_height]
    7. type: 6 (disable)
    8. data:
      • [boolean_tlv:disable]
    9. type: 8 (direction)
    10. data:
      • [boolean_tlv:direction]
    11. type: 10 (cltv_expiry_delta)
    12. data:
      • [u16:cltv_expiry_delta]
    13. type: 12 (htlc_minimum_msat)
    14. data:
      • [u64:htlc_minimum_msat]
    15. type: 14 (htlc_maximum_msat)
    16. data:
      • [u64:htlc_maximum_msat]
    17. type: 16 (fee_base_msat)
    18. data:
      • [u32:fee_base_msat]
    19. type: 18 (fee_proportional_millionths)
    20. data:
      • [u32:fee_proportional_millionths]

4. Legacy channel info in the new messages?

  1. It makese sense to adertise both node_announcement_2 and node_annoucement for a while
  2. Doesnt really make sense to convert old chan announcements to the new message (would also be difficult to do since would need to resign)
  3. Makes sense to use new channel_update_2 for legacy channels so we can get the easier set reconcilliation benifits due to using block height instead of timestamp.

derived by aggregating all the `node_ID` and `bitcoin_key` public keys. The
signature for this key will thus be created by aggregating (via MuSig2) four
partial signatures. One for each of the keys: `node_ID_1`, `node_ID_2`,
`bitcoin_key_1` and `bitcoin_key_2`. A nonce will need to be generated and
Copy link
Contributor

@ProofOfKeags ProofOfKeags Jul 21, 2023

Choose a reason for hiding this comment

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

Suggested change
`bitcoin_key_1` and `bitcoin_key_2`. A nonce will need to be generated and
`bitcoin_key_1` and `bitcoin_key_2`. A (MuSig2) nonce will need to be generated and

Comment on lines 348 to 349
been exchanged and since it is generally preferred to keep nonce exchanges as
Just In Time as possible, the nonces are exchanged via the `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.

Can we include a citation for the Just In Time preference? Reading BIP327, it seems to suggest in bullet 3 that it doesn't really matter how early you exchange them since it is valid to do so even before the exact signing participants have been determined.

Comment on lines +389 to +395
- otherwise:
- MUST NOT send the `announcement_signatures_2` message.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this message constructable before the 6conf requirement? If we send the announcement_signatures_2 message after channel_ready but before 6 confs, how should the peer respond?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point.

I took the original logic from the legacy announcement_signatures. But yeah - perhaps it is possible to construct it sooner as long as it isnt broadcast before 6 confs

@rustyrussell
Copy link
Collaborator

First pass just from a gossip POV:

  1. In bolt12 we used bip340sig instead of bip340_sig, like other fundamental types. Bikeshed time?
  2. Boolean tlv is awkward, can we just name the field second_peer?
  3. MsgHash should use fieldname, not type name. And I suggest renaming to signature everywhere anyway, since context is always present.
  4. type headers fixed so they are parsed correctly by tools/extract-formats.py

No doubt more feedback as we actually implement, this was just from trying to import spec into our code!

diff --git a/bolt-taproot-gossip.md b/bolt-taproot-gossip.md
index b4fbcd5..a9bf523 100644
--- a/bolt-taproot-gossip.md
+++ b/bolt-taproot-gossip.md
@@ -254,7 +254,7 @@ While the network is in the upgrade phase, the following suggestions apply:
 
 The following convenient types are defined:
 
-* `bip340_sig`: a 64-byte bitcoin Elliptic Curve Schnorr signature as
+* `bip340sig`: a 64-byte bitcoin Elliptic Curve Schnorr signature as
   per [BIP-340][bip-340].
 * `partial_signature`: a 32-byte partial MuSig2 signature as defined
   in [BIP-MuSig2][bip-musig2].
@@ -262,8 +262,6 @@ The following convenient types are defined:
 * `utf8`: a byte as part of a UTF-8 string. A writer MUST ensure an array of
   these is a valid UTF-8 string, a reader MAY reject any messages containing an
   array of these which is not a valid UTF-8 string.
-* `boolean_tlv`: a zero length TLV record. If the TLV is present then true is
-  implied, otherwise false is implied.
 
 ### Feature Bits
 
@@ -350,7 +348,7 @@ The recipient:
         - SHOULD ignore the message.
     - Otherwise:
         - MUST store the nonces so that they can be used for the partial signature
-          construction required for constructing the `announcement_signatures`
+          construction required for constructing the `announcement_signatures_2`
           message.
         - If it has not yet done so, SHOULD respond with its own `channel_ready`
           message with the nonce fields set.
@@ -390,7 +388,7 @@ A node:
     - MUST set the `partial_signature` field to the 32-byte `partial_sig` value
       of the partial signature calculated as described in [Partial Signature
       Calculation](#partial-signature-calculation). The message to be signed is
-      `MsgHash("channel_announcement", "announcement_sig", m)` where `m` is the
+      `MsgHash("channel_announcement", "signature", m)` where `m` is the
       serialisation of the `channel_announcement_2` message tlv stream (see the
       [`MsgHash`](#signature-message-construction) definition).
 - otherwise:
@@ -442,7 +440,7 @@ channel.
 
 1. type: 267 (`channel_announcement_2`)
 6. data:
-    * [`bip340_sig`:`announcement_signature`]
+    * [`bip340sig`:`signature`]
     * [`channel_announcement_2_tlvs`:`tlvs`]
 
 1. `tlv_stream`: `channel_announcement_2_tlvs`
@@ -494,9 +492,9 @@ Unlike the legacy `node_announcement` message, this message makes use of a
 BIP340 signature instead of an ECDSA one. This will allow nodes to be backed
 by multiple keys since MuSig2 can be used to construct the single signature.
 
-1. type 269
+1. type: 269 (`node_announcement_2`)
 2. data:
-    * [`bip340_sig`:`announcement_sig`]
+    * [`bip340sig`:`signature`]
     * [`node_announcement_2_tlvs`:`tlvs`]
 
 1. `tlv_stream`: `node_announcement_2_tlvs`
@@ -560,8 +558,8 @@ The following subtypes are defined:
 
 #### Message Field Descriptions
 
-- `bip340_sig` is the [BIP340][bip-340] signature for the `node_id` key. The
-  message to be signed is `MsgHash("node_announcement_2", "bip340_sig", m)`
+- `bip340sig` is the [BIP340][bip-340] signature for the `node_id` key. The
+  message to be signed is `MsgHash("node_announcement_2", "signature", m)`
   where `m` is the serialised TLV stream (see the
   [`MsgHash`](#signature-message-construction) definition).
 - `features` is a bit vector with bits set according to [BOLT #9](09-features.md#assigned-features-flags)
@@ -594,11 +592,11 @@ ok to send both? if so - what if the info inside them differs?
 The sender:
 
 - MUST set TLV fields 0, 2 and 4.
-- MUST set `announcement_sig` to a valid [BIP340][bip-340] signature for the
+- MUST set `signature` to a valid [BIP340][bip-340] signature for the
   `node_id` key. The message to be signed is
-  `MsgHash("node_announcement_2", "announcement_sig", m)` where `m` is the
+  `MsgHash("node_announcement_2", "signature", m)` where `m` is the
   serialisation of the `node_announcement_2` message excluding the
-  `announcement_sig` field (see the
+  `signature` field (see the
   [`MsgHash`](#signature-message-construction) definition).
 - MAY set `color` and `alias` to customise appearance in maps and graphs.
 - If the node sets the `alias`:
@@ -635,7 +633,7 @@ The receiver:
     - SHOULD send a `warning`.
     - MAY close the connection.
     - MUST NOT process the message further.
-- if `announcement_sig` is NOT a valid [BIP340][bip-340] signature (using
+- if `signature` is NOT a valid [BIP340][bip-340] signature (using
   `node_id` over the message):
     - SHOULD send a `warning`.
     - MAY close the connection.
@@ -692,9 +690,9 @@ value for `amount_msat` and the minimal value for `cltv_expiry`, to be used for
 the last HTLC in the route, are provided in the payment request
 
 (see [BOLT #11][[bolt-11-tagged-fields]]).
-1. type 271
+1. type: 271 (`channel_update_2`)
 2. data:
-    * [`bip340_sig`:`bip340_sig`]
+    * [`bip340sig`:`signature`]
     * [`channel_update_2_tlvs`:`tlvs`]
 
 1. `tlv_stream`: `channel_update_2_tlvs`
@@ -711,9 +709,7 @@ the last HTLC in the route, are provided in the payment request
     1. type: 6 (`disable_flags`)
     2. data:
         * [`byte`:`disable_flags`]
-    1. type: 8 (`direction`)
-    2. data:
-        * [`boolean_tlv`:`direction`]
+    1. type: 8 (`second_peer`)
     1. type: 10 (`cltv_expiry_delta`)
     2. data:
         * [`u16`:`cltv_expiry_delta`]
@@ -757,10 +753,9 @@ peer is offline.
 - The `disable` bit field can be used to advertise to the network that a channel
   is disabled and that it should not be used for routing. The individual
   `disable_flags` bits can be used to communicate more fine-grained information.
-- The `direction` is used to indicate which node in the channel node pair
-  has created and signed this message. If the node was `node_id_1` in the
-  `channel_announcment` message then the `direction` is 0 (false) otherwise
-  it is 1 (true) and the node is `node_id_2` in the `channel_announcement`
+- The `second_peer` is used to indicate which node in the channel node pair
+  has created and signed this message. If present, the node was `node_id_2` in the
+  `channel_announcment`, otherwise the node is `node_id_1` in the `channel_announcement`
   message.
 - `cltv_expiry_delta` is the number of blocks that the node will subtract from
   an incoming HTLC's `cltv_expiry`.
@@ -778,14 +773,13 @@ peer is offline.
 
 #### TLV Defaults
 
-For types 0, 6, 8, 10, 12, 14, 16 and 18, the following defaults apply if the
+For types 0, 6, 10, 12, 14, 16 and 18, the following defaults apply if the
 TLV is not present in the message:
 
 | `channel_update_2` TLV Type        | Default Value                                                      | Comment                                                                                     |
 |------------------------------------|--------------------------------------------------------------------|---------------------------------------------------------------------------------------------|
 | 0  (`chain_hash`)                  | `6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000` | The hash of the genesis block of the mainnet Bitcoin blockchain.                            | 
 | 6  (`disable`)                     | empty                                                              |                                                                                             | 
-| 8  (`direction`)                   | false                                                              |                                                                                             | 
 | 10 (`cltv_expiry_delta`)           | 80                                                                 |                                                                                             | 
 | 12 (`htlc_minimum_msat`)           | 1                                                                  |                                                                                             | 
 | 14 (`htlc_maximum_msat`)           | floor(channel capacity / 2)                                        | // TODO: remove this since makes encoding/decoding dependent on things outside the message? | 
@@ -817,9 +811,9 @@ The origin node:
     - MUST set `chain_hash` AND `short_channel_id` to match the 32-byte hash AND
       8-byte channel ID that uniquely identifies the channel specified in the
       `channel_announcement_2` message.
-- MUST set `bip340_sig` to a valid [BIP340][bip-340] signature for its own
+- MUST set `signature` to a valid [BIP340][bip-340] signature for its own
   `node_id` key. The message to be signed is
-  `MsgHash("channel_update_2", "bip340_sig", m)` where `m` is the serialised
+  `MsgHash("channel_update_2", "signature", m)` where `m` is the serialised
   TLV stream of the `channel_update` (see the
   [`MsgHash`](#signature-message-construction) definition).
 - SHOULD NOT create redundant `channel_update_2`s.
@@ -834,7 +828,7 @@ The receiving node:
       channels.
 - SHOULD accept `channel_update_2`s for its own channels (even if non-public),
   in order to learn the associated origin nodes' forwarding parameters.
-- if `bip340_sig` is NOT a valid [BIP340][bip-340] signature (using `node_id`
+- if `signature` is NOT a valid [BIP340][bip-340] signature (using `node_id`
   over the message):
     - SHOULD send a `warning` and close the connection.
     - MUST NOT process the message further.
@@ -890,8 +884,7 @@ aggnonce = Musig2.NonceAgg(announcement_node_secnonce_1, announcement_bitcoin_pu

The message, msg that the peers will sign is the serialisation of
-channel_announcement_2 without the announcement_sig field (i.e. without
-type 0)
+channel_announcement_2 without the signature field (i.e. only the TLV)

With all the information mentioned, both peers can now construct the
[Session Context][musig-session-ctx] defined by the MuSig2 protocol which is
@@ -1004,7 +997,7 @@ The full list of inputs required:

  • tr_output_key
  • msg: the serialised channel_announcement_2 tlv stream.
  • sig: the 64-byte BIP340 signature found in the
  • channel_announcement_signature field of the channel_announcement_2
  • signature field of the channel_announcement_2
    message. This signature must be parsed into R and s values as defined in
    BIP327.

@@ -1016,7 +1009,7 @@ P_agg = MuSig2.KeyAgg(MuSig2.KeySort(node_id_1, node_id_2, tr_output_key))

The signature can then be verified as follows:

  • Let pk = bytes(P_agg) where the bytes function is defined in BIP340.
    -- Let m = MsgHash("channel_announcement_2", "announcement_signature", msg)
    +- Let m = MsgHash("channel_announcement_2", "signature", msg)
  • Use the BIP340 Verify function to determine if the signature is valid by
    passing in pk, m and sig.

@@ -1060,7 +1053,7 @@ The full list of inputs required:

  • bitcoin_key_2
  • msg: the serialised channel_announcement_2 tlv stream.
  • sig: the 64-byte BIP340 signature found in the
  • channel_announcement_signature field of the channel_announcement_2
  • signature field of the channel_announcement_2
    message. This signature must be parsed into R and s values as defined in
    BIP327.

@@ -1072,7 +1065,7 @@ P_agg = MuSig2.KeyAgg(MuSig2.KeySort(node_id_1, node_id_2, bitcoin_key_1,

The signature can then be verified as follows:

  • Let pk = bytes(P_agg) where the bytes function is defined in BIP340.
    -- Let m = MsgHash("channel_announcement_2", "announcement_signature", msg)
    +- Let m = MsgHash("channel_announcement_2", "signature", msg)
  • Use the BIP340 Verify function to determine if the signature is valid by
    passing in pk, m and sig.

@rustyrussell rustyrussell changed the title extension-bolt: taproot gossip extension-bolt: taproot gossip (features 32/33) Apr 16, 2024
@rustyrussell
Copy link
Collaborator

Note: feature bit assignment clash with #1036 ! Someone needs to move?

(This is why we put feature bits in the title, for easy disambiguation).

@t-bast
Copy link
Collaborator

t-bast commented Apr 17, 2024

I closed #1036 for now to free up that feature bit, taproot gossip is much more important to get as soon as we can ;)

* [`u32`:`fee_proportional_millionths`]


The `disable_flags` bitfield is used to indicate that the channel is temporarily
Copy link
Collaborator

Choose a reason for hiding this comment

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

I mentioned this in the meeting but not sure it got back to here, it'd be nice to also include a "this channel has been closed" bit indicating permanent failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool - will add 👍

@ellemouton
Copy link
Contributor Author

ellemouton commented May 6, 2024

Thanks for the feedback (added the suggested diff - thanks @rustyrussell!) ! A few notes/questions for discussion in the upcoming meeting:

  1. Re adding a permanent bit to the disabled flags: Today, we see a channel closure on-chain and can safely delete that channel from our graph. If we have this permanent bit - what do we do in the case of the channel not actually being closed on-chain? We cannot delete the channel data since the channel nodes could send a new channel update with permanent not set. So we'd need to persist a log of: "channel ID -> closed" so that we throw away any updates after receiving one that indicates permanent failure. I guess this is ok as it is a nice set up for Gossip 2.0 where we would need to do that anyway.

  2. The other thing I think still seems a big fuzzy to me is exactly how we want to handle both existing legacy channels along with new legacy channels (ie, new legacy channels between 2 nodes that support taproot gossip):

    • For existing, legacy channels:
      - If both peers update to support option_taproot_gossip, do we want to have a protocol for re-negotiation & re-signing a channel_announcement_2 message? I dont think I see a benefit of doing this.
      - If the individual peers update to option_taproot_gossip, should they start sending out both old and new channel_update? I assume yes. The tricky part here is 1) When can they stop announcing the old one? 2) for nodes that understand both, how do they know which one is newest (they may have conflicting policy values & one uses timestamp while the other uses blockheight) ? should they still re-propagate both?
    • For new, legacy channels where both peers support option_taproot_gossip:
      - should they negotiate both an old and new channel_announceent? again I assume we can just stick to the old one
      - same question re the channel_update as stated above in the existing-legacy case.

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented May 7, 2024

Today, we see a channel closure on-chain and can safely delete that channel from our graph. If we have this permanent bit - what do we do in the case of the channel not actually being closed on-chain? We cannot delete the channel data since the channel nodes could send a new channel update with permanent not set. So we'd need to persist a log of: "channel ID -> closed" so that we throw away any updates after receiving one that indicates permanent failure. I guess this is ok as it is a nice set up for Gossip 2.0 where we would need to do that anyway.

This didn't manage to come up at the meeting, but I'm not sure its worth doing anything here. Yes, someone could send a closed update, cause you to remove it from your gossip store, then go get you to re-add it later, but...so what? They're misbehaving and gonna have people not route through their channel, that's their problem. Sure, you don't want gossip to flap causing lots of traffic on the network, but that can be addressed by just keeping a short-term cache (as long as your usual gossip rate-limiter) that prevents re-accepting this channel for a little bit.

If someone really wants to do something and keep a log of deleted channels, they can totally do that, and again the nodes that screwed up their channel are now hosed, that's their problem, but the spec doesn't need to mandate this.

@mtfs1
Copy link

mtfs1 commented Jul 12, 2024

Is there a future on using ZKP on pubkey ownership inside a large set as a way to eliminate the necessity on the UTXO announcement as spam filtering?

https://github.com/AdamISZ/aut-ct

This guy presented his project on delving bitcoin some months ago, but I have seen few people commenting on it.

I'm not a bitcoin dev, but I'm commenting here because I got really curious on this proposal, and had some fear that people could be missing something important.

@ellemouton
Copy link
Contributor Author

ellemouton commented Oct 7, 2024

Will be actively working on updating this PR given the convo from the summit over the next while. Main things on my mind:

  • Update message structures so that there is a non-signature TLV range (as is done for bolt12)
  • add outpoint to channel_ann_2 (with this and the above, SPV proofs can be added to announcements by any node).
  • Update chan_ann to cater for legacy chans
  • flesh out the logic for handling 2 types (v1 & v2) messages for the same channel
  • logic for re-creating & re-announcing a legacy chan with the new messages (or a new legacy chan with both messages or only the new messages).

I also think it might be a good idea to use 3 new feature bits:

  1. bit 1 means: I understand all new messages & and able to advertise my own new taproot channels
  2. bit 2 means: I am able to announce existing legacy channels with the new messages
  3. bit 3 means: I am able to provide SPV proofs in announcements if asked

Finally, another thought I had was that since we are going to allow nodes to sign chan_ann at any point (since they now need to go re-announce existing channel potentially), this sort of nicely paves the way for allowing 2 channel peers to decide to convert a previously un-announced channel to an announced one. So gonna try see if that can be worked in too.

EDIT: ok I've written a much more in-depth version in this delving post

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.