Lightning Specification Meeting 2020/12/21

[t-bast]

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

Pull Request Review

• op_shutdown_anysegwit BOLT 2: option_shutdown_anysegwit (Feature 26/27) #672
• cltv tie-breaker test vector BOLT 3: add test vectors for htlc-transactions in case where CLTV is used as tie-breaker for sorting #539
• 0 sat/byte anchor htlc txs feature: define option_zero_htlc_tx_fee (feature 22/23) #824
• zlib feature bit Advertize compression algorithms support in init (features 32/33) #825
• less strict reply_channel_range Gossip queries: sync complete is back #826

Issues

Long Term Updates

• Upfront payments / DoS protection
• Offers Offers #798
• Blinded paths Route Blinding (Feature 24/25) #765 (@t-bast)
• Trampoline routing Trampoline Routing #654 (@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

• @cdecker to bump feature bits on BOLT 2: option_shutdown_anysegwit (Feature 26/27) #672
• @t-bast to add other commitment formats test vectors to BOLT 3: add test vectors for htlc-transactions in case where CLTV is used as tie-breaker for sorting #539
• @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
• @halseth to split feature: define option_zero_htlc_tx_fee (feature 22/23) #824: we will merge the feature bits now, and the new commitment format once interop is tested
• @t-bast to try another approach for Advertize compression algorithms support in init (features 32/33) #825 with a dedicated tlv field
• @t-bast to implement Gossip queries: sync complete is back #826 in eclair

[t-bast]

As usual, don't hesitate to bring other topics to the table (ideally a few days before the meeting to let participants prepare).

[cdecker]

Trying to get the make a dent into the backlog it seems like #672 is pretty uncontroversial and can either be merged or closed right away. Shall we try and resolve some of the old ones?

[t-bast]

Sure, I added #672 and we can focus today's meeting on resolving some oldies.

[t-bast]

I'm also adding #539 which is an important test vector IMHO

[niftynei]

IRC chat log, also available at http://gnusha.org/lightning-dev/

