19:10:32 <roasbeef> #startmeeting
19:10:32 <lightningbot> Meeting started Mon Oct 28 19:10:32 2019 UTC.  The chair is roasbeef. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:10:32 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:10:42 <roasbeef> cdecker: yeh it's newish, just was put up today
19:11:11 <cdecker> Shall we cross off some old favorites first?
19:11:49 <roasbeef> #action merged #660
19:11:58 <roasbeef> #672 needs a rebase though, but its g2g other than that
19:12:07 <roasbeef> (allowing greater segwit versions)
19:12:28 <roasbeef> cdecker: which faves?
19:12:28 <rusty> Ack.
19:12:56 <roasbeef> i think we're more or less olidfieid on the mmp payload now? (so combine both value and paymetn addr under a single record)
19:13:06 <cdecker> Nothing, just wanted to start with the old ones first, before getting off track with #688 :-)
19:13:52 <cdecker> roasbeef: sounds reasonable to me
19:13:53 <roasbeef> gotcha, ok going backwards...
19:14:01 <roasbeef> #topic 571 (feature bit renaming)
19:14:23 <roasbeef> atually this was replaced byy #666 I think
19:14:37 <roasbeef> rusty: can we close 571 in favor of 666?
19:14:44 <rusty> Yeah, cwithdrawing now.
19:15:34 <roasbeef> iirc this is mostly g2g, just need final comement clean up, and also CL and eclair both have some form of this rolled out already
19:16:02 <roasbeef> and to ensure backwards compat is maintained (was my main comment)
19:16:15 <rusty> Yeah, I will update it now.
19:16:20 <roasbeef> ok cool
19:16:39 <cdecker> Nice, that gets one of the oldest open PRs off the list ^^
19:16:50 <roasbeef> #action rusty to update #666 after latest round of comments and meatspace discussion
19:17:09 <roasbeef> ok wumbo...
19:17:21 <roasbeef> #topic 596 wumbo bit for channels
19:17:28 <roasbeef> I guess this is interdependent on #666 right?
19:17:33 <roasbeef> as it informs where the bit is placed
19:17:47 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/596
19:18:36 <sstone> roasbeef: yes it is
19:18:44 <cdecker> Needs to be rebased and truncated down to just the changes necessary for the wumbo bit (if any remain after merging #666)
19:18:50 <roasbeef> ok gotcha
19:19:10 <roasbeef> #info 596 has dep on 666, needs to be rebased as well once 666 is in
19:19:24 <roasbeef> #topic 620
19:19:31 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/620
19:19:55 <roasbeef> #info clarification on message placement in bolts (ann sig)
19:20:01 <roasbeef> i don't think we ever reached any rough consensus on this?
19:20:12 <sstone> nope :)
19:20:20 <roasbeef> the origin of this was ambiguitiy in which order of messages to expect
19:20:25 <roasbeef> (chan reest vs ann sig)
19:21:05 <cdecker> Yeah, announcement_signature pretty clearly is bound to a channel, and should therefore be subject to the ordering requirement
19:21:19 <sstone> yes that was our take too, but it'
19:21:32 <sstone> it's not that obvious in the specs
19:22:17 <roasbeef> imo they're on diff levels
19:22:23 <roasbeef> chan reest is link re-establishment
19:22:31 <roasbeef> ann sig is a gossip message for the broader network
19:22:46 <roasbeef> i can use the channel as soon as chan reest is sent, and ann sig isn't bound to that lifecycle
19:23:55 <sstone> but you still need to send funding_locked before ann_sigs
19:24:13 <sstone> so it's bound to the channel lifecycle
19:24:36 <roasbeef> sure, but this is about chan reest and ann sig
19:24:56 <roasbeef> even then with funding locked they're distinct imo
19:24:58 <cdecker> Good point sstone: there are already ordering constraints, so why explicitly relax them here?
19:25:00 <roasbeef> as the events can happen concurrently
19:25:10 <rusty> We have "gossip messages" and "message in bolt 7".  They're not synonymous, but not defined.
19:25:12 <roasbeef> which only comes into palce past 6 confs
19:25:30 <roasbeef> but in general we should also modify that 6 conf value, it doesn't make sense as a blanket value, especially after wumbo
19:25:32 <rusty> The implication was "gossip messages" are re-xmitted, others are inter-peer.
19:26:34 <cdecker> Yep, "gossip messages" mean something to others and are broadcast, announcement_signature and the query messages are not broadcast and only have a meaning in the context of the current connection
19:26:38 <rusty> The spec says" bolt 7 messages" in one place where it means "gossip messages".  But these bolt 7 messages are msgs about setting up gossip, so it's confiusing.
19:27:21 <roasbeef> doesn't seem like we're really getting anywhere with this, the OP of the PR isn't here, but iirc his intention was to update the spec to match existing behavior in the wild and attempt to make it less ambigious, not sur ehow far we can get since they're not here
19:27:26 <roasbeef> the diff in teh PR is also pretty tiny
19:27:44 <roasbeef> names that ann sig is independent of the channel life cycle (which it is, you can not send it but the channel is fully usable)
19:28:08 <cdecker> Yes, but it corrects the wrong way around, really sending the messages in a random order is an implementation error imho
19:28:33 <rusty> But its exactly wrong.  The sentence in BOLT 2 shoujld be changed from "Note that messages described in [BOLT #7](07-routing-gossip.md) are independent of particular channels; their transmission requirements are covered there, and besides being transmitted after `init` (as all messages are), they are independent of requirements here." to "Note that *gossip* messages described...."
19:29:39 <cdecker> Let's not get hung up on gossip messages vs bolt 7 messages, let's just enumerate all the ones that are permissible: channel_announcement, channel_update and node_announcement, nothing else is permissible
19:29:41 <roasbeef> in the end "channel lifecycle" isn't defined anywhere really
19:31:27 <sstone> ann sigs is already special since it is explicitely defined as "not a gossip message"
19:31:51 <rusty> cdecker: language tried to be generic for future schnorrstyle msgs.
19:31:55 <roasbeef> ok i guesrs we shoudl move on? and maybe export some of this in the PR? this is all about semantics at the end of the day, thigns are ambgiuous in the spec, and we all seem to have diff models of it in our heads
19:32:50 <roasbeef> cdecker: permissible as in?
19:34:53 <rusty> OK,fixed up 666 BTW.
19:35:14 <roasbeef> #topic 625
19:35:21 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/625
19:35:24 <roasbeef> this is related lol
19:35:34 <roasbeef> attempts to define an ordering
19:35:40 <roasbeef> on funding locked and ann sig
19:35:51 <roasbeef> since the two events can be triggered concurrently
19:36:21 <rusty> No, it attempts to undefine the existing ordering.  And it's wrong.  We can't announce until we agree on the scid, which isn't finalized until funding_locked,
19:37:01 <roasbeef> it's saying that if the depth is 15, wait to send the ann sig
19:37:14 <roasbeef> don't send it after 6, since funding locked will be sent after 15 blocks
19:38:28 <roasbeef> otherwise ann sig is always sent after 6, which would be before funding locked
19:38:40 <cdecker> Yes, but the dependency on funding_locked is stronger since that also has to wait for the negotiated funding_depth
19:38:44 <roasbeef> (once again 6 should be removed imo and just use the min depth for everything)
19:39:50 <roasbeef> iirc this was created after 620, so perhaps they're alternatives
19:40:06 <niftynei> i'm a bit confused by the `max` function in this change, fwiw. i'd expect the `minimum_depth` to supercede the 'default' of 6, but this implies you'd use 6 as the floor regardless of what the minimum_depth is
19:40:10 <cdecker> If we remove the 6 some implementations might start sending out gossip messages that are not valid according to the broadcast rules, so the 6 in there is a way for us to avoid that
19:40:53 <rusty> roasbeef: yeah, 6 is to protect the gossip network from reorgs.  But you simply can't exchange sigs until both sides agree the funding tx is final.
19:40:54 <cdecker> The problem is that the depth of 6 is a global limit, whereas the funding_locked is a local one
19:40:58 <roasbeef> cdecker: post wumbo, depending on channel ranges, 6 isn't an acceptable value to use everywhere
19:41:10 <niftynei> an easier fix would be to just truncate the six confirmations and pin the `announcement_sig` to an exchange of `funding_locked`, no?
19:41:14 <roasbeef> it isn't even really well defined what to do in the case of a re-org, as yu'd need to generate a new ann sig and advertisement
19:41:28 <rusty> niftynei: *it already is*.
19:41:40 <cdecker> roasbeef: yes, you can use a higher funding_depth value to require more (and implicitly delay the gossip, which is what I was saying)
19:43:05 <niftynei> what's the motivation here for removing the `funding_locked` exchange requirement?
19:43:10 <cdecker> Otherwise I don't see how wumbo comes into play here (I expect with higher value channels you'd want more confirmations)
19:43:20 <rusty> (TBH, I'm not sure what will happen to our gossip with a > 6 reorg, cdecker will we implicitly close re-orged out channels from gossip?)
19:43:23 <roasbeef> niftynei: not about removing it, about trying to define a more strict ordering
19:43:39 <niftynei> this change removes the requirement that `funding_locked` is exchanged
19:44:01 <rusty> roasbeef: No, this *explicitly* removes the "stricter ordering".
19:44:02 <cdecker> rusty: good question, I think we might keep the stale entries in gossipd
19:44:03 <roasbeef> no, it's even in a diff doc that funding_locked
19:44:31 <rusty> roasbeef: "but removes the stricter ordering requirements wrt funding_locked and (indirectly) channel_reestablish"... from first para of desc.
19:44:43 <rusty> And the diff shows it removing that.
19:45:14 <roasbeef> 15 mins left, shall we move on?
19:45:22 <roasbeef> OP not here again
19:45:28 <cdecker> Yes please :-)
19:45:33 <roasbeef> #topic 658 (amp)
19:45:39 <roasbeef> err no
19:45:47 <roasbeef> #topic #643 (mpp)
19:45:55 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/643
19:46:05 <roasbeef> final thing iirc is unifying the payloads?
19:46:14 <rusty> Note I got off my butt and finally updated this a few minutes ago :)
19:46:25 <roasbeef> i think most of us have some type of prototype of this, so we could start interop testing soon after it lands
19:46:29 <roasbeef> ah nice
19:46:44 <roasbeef> ok cool I see that it's combined, will comment on the spec PR
19:46:57 <roasbeef> #info payload now combines payment addr and total amt
19:47:30 <rusty> Yeah, we now are assuming everyone supplies a secret.  No point trying to handle the TLV-but-no-secret case.  No secret, no AMP for you!
19:47:45 <roasbeef> sgtm, e2e security ftw
19:47:55 <rusty> Ack!
19:48:03 <roasbeef> ok, then we'll hash out the rest in the PR ideally to have it on the merge train by the next meeting?
19:48:03 <cdecker> SGTM
19:48:12 <roasbeef> cool
19:48:23 <roasbeef> #info review to continue on PR, aiming to catch merge train in 2 weeks
19:48:28 <rusty> Yep!  Two Weeks(TM) should be plenty of time to implement.
19:48:34 <roasbeef> hehe
19:48:39 <roasbeef> #topic 658 (amp)
19:48:47 <roasbeef> I think this was waiting on if the pyalods were gonna be unified
19:48:55 <roasbeef> but they are now, so conner can start to update this when he's around again
19:49:09 <roasbeef> #action paylaods now unified, conner to update PR to reflect new paylaod structure
19:49:11 <joostjgr> hi guys, I am present now. i expected the meeting to be just one hour earlier because winter time
19:49:21 <roasbeef> joostjgr: heh yeh timezones are the greatest
19:49:33 <roasbeef> #topic 671 (invoice secret)
19:49:39 <roasbeef> I think we don't need this anymore?
19:49:50 <roasbeef> or nah this is for the invoice
19:50:11 <rusty> Side note: we found a "Bad commit_tx signature" error (it's been *years* since we had one of those!) in our implementation, w/ fee changes and reconnect and simultanous HTLCs.  niftynei slid the fix in JIT for current pending release.
19:50:26 <roasbeef> rusty: CL to CL?
19:51:27 <roasbeef> oh I guess there was a questinon on invoice vs everythign else feature bit namespaces too in berlin
19:51:28 <rusty> roasbeef: yeah, that clinched it and made me finally stress enough to repro.  But we saw some previous CL->LND under load, which shamefully I dismissed and blamed you.
19:51:44 <roasbeef> as is, this makes a new namespace (which is correct imo), but som ehave argued that they should all be unified
19:51:47 <rusty> Yeah, we don't *have* a feature bit for this though.
19:51:54 <roasbeef> it uses 0/1
19:51:56 <roasbeef> in the PR atm
19:52:09 <rusty> I mean, there's no "secret" feature bit in bolt 9.
19:52:17 <rusty> Unless we (ab)use the tlv_onion feature/.
19:52:38 <roasbeef> ahh gotcha
19:52:58 <roasbeef> well are they to be defined in bolt 11 for invoices or just everything in bolt 9?
19:53:04 <joostjgr> tbc, the idea is to combine signaling mpp and the secret to a single invoice flag, isn't it?
19:53:06 <roasbeef> rn the PR makes a new invoice feature bit namespace essentially
19:53:50 <rusty> joostjgr: we'd have to make a feature for it.  It's really only reqd for bolt 11, so having a "I support MPP" in (say) node_announce is just showing off :)
19:54:14 <rusty> (Which is not a bad thing, necessarily!)
19:54:51 <rusty> I'm on the fence, but we can define 12/13 for "MPP", put it in BOLT9 and then use it here?
19:55:11 <roasbeef> sounds like that works, no strong feeling on my end, joostjgr ?
19:55:12 <rusty> (If we want to aim for unification)
19:55:57 <joostjgr> yes, i understand we need the flag in the invoice. but just double checking that it will just be a single flag that combines mpp and secret. just as we combined it in the tlv payload
19:56:18 <joostjgr> or is this mpp invoice flag signaling: i support the mpp protocol, but only except single shard payments?
19:56:19 <roasbeef> yes it seems so, 671 will need to be updated to reflect the desired semantics
19:56:28 <roasbeef> ahh i see what you're saying
19:56:40 <roasbeef> so if you signal this, then you also need to be able to combine payment shards
19:56:51 <roasbeef> and not just insta settle (which is really important for mpp!!)
19:57:03 <rusty> joostjgr: Ah, very good point!  Yeah, the presence of the *secret* in b11 indicated you support the TLV format.  The bit means actually takes MPP.
19:57:32 <joostjgr> well there are three flavours: old style single shot payments, mpp payments that only have single shard (payload does contain total amt + secret) and mpp payments that consists of multiple shards. the idea of the single shot mpp payments was possibly for low powered devices
19:57:49 <joostjgr> i am not sold on that single-shot mpp only variation, would rather wait for this to actually emerge
19:58:10 <rusty> joostjgr: yeah, I'd prefer it not to exist, but is it already in the wild?
19:58:18 <joostjgr> is it?
19:58:23 <rusty> Not for c-lightning.
19:58:33 <roasbeef> joostjgr: single shot variation/ so only 1 payment?
19:58:42 <joostjgr> we didn't release anything that decodes mpp payloads
19:59:09 <joostjgr> single shot mpp is a single htlc that does carry the mpp payload (totalamt = htlcamt and there's a secret)
19:59:14 <roasbeef> single shot would be the donation case I think
19:59:18 <rusty> roasbeef: yeah, do you create TLV onions for final dest at the moment? i.e. w/o secret & total amount ?
19:59:40 <roasbeef> the code is there to do so, but it isn't on any api yet
19:59:44 <roasbeef> we just support the new/old formats
20:00:11 <rusty> Hmm, that'd be nice to avoid.  We'd never get a TLV payload w/o the secret (and total amount).
20:00:37 <joostjgr> it is too late for that in lnd
20:00:45 <roasbeef> joostjgr: too late for what?
20:00:56 <joostjgr> we already send tlv payloads without the usual fields
20:01:06 <roasbeef> no we don't...
20:01:10 <roasbeef> w/o the usual?
20:01:19 <joostjgr> oh sorry, with the usual
20:01:34 <joostjgr> so the same old fields in a tlv. that is what you rusty are referring to, isn't it?
20:01:36 <roasbeef> yes we can send the tlv payload with only the reqwuired information as is
20:01:43 <roasbeef> if you signal it
20:01:45 <rusty> joostjgr: Ah, if final node's node_announcement indicates support?  OK, thought so.
20:02:04 <joostjgr> yes, if it signals support
20:02:05 <rusty> OK, no problem.  We can eventually remove this, but implementations must handle it.
20:02:11 <roasbeef> yeh
20:02:42 <roasbeef> #action 671 to clarify being able to signal for mpp (can combine) and just presence of secret that must be included (e2e security for donation case)
20:02:42 <rusty> (Evenetually we'll be insisting on the secret, so we deprecate this along with legacy I guess)
20:02:57 <roasbeef> and the path to do so is the even/odd bits rusty ?
20:03:04 <roasbeef> it does allocate a pair
20:03:06 <roasbeef> as is
20:03:45 <rusty> roasbeef: hmm, good q.  If we *don't support* MPP, we won't have a feature bit in th b11 invoice.  We'll just hve the secret.
20:03:55 <rusty> (Assuming FB unification).
20:03:59 <roasbeef> mhmm
20:04:29 <rusty> I mean, we'd better not deprecate it until there's ~nobody left using it, but still.
20:05:05 <rusty> "Correct" solution is YA feature bit, for "suopports secret".
20:05:10 <roasbeef> yeh could be even there's some brand new invoice format as well, so would be a way to do the hard hard swtich over
20:05:39 <rusty> roasbeef: TLV like the offer format, damn tempting.  But the churn is annoying...
20:05:41 <roasbeef> rusty: if you rebase #672, we can merge it
20:06:12 <roasbeef> i'm gonna clsoe #680, we do it in the wild, seems to be compat w/ others, #66 on the way
20:06:28 <rusty> Ack.
20:06:43 <rusty> roasbeef: Can we do 682?
20:06:43 <roasbeef> ok last one (np if ppl can't stick around)
20:07:12 <roasbeef> ah didn't see it labelled
20:08:02 <roasbeef> ok actions on this seem to be specyfing what to do w/ unknown chains?
20:08:10 <roasbeef> or rather not a complete intersection?
20:08:15 <roasbeef> #topic 682
20:08:20 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/682
20:08:49 <roasbeef> hmm shouldn't we have one big tlv namespace at the end?
20:08:50 <rusty> roasbeef: yeah, I didn't specify. SHOULD close connection?
20:09:01 <roasbeef> otherwise parsing can be weird, if you don't support this, but do support w/e the next init extension is
20:09:17 <roasbeef> sstone: does the new acinq node add any innt level tlvs?
20:09:46 <rusty> roasbeef: it does add this field via adding a TLV at end.  I'm confused...
20:09:50 <sstone> roasbeef: no we don't
20:10:03 <roasbeef> ah nvm didn't read it properly
20:10:18 <roasbeef> thought it meant like "this is where this new tlv goes", rather than "this is where all the init tlvs go
20:10:21 <roasbeef> "
20:10:45 <roasbeef> conner is the one that looked at it first it seems, btu not here atm, can add an action for him to follow up on it
20:10:51 <roasbeef> but no major comments on my end
20:10:57 <rusty> Ah, I forgot that the spec actually says you MAY fail if no intersection.  Don't think we need to go further with partial intersectino, I mean, depends right?
20:11:14 <rusty> Agreed, let's get bitconner's input here/.
20:11:26 <roasbeef> #action wait on input from conner to clarify comments
20:11:43 <roasbeef> #topic 688
20:11:48 <roasbeef> #info anchor outputs
20:11:50 <rusty> Ah, he wanted to restrict traffic based on what chains.  Makes sense, not to waste others' time & bw.
20:11:57 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/688
20:12:11 <roasbeef> joostjgr: wnana give us an overview, only a few hours old, so don't think anyof us have really looked at it
20:12:25 <joostjgr> yes, i took your pr rusty and modified with some latest insights
20:12:26 <roasbeef> main thing seems to be infection of other scripts by the csv 1, and diff handling for second level htlcs
20:12:47 <joostjgr> other changes are communicating the anchor addresses in the funding flow
20:12:57 <rusty> Damn, sorry I haven't reviewed :(  Need to go through this thoroughly sorry.
20:13:13 <joostjgr> and dropping all the tweaks
20:13:29 <niftynei> "infection" lol
20:13:43 <joostjgr> yes ofc, I just put it up today. but comments are welcome on the ML or PR
20:14:45 <roasbeef> joostjgr: why do they need to be anything but p2wsh op_true?
20:14:50 <roasbeef> w/ the sig from a new key
20:14:51 <rusty> joostjgr: Removing tweaks is a big change.  It means that a watchtower, seeing a single unilateral close, can derive the rest of the channel history.
20:14:52 <roasbeef> or an existing one
20:15:49 <roasbeef> yeh removing those doesn't seem necessary and just expands the scope of it even more (beyond csv 1 infection)
20:15:50 <joostjgr> op_true w/ the sig?
20:15:51 <rusty> Yeah, using anchors as P2WPKH is also massive spam.  It's *unecomonic* to spend dust.
20:15:52 <roasbeef> csv flu
20:16:24 <rusty> First thing someone will hack in the implementation is to remove spending these.
20:16:46 <joostjgr> you can set your dust limit
20:17:11 <rusty> joostjgr: but you're paying for that up-front, and it doesn't vary with fees.
20:17:38 <joostjgr> hmm, you mean if fees go up, they become uneconomic to spend
20:17:42 <rusty> Seems like this needs to be debated on issue though.  I gtg (found a coffee shop which opens at 5:30, but now have to head back and wrangle kids).
20:18:07 <rusty> joostjgr: I mean, we should always have some kind of dynamic dust limit, but that's a bigger change, too :(
20:19:06 <roasbeef> ok just wanted to intro it here, but now that the PR is up, we can hash things out on there
20:19:51 <joostjgr> yes, sounds good.
20:20:15 <cdecker> Yep, I'll read it asap and respond on the PR if I find something ^^
20:23:12 <roasbeef> #endmeeting