Lightning Specification Meeting 2021/01/04

[t-bast]

The meeting will take place on Monday 2021/01/04 at 7pm UTC on IRC #lightning-dev. It is open to the public.

Pending from last meeting

• @rustyrussell to validate BOLT 3: add test vectors for htlc-transactions in case where CLTV is used as tie-breaker for sorting #539 test vectors in c-lightning, @jkczyz to test rust-lightning
• @t-bast to implement Gossip queries: sync complete is back #826 in eclair

Pull Request Review

• Anchor outputs 0-fee txs feature bits bolt-09: reserve feature bits for option_anchors_zero_fee_htlc_tx #828
• Compression algorithm gossip Advertize compression algorithms support in init (features 32/33) #825

Issues

Long Term Updates

• Trampoline routing Trampoline Routing (2021 edition) (Feature 56/57) #829 (@t-bast)
• Upfront payments / DoS protection
• Offers Offers #798
• Blinded paths Route Blinding (Feature 24/25) #765 (@t-bast)

Backlog

The following are topics that we should discuss at some point, so if we have time to discuss them great, otherwise they slip to the next meeting.

• Github built-in security tab for vulnerability disclosure (@ariard)
• Hornet (@cfromknecht)
• lnprototest https://github.com/rustyrussell/lnprototest (@rustyrussell)

---

Post-Meeting notes:

Action items

• @t-bast to add static remote key test vectors to BOLT 3: add test vectors for htlc-transactions in case where CLTV is used as tie-breaker for sorting #539
• all implementations to correctly set sync_complete as per Gossip queries: sync complete is back #826
• @t-bast rework the no-compression requirements for Advertize compression algorithms support in init (features 32/33) #825
• @t-bast rework trampoline routing hints in Trampoline Routing (2021 edition) (Feature 56/57) #829
• second implementation of BOLT 2: option_shutdown_anysegwit (Feature 26/27) #672 for compatibility testing with c-lightning

[t-bast]

As usual, feel free to propose topics for the meeting (ideally a few days before to allow some time to prepare).

[ariard]

Should we start to work on a spec refactoring to move towards a more BIP-like model and as such let each LN team have different priorities without requiring full-coordination on everything ?

[t-bast]