<niftynei> #startmeeting
<niftynei> if you haven't seen the agenda yet, please see https://github.com/lightningnetwork/lightning-rfc/issues/827
<cdecker> Beep boop, meeting starting :-)
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/issues/827
<niftynei> is there any old business from last meeting that we should address first?
* sh_smith ([email protected]) has joined
<t-bast> I don't think so, I think we'd covered everything
<niftynei> great.
<bitconner> sorry forgot to mention i'm also technically ooo so mainly lurking today :)
<niftynei> #topic #672, OP_SHUTDOWN_ANYSEGWIT
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/pull/672
<t-bast> Did someone run some taproot tests and play with early non-zero witness programs?
<niftynei> so this is proposing to allow "any segwit version" script addresses for closing 
<t-bast> Or is this PR only preparing for the future early enough to have this deployed before we actually start lveraging it?
<cdecker> Yep, it's only preparation so we can upgrade rigt away 
<niftynei> do the latest bitcoin-core mailing list discussions around 'future segwit' version's bech being different relevant here?
<niftynei> *are the
<bitconner> the feature bit assingment may need to change, 22/23 is being deployed in lnd0.12 as anchor_zero_fee_htlc
<cdecker> I don't think the address encoding matter for script acceptance
<cdecker> Good catch bitconner
<t-bast> Agree with bitconner, it would be nice to have anchor-0-fee grab 22/23
<t-bast> At this point I guess we're pretty confident this will cover at least witness programs version 1, but do we have any guarantee for future versions?
<cdecker> Ok, I'll bump the featurebits of #672 then, since Rusty is afk for some more days
<t-bast> I don't know if the bech32 changes will impact that at all TBH
<t-bast> But we can always roll out a different set of feature bits later for `op_shutdown_anysegwit_revisited` if needed, it's quite a cheap change :)
<cdecker> The phrasing seems pretty relaxed, and I don't think we're going to have >40byte output scripts in future segwit versions
<niftynei> sounds like the only point of concern here is the version bit numbers
<t-bast> And we should be quite lax with this since it's really the owner of the output who chooses where to send the funds
<cdecker> Currently not sure if OP_16 is defined in segwit versioning at all tbh
<cdecker> Just checked: OP_16 matches what is covered by BIP 141 "If the version byte is 1 to 16, no further interpretation of the witness program or witness stack happens, and there is no size restriction for the witness stack. These versions are reserved for future extensions." So no weird discrepancy here
<niftynei> great, thanks for checking
<t-bast> Great, once the feature bits are bumped it LGTM
<cdecker> Ok
<cdecker> #action cdecker to bump featurebits due to collision :-)
<niftynei> (#824 does in fact have the bit collision)
<niftynei> moving on then. 
<niftynei> #topic cltv tie-breaker test vector
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/pull/539
<cdecker> Yeah, we seem to not be very good at avoiding collisions (#819 collides with the BOLT for offers)
<t-bast> This PR is particularly relevant now that MPP has rolled out, and such collisions do happen in production
<cdecker> Hm, too bad Rusty isn't here, since he checked the vector iirc
<t-bast> If you think this is worth adding to the spec, the PR only contains a test vector for the 1.0 commitment format, but we have the corresponding vectors for static_remotekey and anchor_outputs in eclair, I can update the PR to add them as well
<niftynei> Has anyone else verified the vector?
<t-bast> We have them in eclair, don't know about rust-lightning and lnd
<cdecker> ccccccfnijgigfhitrrdkhnninueehnngtikbcnnrtet
<cdecker> Hello yubikey :-)
<cdecker> Sorry about that
<t-bast> Oh so that wasn't your cat :D
<cdecker> Very heavy on the `c` that cat ^^
<cdecker> Anyway, seems we need confirmation that the vectors work as expected
<niftynei> i think t-bast said they've incorporated them into their test suite
<t-bast> I'll add the ones for anchor_outputs and static_remotekey then
<cdecker> That'd be great
<t-bast> It would be great to have another implementation confirm
<t-bast> #action t-bast to add other commitment formats test vectors
<t-bast> #rusty to test them in c-lightning
<t-bast> #action rusty to test them in c-lightning
<t-bast> I just promoted rusty to a keyword :facepalm:
<niftynei> #action lnd/rust lightning/electrum to also test
<niftynei> lol
<jkczyz> will look into rust-lightning
<cdecker> Great, thanks jkczyz
<niftynei> being promoted while you're on vacation is a nice surprise to come back to :P
<niftynei> let's keep moving then
<t-bast> the #rusty keyword is a cheat code in lightning
<niftynei> #action jkczyz to test on rust-lightning
<niftynei> #topic 0 sat/byte anchor htlc txs
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/pull/824
<niftynei> oof this one's long
<t-bast> heh but it's basically just replacing an "if" in several places
<cdecker> What I stumbled over was that we now have two options with the `option_anchor_*` prefix, but then you talk about `option_outputs` as if it were a separate option
<cdecker> Is it a combination of the two (if yes how?) or how should we read the semantics?
<t-bast> yes it's defined in bolt 9 in the PR as the OR of the two
<t-bast> We define `option_anchors` as `option_anchor_outputs || option_anchors_zero_fee_htlc_tx`
<cdecker> I see
<cdecker> At some point I'll have to draw a flowchart of the various options xD
<t-bast> The PR looks good to me, but I'd like to have at least one other implementation do interop testing; we had some surprises when doing interop testing for the previous anchor outputs and found issues
<niftynei> so eventually this would deprecate the opt_anchor_outputs flag, in favor of flagging for oazfht
<niftynei> t-bast, right, sounds like lnd has this implemented, needs a 2nd to be eligible for merging
<t-bast> niftynei: yes, even though it can be handy to keep the opt_anchor_outputs for a while (we'll use it in Phoenix where we don't have any utxo management for now)
<t-bast> the fact that it makes the htlc txs impossible to relay if you don't add an input can be painful to handle on mobile wallets
<t-bast> where you likely won't have many utxos available
<t-bast> I'm still unsure what's the best strategy will be for those small mobile wallets, maybe the current anchor_output with a fixed feerate (and potential CPFP from a different wallet) will be better there
<t-bast> TL;DR: I quite like the fact that we have both options :)
* bmancini55 has quit (Ping timeout: 245 seconds)
<niftynei> Sounds like the space for anchor output design is still a bit of an open question?
<t-bast> yes, clearly, and until we know what package relay will look like in bitcoin things may keep changing a bit 
<niftynei> Maybe worth noting that this is being done in response to a "funds loss vector" ariard identified 
* bmancini55 ([email protected]) has joined
<cdecker> Yep, the PR has a sidenote to that effect
<niftynei> i'm not really adding new info here. is there anything else worth discussing?
<t-bast> I think we can mark this as mostly waiting for interop?
<niftynei> #action wait for interop
<cdecker> Sounds like we need a second implementation for interop testing before merging?
<t-bast> But we can choose to really reserve the feature bits since lnd ships with those
<niftynei> Good point.
<cdecker> Yep, can we split out the featurebits change and apply those already? That'd at least prevent future collisions
<t-bast> I think it would make sense, let's ping halseth for that
<niftynei> #action ask halseth to break out featurebit change into separate PR
<niftynei> ok. let's move on
<niftynei> #topic advertize zlib support via feature bits
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/pull/825
<t-bast> Even though I'm the author, I'm not sure at all we want to do that (or how we want to do it)
<t-bast> This is really fishing for comments :)
<t-bast> rust-lightning would like to avoid adding a dependency on zlib
<niftynei> this is addressing an issue that was raised by rust lightning?
<t-bast> exactly
<t-bast> the spec assumes all implementation would add zlib support
<t-bast> defining a feature bit to say I support zlib is a bit ugly, as we could need one feature bit per compression algorithm we'd support, which is horrible
<bmancini55> yes, the primary issue is that as a query initiator, there is no way to signal that we would like reply_channel_range in raw format
<cdecker> Yeah, it's a bit weird to introduce an optional bit after assuming everybody had it already
<t-bast> the alternative suggested by matt and ariard is to instead have a kind of "subtractive" feature bit to say "I only support raw format"
<cdecker> Whatever we do the signal is going to be hit r miss for some time
<t-bast> this `format_raw` alternative would allow backwards-compatibility
<t-bast> I'm not a big fan of feature bit that advertize that we *don't* support features though
<cdecker> But hey, anything that allows us to be more specific, even if it takes some time
<cdecker> Yeah, let's opt-in whenever possible, and keep opt-outs in the form of mandatory flags, otherwise this gets really confusing
<t-bast> bmancini55: we can also take the problem differently: instead of my approach of adding a feature bit, we can re-work a bit the queries to let the query initiator signal what format it wants
<niftynei> ok that was going to be my question
<t-bast> and this would provide us more flexibility on the formats, and could be an even tlv field so that rust-lightning only connects to nodes who have added support for this
<niftynei> it seems like what we actually want here is the ability to specify the type of response that we expect
<t-bast> niftynei: exactly, and the approach of a `zlib_support` feature bit is probably not the best way of achieving that
<niftynei> you could still add a feature bit for *supports gossip query type signaling*?
<t-bast> it's a better direction indeed
<t-bast> I'll re-work the PR (or offer a competing one)
<niftynei> sweet
<cdecker> Sounds good
<bmancini55> a tlv on query_channgel_range would certainly solve the problem we have.  inbound query_scids queries would might be an issue though
<t-bast> #action t-bast to try a different approach with query initiator tlv field
<t-bast> bmancini55: I'll dive a bit into that rabbit hole and will send a PR :)
<bmancini55> sounds good
<niftynei> ok, sounds like we have a new direction on this.
<niftynei> moving on. 
<niftynei> i think this might be the last item we have time for today -- seems like most need to head out early
<niftynei> and i'm running out of snacks
<t-bast> OP_SECURE_MORE_CHIPS
<niftynei> #topic gossip queries: sync complete is back
<niftynei> #linkhttps://github.com/lightningnetwork/lightning-rfc/pull/826
<niftynei> #link https://github.com/lightningnetwork/lightning-rfc/pull/826
<t-bast> The TL;DR for that one is: I still don't understand why we went for implicit end of sync (to save a couple bytes after exchanging KBs worth of data???)
<t-bast> If we add an explicit signal for end-of-sync, we can easily split blocks across multiple responses which is more flexible
<t-bast> The only open question in my opinion is whether we want to re-use the `full_information` field that no-one uses because it's weird AF, or introduce a new tlv field
<niftynei> what does it mean 'not to maintain up-to-date channel information'
* joost ([email protected]) has joined
<niftynei> it seems equivalent to saying "go ask other nodes, my data's unreliable"?
<t-bast> ¯\_(ツ)_/¯
<bmancini55> That's my understanding of it.  "I'm still syncing"  perhaps.
<t-bast> that's why I lean towards removing it, I don't see how it's really actionable (and it would be fully trusted anyway)
<t-bast> your peer can lie on that field anyday
<bmancini55> I think I prefer to reuse `full_information` to signal completion.  
<t-bast> :+1:
<bitconner>  +1
<bmancini55> I believe legacy LND still uses that field to signal multipart replies
<bitconner> that's correct
<bmancini55> My question is whether it's worth adding a TLV to signal a failure.  
<niftynei> c-lightning sends back 'full-information' true iff you've requested blocks from a chain we don't know
<niftynei> :s/true/false/
<niftynei> sorry, i mean to say we signal we don't know 'full-information' if you've asked us for queries on an unknown chain
<t-bast> niftynei: but you only really support one chain, right? That's probably a usecase we can ignore?
<t-bast> and we advertize it in `init`
<t-bast> bmancini55: can you detail? what kind of failure?
<t-bast> oh you mean instead of silently dropping a request and not responding to it?
<bmancini55> wrong chain, or even "I don't want to reply"
<bmancini55> @tbast: yep
<t-bast> since we advertize the chains we support in `init`, I think that a sender asking us for a chain we don't support doesn't really deserve a nice error, we can just drop it
<niftynei> or send an error back in reply?
<t-bast> maybe it makes sense for throttling though, responding with `sorry, I have that information, but you're asking too much, retry later`
<t-bast> but that's probably a separate change, isn't it?
<niftynei> i think so
<bmancini55> yeah I think it could addressed at another time if there was a need/desire
<t-bast> sgtm
<niftynei> feels like the 'full-information' bit is standing in where an error message might be more appropriate
<bmancini55> agreed
<niftynei> i might be missing some nuance here around error message etiquette though
<niftynei> ok. what actions do we have outstanding here?
<t-bast> TBH we haven't really felt the need for an error here, sync is some kind of a best effort / partial information mechanism anyway where you regularly ask different peers to try to get a good enough view of what's happened in the network
<t-bast> I'd simply wait for now for rusty's opinion on the PR since he opened the issue
<niftynei> #rusty opine on issue
<niftynei> :s/issue/pr
<t-bast> It looks like we're ok with re-using the `full_information` field for now, and maybe later add explicit errors if we feel it's worth it
<niftynei> in terms of implemenation, is this something you've done for ACINQ yet t-bast?
<t-bast> Not yet, but that's going to be my next step :)
<niftynei> or are we at the 'draft and comment' stage still
<niftynei> ok cool.
<niftynei> #action t-bast implementing first draft in ACINQ
<t-bast> just waiting to see if I get a big nack from someone, but otherwise I'll implement
<niftynei> alright. that's all we have time for today, unfortunately[
<niftynei> i'm 100% out of chips now, so i'm going to be abdicating the meeting chair.
<t-bast> haha thank you for chairing!
<t-bast> great, right on time on my side
<niftynei> happy holidays to everyone!
<niftynei> #endmeeting
<t-bast> thank you all,

[halseth]

Thanks for posting the logs, the Slack bot[1] broke down so I missed this. Separated the feature bits reservation into its own PR: #828

[1] yes this is embarrasing im not a real haxxor sorry

[cdecker]

#672 has been updated with new featurebits as per discussion.