20:09:29 <t-bast> #startmeeting
20:09:29 <lightningbot> Meeting started Mon Aug  3 20:09:29 2020 UTC.  The chair is t-bast. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:09:29 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
20:09:45 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/issues/792
20:10:00 <t-bast> Here is the (tentative) agenda for today, don't hesitate to suggest something
20:10:29 <t-bast> A bunch of small PRs that shouldn't take too long and then the usual long standing issues ;)
20:10:43 <t-bast> #topic Harden cltv_expiry_delta recommendations
20:10:51 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/785
20:11:26 <t-bast> The discussion has made good progress on the PR, IIUC the only thing you'd like me to change is to revert the default back to 9 (instead of 18) in Bolt 11 for backwards-compat?
20:11:30 <cdecker> Tiny nit on changing the default: that change is redundant if we make the field mandatory, and would automatically make all existing nodes non-compliant
20:11:48 <roasbeef> so default and recommened?
20:12:12 <cdecker> Yep, just for cleanliness' sake, make the field mandatory, but keep the old default if a node doesn't specify it
20:12:28 <cdecker> (mandatory but not enforced that is)
20:12:32 <roasbeef> fwiw lnd always includes our value explicitly, as has for soem time I think
20:12:35 <t-bast> we now say that implementation MUST specify this field in invoices, so I don't really know what to do with the previous suggestion of a default of 9
20:12:45 <roasbeef> isn't it backwards compat to assume a new higher default?
20:12:57 <rusty> Note that the 18 *is* backwards compatible, since you can always set more than they ask.
20:13:01 <roasbeef> in that if they want 9, and we send 18, that should be ok assuming it's in their tolerance
20:13:06 <cdecker> Oh right, nevermind then ^^
20:13:08 <t-bast> I agree with roasbeef, even if nodes accept 9, sending them 18 is always better, right?
20:13:09 <roasbeef> but sending a lower would be a no go ofc
20:13:10 <rusty> roasbeef: snap.
20:13:13 <bitconner> it's backwards compatible on the sender-side, it's not on the receiver side
20:13:20 <cdecker> In that case we don't need to make it mandatory either, do we?
20:13:33 <bitconner> the sender can always use a higher value, the receiver must continue to accept lower values if the sender still uses 9
20:13:46 <t-bast> it's better to be explicit in the invoice instead of relying on a magic value in the spec IMHO
20:14:00 <bitconner> agreed, but i always agree with decker that we can't change the assumed default
20:14:00 <t-bast> exactly like bitconner says
20:14:01 <ariard> the policy doesn't change on the receiver side, is just a recommendation on the sender-side
20:14:11 <rusty> bitconner: that's why it's mandatory, for sender though, at least until some magical day...
20:14:30 <bitconner> rusty: yes i think that's all we can do
20:14:31 <t-bast> receivers will now have it specified in their invoice so they don't care
20:14:49 <cdecker> Hm, you're right, there is no clean solution. I'll retract my comments
20:14:52 <bitconner> t-bast, they have old invoices which don't
20:15:02 <t-bast> but these old invoices expires after 1 hour
20:15:07 <t-bast> so it's ok, right?
20:15:07 <bitconner> they might*
20:15:17 <bitconner> you can set longer expirations :P
20:15:28 <t-bast> do you use long expirations?
20:15:31 <cdecker> c-lightning will also set the field fwiw
20:16:05 <bitconner> i personally don't, but there are users that do
20:16:15 <t-bast> if we all already set this field, then there's probably no need to worry about backwards-compat; in that case we could completely remove the recommended default of 9 from the spec and don't recommend any default?
20:16:20 <rusty> cdecker: not if it's actually 9...
20:16:38 <roasbeef> yeh we also recently had to tighten up our config validation to ensure a user can't set a value lower than the existing default (of 9)
20:16:54 <rusty> t-bast: no, if someone actually sets it to 9 we will omit it :(
20:16:58 <bitconner> roasbeef: our validation now requires all user-chosen CLTV values to be at least 18
20:17:00 <ariard> but really isn't two different things what you write inside the invoice and how do you reject HTLC when they arrive at last hop?
20:17:20 <t-bast> rusty: daaamn, then I'll just put back 9 in there and it should be good?
20:17:29 <rusty> t-bast: no, I prefer 18 :)
20:17:56 <t-bast> I'm lost on who prefers what now :D
20:18:12 <rusty> I mean, it's not critical, but 18 works too and is more sensible?
20:18:17 <t-bast> ariard: but this one you reject based on what's in your invoice since you're the recipient
20:18:21 <rusty> We do need some default value unf.
20:18:44 <bitconner> imo there's no need to change the assumed default, it can stay at 9
20:18:53 <t-bast> My preference is to put 18 for future implementers that don't have backwards-compat requirements (for example electrum and rust-lightning)
20:19:13 <t-bast> So that we don't have an unhealthy recommendation lingering in the spec
20:19:34 <t-bast> But that's nit TBH so I can settle for 9 if the majority prefers that
20:20:08 <bitconner> perhaps we can just note that the goal is to ultimately remove the assumed default entirely
20:20:27 <bitconner> (same should be done for expiry tbh)
20:20:43 <rusty> bitconner: yeah, we can simply note that you assume X if not specified, for backwards compat with old spec mistakes...
20:21:30 <t-bast> bitconner: I could do something like that: "Default is 9 if not specified (for backwards-compatibility with old nodes)"
20:21:37 <ariard> t-bast: but IIRC the spec doesn't require you to reject HTLC as final hop based on invoice non-compliance, it might not even has one (spontaneous payment)
20:22:23 <t-bast> ariard: true, but it's for your own safety that you want to check that value (but I agree that it may come from your internal configuration instead of the invoice, but having it in the invoice lets the sender know what value they should use)
20:23:09 <t-bast> ariard: keysend are clearly an issue in that regard: we don't know what final_expiry to use (it should be added to node_announcement in an extension tlv or something)
20:24:44 <bitconner> for keysends we just check that the final ctlv is greater than our go-to-chain delta
20:24:48 <ariard> t-bast: really I think that's 2 different parameters, minimum cltv_expiry is a safety measure to avoid someone easily double-spending the HTLC, beyond and what should be in the invoice is an application-specific one on how much block parties let them to come to resolution
20:24:51 <cdecker> Doesn't accepting payments with the wrong final CLTV open you up to probing for being the last hop?
20:25:06 <rusty> t-bast: actually, I am swayed by the idea of making 18 the default because keysend.
20:25:07 <ariard> like delayed HTLC
20:25:33 <ariard> cdecker: not an issue anymore with payment_secret ?
20:25:45 <cdecker> Righto
20:26:44 <t-bast> ariard: I don't understand, we're talking about `min_final_expiry_delta` right? Can you clarify?
20:28:20 <bitconner> rusty: default on invoices? there are no invoices in keysend?
20:28:37 <rusty> bitconner: yeah, but it's a nudge in the right direction :)
20:28:48 <bitconner> if it's just a sender-side default that is easy, but doesn't need to affect BOLT 11 at all
20:29:11 <rusty> Like, "FFS, just use 18 if you don't know what to use" matches our advice on the recipient side...
20:29:23 <t-bast> bitconner: I think the goal is that this way senders will use 18 for keysend - for example I believe lnd uses 13 instead of 9 today for keysend
20:29:51 <t-bast> rusty: YES, the goal is to say: just use a high enough values when in doubt, because it's not harmful in any way
20:30:09 <ariard> t-bast: right, but why if upgraded nodes just send 18 by default as sender, it's above old minimal cltv_expiry and is going to be accepted by non-upgraded?
20:30:11 <bitconner> why does 9 vs 13 matter?
20:30:29 <bitconner> fwiw lnd pads the cltvs in the path with and extra 3 blocks, so 18 -> 21
20:30:54 <ariard> I agree with bitconner, doesn't need to affect BOLT 11, implementations should reject invoices with values under 18
20:30:58 <t-bast> ariard: yes they're going to be accepted by non-upgraded since the check is `actual_ctlv_expiry > my_expiry_from_invcoie`
20:32:07 <cdecker> bitconner as an aside: does lnd also insist on receiving the +1 we add for the case a block is found in the meantime? I found that if I send with a final cltv of 9 (which is the current default) some nodes would reject, but not if I do the courtesy +1
20:32:12 <t-bast> bitconner: simply that if I try to send you a keysend with the default recommendation of 9, will you accept it? your code makes it look like you won't because you want 13
20:32:53 <t-bast> bitconner: but I just skimmed through it, I'm probably wrong
20:32:54 <ariard> cdecker: we do a courtesy +3 blocks, as block processing can be really slow on shitty devices
20:33:11 <t-bast> interesting, we only do a +1 courtesy on eclair, maybe we should bump that
20:33:17 <bitconner> cdecker: we add a few blocks in case blocks are found while the htlc is in transit, yes. but it is only done by the sender, nothing changes on the receiver side
20:33:18 <cdecker> That's on the sender side though ariard, correct?
20:33:38 <bitconner> ariard: lnd also does +3
20:33:55 <ariard> cdecker: yes, on the sender-side, you don't have issue on the receiver-side to be in the future?
20:34:01 <cdecker> Hm, still hunting for the implementation that didn't add a final cltv to its invoices and was insisting on getting a 10 instead of a 9
20:34:26 <bitconner> t-bast: i'm pretty sure we'll accept it with 9, but i'd have to double check on that
20:34:28 <rusty> I think we're in bikeshed territory.  We need to specify a  (deprecated) default, and 9 and 18 are the obv.  Can we have a straight up vote?
20:34:57 <bitconner> sgtm
20:34:58 <rusty> (TBH I don't really care, it's not worth delaying this PR for either value!)
20:35:07 <rusty> 18.
20:35:10 * cdecker votes for 18 being the default, and making the field mandatory for future implementations
20:35:26 <t-bast> yes let's vote, I vote for 18
20:35:27 <ariard> rusty: I think people disagree on scope should 18 be both on a) mimimal_cltv_expirty and b) on invoices?
20:35:34 <cdecker> (i.e., keep it as the PR proposes)
20:35:48 <ariard> if it just minimal_cltv_expiry, I vote 18
20:35:56 <bitconner> i'd say 9 because bolt 11 is one of the most widely implemented parts of lightning, and there's no need to change something we're going to delete and is obsoleted by just setting an explicit value
20:36:29 <bitconner> (which we're already doing anyways)
20:36:43 <t-bast> but only the encoding/decoding part is what's implemented very widely, not this check which is really when constructing the onion and paying, right?
20:36:58 <rusty> bitconner: because it's widely implemented I'm not sure we're ever going to be able to remove default... but I mildly prefer a consistent "TL;DR: use 18".
20:37:14 <roasbeef> well when we update, all the wallets do essentially
20:37:20 <ariard> bitconner: why not un-tie final_cltv_expiry verification from invoices matching at HTLC reception ? IIRC lnd does this, but post-payment_secret not an issue anymore ?
20:37:42 <ariard> or at least removing the cltv_expiry requirement for invoices post-payment_secret?
20:37:55 <roasbeef> 18?
20:38:40 <bitconner> ariard: why so? certain payment might require longer timeouts, like if the payment is composed with external protocols
20:39:07 <rusty> (yes, I specifically have a Bwahahaha plan for using longer values!)
20:39:25 <roasbeef> yeh or if it's been excessively sharded and all needs to be resolvd on-chain
20:39:35 <cdecker> +1 for keeping  the `c` field in the invoice
20:39:41 <t-bast> I'm seeing only one vote for 9...?
20:39:55 <bitconner> just doesn't realistic to update the assumed default at the same time we are changing to say you have to set an explicit value without breaking things
20:40:16 <roasbeef> bitconner: is it breaking though? ppl that accept and/or are expecting 9 should also accept 18 right?
20:40:29 <roasbeef> up to the max cltv limit
20:40:39 <bitconner> it is breaking if the receiver starts to assume 18 while the sender is still assuming 9
20:40:57 <roasbeef> mhmm, so all receivers that assume 18 should also set the value explcitily in the invoice
20:41:08 <rusty> bitconner: they can't do that, since spec says they *have* to set it?
20:41:32 <bitconner> rusty: there is still a transition period, the spec doesn't instantly update all deployments :)
20:41:38 <t-bast> roasbeef: exactly, it's not breaking anyone. If you're before that change, you will accept 9 when receiving. If you're after that change, you will specify a value in your invoice so even old senders will comply. You will send a higher value than necessary to old receivers. The only issue may be with your pending invoices
20:41:52 <roasbeef> t-bast: yep
20:41:54 <ariard> bitconner: you're right that's an application-check, but overall sems to me we should remove this change to require an explicit value
20:42:12 <rusty> bitconner: no, old deployments will write nothing IFF it's 9.  In which case, reader assuming 18 works.,
20:42:16 <bitconner> ariard: agree 100%, it's just a matter of staggering the deployment
20:42:18 <roasbeef> there may be some outlier wallets like BLW (idk how active they are anymore), but for the most part we should be able to deploy the change smoothly
20:42:34 <bitconner> rusty: not if you read your own invoice
20:43:19 <bitconner> anyway, if people want to change the spec to 18 that's fine
20:43:21 <t-bast> the only trick in implementation is that if you read your own invoice and it didn't specify anything, you'll want to accept 9 for a while, just until those invoices have expired
20:43:33 <t-bast> have expired or been paid
20:43:33 <rusty> bitconner: you mean create an invoice pre-upgrade, read it post upgrade and interpret it differently?  I cannot think of a case where that will be a problem (at least, for c-lightning)
20:43:40 <bitconner> just saying it probably won't land in lnd overnight bc of the rollout
20:44:13 <t-bast> it takes 2 releases to do it perfectly cleanly, but that shouldn't prevent us from changing it in the spec I think
20:44:43 <bitconner> t-bast: doesn't that prove that it is not backwards compatible? the fact that you have to wait for them to expire?
20:45:09 <t-bast> bitconner: no it doesn't!
20:45:26 <t-bast> bitconner: because it's on the receiver side that you do that trick, the spec specifies this default for the sender side
20:45:48 <t-bast> bitconner: because as a receiver you now MUST set it explicitly
20:46:04 <bitconner> i don't see that distinction? the default is being set in the field description, which i assume applies to both parties
20:46:22 <t-bast> bitconner: you just allow your implementation to carry over some left-over from things created before the release, which is expected
20:46:38 <bitconner> i see, it is actually specified in two palces
20:46:41 <bitconner> places*
20:47:13 <bitconner> like i said, if we want to change to 18 that's fine. it is all moot if we are setting explicit values
20:47:28 <bitconner> idk how much more time we should spend on this tbh
20:47:45 <t-bast> great! so are we good to go with the current state of the PR?
20:48:01 <t-bast> or do we want to have another look and sign-off on github later?
20:48:50 <rusty> t-bast: approved!  Kill me now (or at least get some more coffee...!)
20:48:57 <bitconner> i'll take a last look on github
20:49:12 <t-bast> sgtm
20:49:27 <t-bast> #action bitconner and t-bast to finalize on github
20:49:47 <t-bast> #topic ChaCha20 RFC correction
20:49:50 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/763
20:50:09 <t-bast> So the RFC we've been linking to for ChaCha20-Poly1305 is obsoleted by another one
20:50:14 <cdecker> lgtm
20:50:16 <t-bast> There are a few erratas as well
20:50:28 <rusty> Ack.
20:50:40 <cdecker> Couldn't find anything that'd impact us
20:50:40 <t-bast> There's no change in any implementation in the new RFC, so what we're doing today should comply with the new one
20:52:36 <roasbeef> yeh prob just best to link to the latest stuff, tho i don't see chacha really changing much at this point given msot ppl have pretty standarized libs
20:53:16 <t-bast> agreed, it just feels better, we won't have people twitting that LN is using insecure, obsolete crypto xD
20:53:33 <t-bast> shall we apply?
20:53:53 <rusty> (They added a third round, it's now ChaChaCha... turns out ChaCha was no more secure than single-cha!)
20:54:00 <rusty> t-bast: yes.
20:54:33 <t-bast> roasbeef/bitconner?
20:54:51 <roasbeef> oui
20:54:58 <bitconner> approved
20:55:06 <t-bast> nice!
20:55:11 <t-bast> #action merge PR
20:55:22 <t-bast> #topic Uppercase invoices
20:55:24 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/677
20:55:48 <cdecker> ack
20:56:47 <t-bast> this PR simply makes it explicit that uppercase invoices are ok, since it's more efficient to use in QR codes and wallet usually do that
20:57:03 <rusty> Yes, more test vectors good.  And they pass, so....
20:57:05 <t-bast> I believe everyone already does that
20:57:59 <roasbeef> hmm yeh dunno we don't have QR code in lnd, but I _think_ wallets on lnd do use upper case?
20:58:18 <roasbeef> we also had a PR to lnd to "fix" what we had as a max invoice size given what can be actually encoded in QR code
20:58:24 <bitconner> why is calling toLowerCase before calling bolt11.decode insufficient/
20:58:25 <bitconner> ?
20:58:54 <bitconner> doesn't seem strictly necessary to have this in the spec but either way works for me :)
20:58:55 <rusty> bitconner: technically you don't have to handle mixed case?  The orig bech32 code enforces only one case at decode time.
20:59:06 <cdecker> bitconner that's an implementation detail, but you should accept upper case ones
20:59:13 <rusty> bitconner: yeah, the modified test vector is more important IMHO.
20:59:50 <bitconner> sgtm
21:00:25 <t-bast> allright, let's apply!
21:00:28 <rusty> Yeah, ack from me.
21:00:45 <bitconner> ACK
21:00:53 <t-bast> #action Merge #677
21:01:09 <t-bast> #topic Anchor outputs (landing soon!)
21:01:21 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/688
21:01:35 <t-bast> I think we cleared that last outstanding question on anchor outputs regarding closing fees
21:01:40 <rusty> I've been slack on testing... we also need closure on.... um.. closing?
21:01:43 <t-bast> johanth are you satisfied with rusty's answer?
21:02:08 <t-bast> I've been able to test all normal operations and force-close scenario with lnd last week
21:03:06 <lndbot> <johanth> Yeah, that makes it clear
21:03:20 <t-bast> rusty: damn, there's a conflict in #677, could you fix it and re-push?
21:03:23 <rusty> t-bast:  do you have a node I can test against?
21:03:28 <rusty> t-bast: will fix...
21:04:02 <rusty> (BTW, 024b9a1fa8e006f1e3937f65f66c408e6da8e1ca728ea43222a7381df1cc449605 my node is running --experimental-features, so has anchor output support)
21:04:04 <t-bast> rusty: I can get probably it deployed on our testnet node next week
21:04:35 <t-bast> rusty: otherwise eclair at that commit has full support: https://github.com/ACINQ/eclair/pull/1501
21:04:47 <rusty> t-bast: OK, but anyway if you two have interop I'm happy to ack this!
21:04:48 <t-bast> rusty: but you may not want the JVM near your machines xD
21:05:14 <t-bast> We still have the closing fee to fix, it's the only remaining thing that still doesn't match between eclair and lnd
21:05:22 <lndbot> <johanth> Will need to update the feature bit and the coop close fee computation, otherwise it think we are there :)
21:05:35 <t-bast> johanth do you think you can have something ready soon with that fixed? Once you do I can do interop testing
21:05:50 <lndbot> <johanth> Yeah, can have it tmrw probably
21:06:10 <t-bast> oh great, we'll be able to finalize this quickly, let me know over slack when you have a branch I can use!
21:06:21 <lndbot> <johanth> :+1:
21:06:42 <t-bast> #action johanth and t-bast to finalize interop tests this week
21:07:18 <t-bast> Do you want to discuss one last topic? Or shall we call it a day?
21:08:09 <bitconner> i have some time
21:08:24 <bitconner> know it's late for europe folks tho :)
21:08:33 <ariard> rusty: If you do have a bit of time to laying out your ideas on better RBF/package relay there : https://github.com/bitcoin/bitcoin/issues/14895, that's welcome :)
21:08:35 <rusty> (Aside #1/2: I found someone to hack on ln.dev, first I'd like to create a spec features vs major releases matrix for my own reference, so features.ln.dev or something...)
21:08:36 <t-bast> What topic do you want to chat about?
21:08:54 <ariard> sipa was describing an idea of yours to stagger cheap replacements
21:09:12 <t-bast> oh yeah doing a bit of package relay is a great idea, let's bridge layers
21:09:15 <rusty> (Aside #2/2: I am supposed to be giving a live thing on lnprototest in ~24 hours)
21:09:41 <t-bast> rusty: nice! tweet the link, we'll watch ;)
21:11:03 <ariard> t-bast: I think we agree on a "few core assumptions that need to remain "loosely true"
21:11:47 <ariard> like stable policy rules, and being sure they're not changed under our feets
21:12:42 <t-bast> Yeah, I figured that's what we were both thinking. What do you think of my suggestion of only allowing much smaller packages in mempool?
21:13:17 <t-bast> I can't see the usecase for allowing very long chains of unconfirmed txs in the mempool, I believe that complexity could shift to clients instead of having nodes take that burden
21:13:31 <t-bast> But I'm likely missing use-cases
21:13:49 <rusty> t-bast: ah, we included an upper-case test-vector in another cleanup. So removing from PR.
21:14:18 <t-bast> rusty: got it, that's why I already had a slightly different uppercase test vector in my test suite!
21:14:25 <ariard> t-bast: I'm more worried with absolute-fee-but-lower-feerate obstrucating package rather than irreplaceable packages due to limits
21:15:44 <ariard> and I verify bip 125 rule 5 recently, you will hit first ancestor/descendants limits before to reach 100 descendants case
21:16:06 <ariard> unless an intentional reorg, where these limits aren't applied
21:16:45 <t-bast> ariard: I think we need to fix both, but I was under the impression that people felt the absolute-fee-lower-feerate rule would not be too hard to lift, that's why I wanted to mention the other painful rule
21:17:11 <rusty> t-bast: #677 rebased.
21:17:44 <t-bast> rusty: great, even more no-brainer to apply, I'll do it right now
21:20:03 <t-bast> ariard: what scenario uses big unconfirmed packages?
21:20:29 <roasbeef> t-bast: potentially ctv type stuff, or anything that uses large transaction trees as part of the protocol, like duplex potentially
21:20:33 <t-bast> ariard: though big is vague, I only mean in number of distinct txs
21:20:38 <ariard> t-bast: iterative batching where you reuse a change output to bump new spends but I think no exchange does it
21:20:45 <roasbeef> or if ppl start using new crazy commitment schemes after some form of dynamic ocmimtment stuff is rolled out
21:20:51 <t-bast> roasbeef: but does CTV require these long chains to all be in the mempool?
21:20:53 <roasbeef> also not sure what bitmex's withdrawl transaction looks like
21:21:01 <roasbeef> t-bast: depends....
21:21:13 <t-bast> roasbeef: I was under the impression that the client could get these txs locally and then broadcast batches by batches as some confirm
21:21:20 <ariard> yeah it's really depends how people design this-not-yet existent things
21:21:32 <ariard> but you may use the anchor trick of 1 CSV to avoid issue
21:22:08 <t-bast> airard: this could potentially be long but it will likely only use a few txs, it doesn't need a chain of 100 of them right>
21:22:39 <ariard> t-bast: wrt to absolute-fee lifting I think it would work, but not the RBF-pinning resolution as proposed by Suhas I think
21:23:37 <t-bast> you mean the explicit flag he suggests wouldn't work? https://github.com/bitcoin/bitcoin/issues/14895#issuecomment-665969902
21:24:22 <ariard> t-bast: you can still obstrucate at the commitment-level, like using old commitment with lower-feerate higher absolute-fee
21:24:33 <t-bast> this proposal requires us to go in the direction of pre-signing both sides of the HTLC outputs (claiming from the remote commitment will need to be done from a pre-signed tx)
21:24:40 <ariard> so lifting the rules need to apply on oldest ancestor too, not only children
21:25:30 <ariard> t-bast: no? even a malleable HTLC if low-feerate with such rules would be rejected
21:26:22 <t-bast> you mean it wouldn't work either way (even if we pre-signed)?
21:27:25 <t-bast> I didn't realize I forgot to end the meeting, looks like we're the only ones discussing this so let's stop recording
21:27:29 <t-bast> #endmeeting