<t-bast> #startmeeting
<t-bast> It's not back :)
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/issues/830
<t-bast> #topic additional transaction test vectors
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/539
<t-bast> Did someone have time to test these new vectors?
<jkczyz> my attempt ran into a hurdle in the RL uses vectors produced with static remote key
<jkczyz> but getting there
<niftynei> afaik no one from clightning has; rusty has been out for the holiday
<BlueMatt> why are we adding tests that *dont* use static remote key
<BlueMatt> given IIRC most implementations are moving towards requiring it/already have
<jkczyz> BlueMatt: Good question. I think it predates https://github.com/lightningnetwork/lightning-rfc/pull/758
<t-bast> Exactly, it predates static remote key, but I can also duplicate them for the static remote key case if you want (I still think it may be useful to keep the existing ones around)
* rusty (~rusty@pdpc/supporter/bronze/rusty) has joined
<roasbeef> well it's part of the spec, so might as well add it 
<BlueMatt> the old ones will always be in git, if people arent supporting non-static-remotekey, we should just drop it from the spec, including the test vectors.
<roasbeef> for eaxmple afaik, eclair doesn't actually spport static key? 
<t-bast> roasbeef: only eclair-mobile
<t-bast> but I think it's too early to remove this entirely from the spec TBH
<jkczyz> t-bast: I found Eclair's test vectors so have been working off them
<BlueMatt> not worth fighting over it, but, yes, it would be nice to have a version with static-remotekey
<t-bast> jkczyz: great
<roasbeef> ah gotcha, but y'all plan on adding it for anchors right t-bast ?
<cdecker> Heya rusty :-)
<rusty> Hi!
<t-bast> roasbeef: yes, but we don't know when anchor will make it to eclair-mobile though...
* t-bast waves at rusty
<roasbeef> intresting, we had to implement a work around for eclair-mobile, since we've flipped out static key bit to required 
<t-bast> #action t-bast to also add these test vector for the static remote key case
<roasbeef> also made us reailize, that we should advertise channel level feature bits differntly, since rn we're forced to _downgrade_ in order to maintain the connection if a node has a legacy channel open t-bast 
<roasbeef> which ofc isn't ideal, and shows how we need to make channel negotiation explicit to avoid inadvertent downgrades which could cause security issues in the fturue 
<t-bast> roasbeef: yes I see what you mean here, I've thought about this too
<rusty> I'm really surprised that static remotekey isn't universal.  Yech.
<roasbeef> lnd 0.12 won't make channels that don't use it, which caused us to discover the way we implemented it would fragment connections 
<t-bast> in fact we'd like to relax dropping the connection on unsupported mandatory channel features when previous channels don't have this feature set, or something like it
<cdecker> Yep, can't wait to strip that code (and special cases) out of c-lightning
<roasbeef> so you need to advertise the "lowest" existing chanenl feature bit, tho this isn't a problem for CL since they don't suport multiple chans to a peer, in that case they'd still need to advertise that old bit htho 
<rusty> Technically your implementation has to advertize it.  Ofc channels may predate it.
<t-bast> In that case it's quite painful: an lnd node wants to require static_remote_key, but has channels with an eclair-mobile that doesn't support the feature at all
<t-bast> We're quite cornered because the spec makes this case impossible (we'd disconnect immediately)
<roasbeef> rusty: so the issue was we want to make the bit required, but they don't understand the bit (have legacy v1 channels), so we need extra logic to downgrade that bit for them 
<roasbeef> yeh
<roasbeef> so we want to _maintain_ the connection, but just not allow new channels to be created ideally in that context 
<rusty> Yeah, you can't make it required if a peer doesn't support it.  Y'know, unless you never want to talk to them again.
<BlueMatt> if you want to require it, you need to force-close the channel anyway. if you just dont want to create new channels without it, then you can just reject channels, not announcing the bit as required?
<roasbeef> mhmm, we didn't anticipate needing that dynamic implicit negotiation logic initially rusty 
<BlueMatt> you can always reject a channel for any reason you want.
<rusty> Yeah, I assumed everyone would just support it before we force it on.
<roasbeef> auto force closing a channel for users isn't a very good idea
<BlueMatt> sure, but you have to do that either way if you dont support the code to run such a channel
<BlueMatt> in any other case, you can set the bit optional and reject all new channels to such peers
<BlueMatt> sucks for nodes seeking out to connect to you based on your lack of unknown required bits, but shouldnt be too painful for them, an immediate error message and maybe disconnect should make them go away.
<rusty> Well, why make it compulsory?  Seems premature (unf)
<roasbeef> rusty: make static key cumpulsory? it's waaay safer 
<roasbeef> bout time
<roasbeef> we're also making the mpp payload reqwuired as well 
<rusty> Yeah, but we need Eclair mobile to upgrade first.
<BlueMatt> right, I'm not suggesting any specific point, only noting that its not really an issue with the way the bits are set afaiu
<t-bast> I agree with BlueMatt, having a small logic to set the bit as only optional when connecting to a peer you have old channels with, and simply rejecting new channels should work fine
<roasbeef> yeh that's what we do
<roasbeef> just pointing out that it isn't explicitly mentioned in the spec sincew e don't have a specific section for how to handle channel upgrades
<t-bast> true, it's an edge case that's a bit painful to handle :/
<roasbeef> do y'all aready have logic for that? we had to add it as a special case 
<roasbeef> live in lnd 0.12rc3 
<t-bast> we don't have that in eclair, we should add it - we do have the ability to configure feature bit overrides per-peer, so node operators can work around this manually though
<roasbeef> ok cool, what we have downgrades properly and has been tested with users running eclair-mobile that have a legcay channel w/ an lnd node 
<niftynei> should i know what a channel upgrade is?
<t-bast> niftynei: this refers to roasbeef's ML proposal a few months back to upgrade channels without needing a close
<t-bast> for example going from legacy commitments to anchor outputs by re-signing new commit txs after pausing the channel
<niftynei> right. thank you.
<roasbeef> or even what can happen rn: you have old channel with node, and want to open new channel with node, how do you do that? 
<roasbeef> rn it's implicit, but my goal is to make it explicit 
<roasbeef> some of the issues rn arise since things are implicit, and it being implicit means a peer can make you downgrade essentially based on its feature bits 
<roasbeef> short path to explicit, is adding a new tlv to open/accpt that's just chan_type, and we back value everything to make it work properly 
<t-bast> let's move on to the next topic, I'll add static remote key test vector to the PR before next time
<roasbeef> woudl also make testing protocl stuff easier as well, rn we use build flags to force old behavior 
<roasbeef> +1
<t-bast> #topic sync complete
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/826
<t-bast> I haven't had time to implement it, but it should be quite straight-forward. Before I go into that direction, are all on the same page that implementations are willing to at least do the first bullet point in a next release (correctly set the `complete` boolean flag)?
* Luc21 ([email protected]) has joined
<roasbeef> lnd already sets it ;) 
<roasbeef> since it never even did anything with the "proper" interpretation of the spec, and we needed to leave it around for our older nodes 
<t-bast> great, that's one down ;)
<rusty> t-bast: yeah.  We don't currently use the flag, though we were going to set it appropriately (particularly for the case where we're only connected with one node).
<t-bast> allright, then I'll start implementing it re-using the `full_information` field
<rusty> But IIRC that gave lnd issues, so we always set it to True right now.
<t-bast> you mean in each reply or only the last one?
<rusty> t-bast: we set it always.  But we won't notice if someone doesn't.
<rusty> So ack from me.
<t-bast> ok, roasbeef do you know why setting it to `false` caused issues with lnd?
<rusty> (I could be misremembering, ofc: pretty sure modern lnd will be fine)
<t-bast> #action t-bast to implement in eclair for real
<t-bast> allright, let's start with implementing the part where we correctly set this to `sync_complete` and see where that takes us when doing cross-impl tests
<t-bast> #topic anchor outputs 0-fee feature bits
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/828
<t-bast> Looks like this has ACKs all over the place, shall we just merge it and then we can wait for cross-compatibility tests for #824?
<lndbot> <johanth> I say yes :)
<rusty> Ack (embarrassingly, I've gotten logged out of GH and my phone required for 2fa is sitting next to my sleeping wife, so I can't log back in right now!)
<t-bast> xD
* lnd-bot ([email protected]) has joined
<lnd-bot> [lightning-rfc] t-bast merged pull request #828: bolt-09: reserve feature bits for option_anchors_zero_fee_htlc_tx (master...zero-fee-htlc-feature-bits) https://github.com/lightningnetwork/lightning-rfc/pull/828
* lnd-bot ([email protected]) has left
* lnd-bot ([email protected]) has joined
<lnd-bot> [lightning-rfc] t-bast pushed 1 commit to master: https://github.com/lightningnetwork/lightning-rfc/compare/d0c83854de7e...a00418f5f9f1
<lnd-bot> lightning-rfc/master a00418f Johan T. Halseth: bolt-09: reserve feature bits for option_anchors_zero_fee_htlc_tx (#828)
* lnd-bot ([email protected]) has left
<t-bast> Done!
<t-bast> #topic advertize compression algorithms in init
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/825
<t-bast> I've reworked the approach since last spec meeting for this PR, I now suggest using a new `init` tlv field to advertize what compression algorithms we support
<t-bast> Then wherever compression may be used this will apply (currently only for gossip queries)
<roasbeef> t-bast: false only affects older versions, as they think there's more to send, the latest version of lnd (since a few ago) won't even look at the field and only uses the block markers 
<t-bast> roasbeef: great, so we should be fine making progress on this
<roasbeef> t-bast: bit behind on this but why this route over feature bits? 
<roasbeef> re tlv in init 
<rusty> Why the feature bit?   Non-existence of the field should provide the same, no?
<roasbeef> also wasn't there some older proposal to use this for something else by Z? 
<roasbeef> rusty: don't follow the second part 
<t-bast> roasbeef: the issue with the feature bit route is that if we want to support multiple compression algorithms, we'd need one feature bit per algorithm? And since old nodes that do support zlib don't set this feature bit, it won't help with non upgraded nodes
<roasbeef> oh yeh this already exists lol
<t-bast> rusty: I added it so that you can see it in node_announcement
<rusty> roasbeef: if the field doesn't exist, you can behave as now.  If it does, you know (1) they understand the feature, and (2) don't do zlib if not present in it.
<rusty> t-bast: ah, right.
<t-bast> and choose to connect only to those peers that you know will actively tell you what they support
<t-bast> otherwise there's still a hole for the case where you don't want to support any compression (what RL wants to do)
<roasbeef> ahh I see 
<roasbeef> i'd also forgotten that we'd already added init tlvs as well 
<t-bast> they could try connecting and then fail if the tlv isn't set, but that's wasteful
<t-bast> also there's a nit for the case where no compression is supported: the absence of the field, a 0-length byte array or any array with all 0s have the same meaning
<t-bast> I don't know if we should explicitly state what should be set in that case (I currently chose having the tlv field with a 0-length array)
<t-bast> Or if we should allow all
<rusty> t-bast: I believe it's logically consistent to say "missing === empty".
<roasbeef> rusty: +1
<roasbeef> one of those empty byte array vs nil array type tings
<BlueMatt> you could mention that it should be minimally-encoded, which would imply missing if no compression algs are supported, but not sure it matters
<t-bast> Ok, I'll re-work that part a bit then, I'll relax the requirements and add `SHOULD` recommandations
<roasbeef> g2g early, something came up, will read scrollback (also last week's) 
<t-bast> #action t-bast to re-work the requirements for no compression case
* t-bast waves at roasbeef
<t-bast> Since I'm chair, let's shill a bit my own long-term thing :)
<t-bast> I have opened a trampoline 2021 edition PR
<t-bast> a bit simpler and more flexible than the previous one, mostly since electrum has implemented it and wanted to interoperate
<t-bast> #topic Trampoline 2021 edition
<t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/829
<cdecker> Nice, I was worried that we wouldn't get to discuss it so I added my thoughts as a comment before 
<t-bast> I see that cdecker already added interesting comments :)
<t-bast> Regarding your first point (size of the total route), we could impose a maximum size on the trampoline onion; however I'm not sure it can be used as a very effective multiplier, since the attacker doesn't control what happens between trampoline nodes
<t-bast> And if we make progress on fighting spam, do we even need to address this issue?
<cdecker> t-bast: that's why I'm using the network diameter, there is no shorter path between those nodes than through the entire network
<t-bast> I like the flexibility of the size, in case we need to add more tlvs in the onion for route blinding / rendezvous types of things
<t-bast> cdecker: there is no path *that you know of* but there may be unannounced channels
<cdecker> Agreed, but I think a simple argument as to what a trampoline hop's size looks like and giving an upper bound is already enough
* Luc21 has quit (Remote host closed the connection)
<BlueMatt> I admittedly havent gotten to look yet, but the second bullet seems scary - is the intent here that users can send to a node with a next-hop node that supports trampoline and no other information by letting the recipient specify middle nodes? I mean in many cases you likely cant avoid letting users do that, but implementations actively chosing to do that seems like we've given up on privacy.
<BlueMatt> bits being in gossip was nice cause then at least you could in the future ask your trampoline-supporting next-hop to give you gossip info but only the top N nodes it knows about that support trampoline
<cdecker> Yeah, I'm struggling with the recipient-defined trampoline nodes tbh
<BlueMatt> and you could ask around for more copies of the same and then at least build routes with a few trampoline hops
<t-bast> BlueMatt: I don't follow why: the reason here is that we let the recipient choose how you'd get to them, it seems good for privacy rather than bad?
<cdecker> That's assuming route blinding though, isn't it?
<t-bast> I removed the gossip part because I think 1) it's the one that will take the longest to agree one and 2) it can be added in a second phase
<t-bast> cdecker: not necessarily, it's exactly like the current routing hints
<cdecker> Which leak the existence of private channels to nodes that would otherwise not learn about them
<t-bast> yes and here with trampoline it's better, because you only leak that you're close to a node_id (but without specifying exactly how close)
<t-bast> not even necessarily close, but rather that you're reachable from that node
<cdecker> I'd much rather stick with route hints, and just pass them along to the last trampoline. We're not leaking more info that way, and we don't assume the trampoline is either the destination or knows how to get there
<BlueMatt> t-bast: I always assumed the only reasonable way to do trampoline was similar to tor onions - specify at least two-ish trampoline hops between you and some middle node, then take a few hops from the recipient's hints
<t-bast> BlueMatt: you can still add more trampoline nodes before the recipient's (or choose to ignore his recommendations)
<cdecker> BlueMatt: that'd make only sense with rendez-vous routing imho
<BlueMatt> t-bast: only if trampoline info is in gossip info, though
<t-bast> cdecker: if we pass the route hints to the trampoline node, we're doxxing the recipient
<BlueMatt> cdecker: right, eventually we'd need that as well.
<t-bast> cdecker: because the trampoline node know clearly knows that the next node is the final recipient
<cdecker> Not if you use decoy route hints xD
<t-bast> BlueMatt: it will be advertized in `node_announcement`, nodes will be able to set the trampoline feature bit
<cdecker> Which is what I'm doing here https://github.com/lightningd/plugins/pull/193 btw :-)
<t-bast> BlueMatt: so you still have a way of finding trampoline nodes yourself
<t-bast> cdecker: true :)
<BlueMatt> t-bast: ah, ok, i didnt understand what "any new gossip mechanism" meant, then
<t-bast> BlueMatt: it's because I clearly put too many things in my first trampoline PRs, and focused on many gossip optimizations that can be done in a second phase
<BlueMatt> ok, fair
<t-bast> BlueMatt: I removed them from that one to focus on the core of the proposal (which in itself is enough and quite flexible, allows choosing your perf/privacy trade-off)
<BlueMatt> right. well gotta get nodes out there that support trampoline before we can get any privacy for trampoline users :)
<t-bast> exactly!
<rusty> (BTW, tangentially related: I dropped my idea of using x-only pubkeys for onion messages, since it means we don't get to reuse the current onion code)
<t-bast> and implementing the set of functionality in this PR is not too hard, should be a 3-5 dev days I believe, which would get the ball rolling
<t-bast> rusty: good to know, I didn't have time to try them so I can cross that off my todo list xD
<t-bast> cdecker: I'll think a bit more about the routing hints, it may work better than I thought
<t-bast> cdecker: it takes a lot of space in the onion but well, that happens
<cdecker> Agreed that we should get the trampoline base functionality out, but the trampoline hints need some work
<cdecker> That being said, my main concern is the size impact to the invoices, not really privacy (there is none imho)
<t-bast> I'll work on those and see where it takes me, meanwhile don't hesitate to look at the rest of the PR and let me know if something feels off!
<t-bast> #action t-bast to re-work trampoline routing hints
<cdecker> Personally I've been thinking about unannounced channels much more as an optimization (lowering the gossip traffic on the network) rather than a privacy enhancement (which is a shaky argument in any case with today's routing)
<t-bast> cdecker: we're doing this as well already: no need to advertize 10 channels to the same peer for example, a portion of them can be unannounced
<t-bast> and having unannoucned "shortcut" channels can also be useful, just to throw off graph analysis
<cdecker> Hehe, we optimize by only ever having one channel xD
<t-bast> xD
<t-bast> Looks like we're already over 1 hour, do you want to discuss another topic?
<cdecker> t-bast: shortcuts that don't match exising channels can't be used due to the onion construction, unless you retroactively tear down an HTLC once you realize that two of your nodes are part of the payment. 
<t-bast> cdecker: they're useful with trampoline, but you're right that they're not usable for normal channel-relay
<cdecker> This has been (wrongfully) called wormhole attack, however I agree that it's hardly an attack if the end-to-end security is maintained, we're just more efficient, involving fewer channels, and getting a bit more fees for it xD
<cdecker> Anyway, I better be off soon :-)
<t-bast> Let's end now then, thanks everyone for your time!
<cdecker> Thanks t-bast for chairing and setting up the agenda ^^
<t-bast> #endmeeting