20:05:24 <cdecker> #startmeeting
20:05:24 <lightningbot> Meeting started Mon Apr 29 20:05:24 2019 UTC.  The chair is cdecker. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:05:24 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
20:05:26 <niftynei> :)
20:05:55 <t-bast> #info
20:05:57 <cdecker> The tag for today's meeting is here: https://github.com/lightningnetwork/lightning-rfc/issues?utf8=%E2%9C%93&q=label%3A%22Meeting+Discussion%22
20:06:22 <cdecker> Most of the things were discussed last time
20:06:47 <cdecker> t-bast: is sstone joining today?
20:07:01 <t-bast> sstone is out for the week
20:07:09 <t-bast> but I think andrea can speak for his progress on the range queries
20:07:25 <cdecker> Excellent, that was what I was hoping to get
20:07:41 <cdecker> #topic BOLT7: extend channel range queries with optional fields #557
20:08:04 <cdecker> Last time we deferred to wait for a TLVified version of the PR
20:08:14 <cdecker> I guess that is still pending?
20:08:37 <bitconner> the pr has been updated w/ tlv fields
20:08:57 <cdecker> Oh has it? I must've missed that
20:09:37 <bitconner> yeah it assumes tlv in rusty's current proposal, so might need some slight modification if that changes
20:09:38 <t-bast> there were some changes pushed last friday - I think sstone was now waiting for comments
20:10:13 <bitconner> got a chance to review last night, latest form of the proposal looks solid
20:10:13 <cdecker> I'm sorry I missed that, and didn't give feedback yet
20:12:42 <cdecker> Maybe you could clarify the format of the TLV here for me quickly: in `query_channel_range_tlv` the format would be a single dict so to speak with two possible keys (1 for timestamps_tlv and 3 for checksums_tlv), right?
20:13:10 <cdecker> The values then would back to back serialized arrays of timestamps and checksums respectively, right?
20:13:21 <bitconner> cdecker, yes i think so. i've always imagined there would be a single "tlv blob" with up to 255 keys
20:13:55 <cdecker> Excellent, that clarifies that
20:14:26 <cdecker> Other than that it appears to be just a 1-to-1 translation from feature bits to TLV
20:14:46 <cdecker> SGTM
20:15:03 <cdecker> What is the general feeling, did we torture this proposal long enough?
20:16:08 <bitconner> im happy with this latest iteration, i think most of the discussion now is on finalizing the exact tlv spec
20:16:54 <cdecker> I'm a bit worried about assigning the TLV types, since it seems we are about to make the TLV types context specific again, but other than that it looks good
20:17:20 <cdecker> Then again we can solve that later
20:17:39 <bitconner> what do you mean by context specific? as in they could be different for each message?
20:18:11 <cdecker> Yeah, we had this ambiguity for the feature bits as well. When they appear in one message or another their meaning could change
20:18:46 <cdecker> That was the basic idea behind Rusty's #571 Feature bit unification PR
20:19:15 <roasbeef> what's the ambiguity in the existing feature bits?
20:19:30 <roasbeef> from my pov the unification doesn't really clear anything up
20:19:38 <niftynei> my understanding for tlv types is that there's one tlv per message. each tlv has its own set of message types
20:20:04 <cdecker> Well, the hope was that we could have a single lookup table num->type instead of having to have one per message type since the mapping may be different
20:20:12 <bitconner> niftynei, same here
20:20:49 <cdecker> Ok, if that's the general interpretation I'm happy to accept that as well
20:20:53 <bitconner> imo
20:21:09 <bitconner> imo having a global namespace for keys/types would limit the effectiveness of the proposal
20:21:24 <niftynei> having a unified set significantly constrains the total number of sub-types that we can create for messages
20:21:42 <cdecker> In that case we must definitely amend the TLV PR from Rusty since that contains global TLV type specifications: https://github.com/lightningnetwork/lightning-rfc/pull/572/files#diff-9198bb316a3387cc67fd543b03339b35R651
20:22:20 <bitconner> was not aware, thanks
20:22:20 <cdecker> We just need to qualify them to be message specific mappings and then it works again
20:22:32 <cdecker> That's where my confusion comes from btw :-)
20:23:02 <bitconner> makes sense now :)
20:23:17 <bitconner> should we change the topic to tlv btw?
20:23:20 <cdecker> Funnily enough the TLV types in PR #557 seem to be chosen not to clash with the ones in the TLV PR
20:23:53 <cdecker> Ok, just one decision on PR #557 first? Defer or accept?
20:23:56 <niftynei> it seems like the tlv spec proposal should be updated to reflect that they're per tlv definition, not global
20:25:02 <cdecker> My vote would be to accept #557 and then switch to #572 (TLV) and amend that
20:25:06 <niftynei> (that's because even are required, odds are optional. the tlv-types in #572 are required whereas the #557 ones are intended to be optional)_
20:25:10 <bitconner> in favor of accepting #557, but it's kinda blocked by the tlb proposal
20:25:37 <bitconner> imo we shouldn't have the even/odd rule for wire tlv
20:25:51 <cdecker> Oh, but we should
20:25:59 <bitconner> what's the rationale?
20:26:11 <cdecker> For #557 for example everything works even if you don't understand the TLV fields
20:26:31 <bitconner> the same is true without the even odd rule, all fields are optional
20:26:33 <cdecker> There are other proposals where understanding a TLV field is important
20:26:53 <roasbeef> in either case we'd need it scope per message, otherwise there wouldn't be enough space or even the onion there
20:27:09 <cdecker> Yep, agreed with roasbeef
20:27:29 <roasbeef> why do we need even/odd for tlv?
20:27:51 <bitconner> if a field is mandatory, that should be signaled via a feature bit
20:27:58 <cdecker> It's just really useful to be able to say "it's ok for you to ignore this", and "you must understand this otherwise the world collapses"
20:28:26 <niftynei> right my initial understanding was that TLV fields are all optional by nature of their being in the TLV. rusty talked me out of it but i can't remember the rationale now
20:28:43 <roasbeef> yeah...if you don't understand ok, anythign else can be up front negotiated with feature bits
20:29:38 <bitconner> so far i think the only place even/odd adds value is in onion tlv, because we don't have feature bits
20:30:06 <bitconner> but that can just be an additional constraint on the onion keys/types, and not embedded in the base tlv format
20:31:12 <cdecker> Feature bits seems like an odd place to put message specific things though
20:31:26 <niftynei> right i think it depends on how you see the future of the TLV fieldset evolving. having a rule about even and odd allows us to expand the usage of TLVs to include mandatory fields
20:31:33 <niftynei> even if at the outset we're intending it for optional fields
20:31:49 <cdecker> We are splitting message capability negotiation over multiple fields
20:32:02 <cdecker> And not all TLVs are paired with a featurebitmap
20:32:24 <cdecker> There seems to be no harm in at least reserving even TLV types for now
20:32:36 <niftynei> right. i can imagine adding a featurebitmap to a TLV, as a required field, and the rest being optional.
20:32:57 <cdecker> Whoa, that's a featurebit / TLV inception :-)
20:33:02 <niftynei> too meta? lol
20:33:07 <cdecker> #topic Specify tlv format, initial MPP #572
20:33:22 <cdecker> Switching since we are now very much in #572 territory
20:33:25 <cdecker> :-)
20:33:25 <roasbeef> why do we need to reserve them ahead of time if they're all message specific?
20:33:58 <niftynei> oh good point. the type number is basically a featurebitmap mapping
20:34:15 <cdecker> Because even if they are message specific you want to tell some old version of yourself that it is dangerous to continue if you don't understand a certain field
20:35:56 <roasbeef> "continue" is very context specific
20:36:12 <roasbeef> even then, most of the time one party can tell and just not include that field
20:36:23 <roasbeef> for example anythig in the onion may restrict the path you can actually take
20:36:28 <cdecker> Well, then let's safe "you need to fail-safe if you don't understand the type I'm giving you"
20:37:06 <cdecker> s/safe/say/
20:37:13 <niftynei> that feels a bit like creating exploding honeypots of TLV's. it's hard to tell when incompatibilities are going to arise. the other way to deal with this would be creating a v2 of the message format and only communicate with peers that understand that message format. it's basically the same thing, no?
20:37:37 <niftynei> 'that' here meaning making mandatory TLV fields
20:38:04 <cdecker> I'm just uncomfortable with giving someone instructions (in an onion for example) and he might just ignore the important instructions because he's too old to know what I'm telling him
20:38:11 <roasbeef> niftynei: what does?
20:38:26 <roasbeef> cdecker: if they ignore them, the your protocol likley won't work, or they can just cancel it bac
20:38:57 <cdecker> Right, that's what I'm saying: they should cancel it back whenever they don't understand it
20:39:31 <cdecker> And they should know whether what I'm telling them is just additional information, as in the #557 case, or whether it is paramount for them to really understand things
20:40:32 <bitconner> these concerns seem to be specific to onion tlv iiuc?
20:41:05 <roasbeef> in what case would one provide additional information, that doesn't shape the route at all?
20:41:31 <roasbeef> or provide info that they can pass off for not verifying
20:41:39 <cdecker> Not really, let's say I want to tell my peer not to send me a gossip dump if it is > 10 MB, and the peer doesn't understand that, I will get flooded despite me trying to protect against that
20:42:02 <cdecker> There are likely cases of this kind of interaction in many other contexts as well
20:42:04 <roasbeef> gotcha, i'm speaking more about in the onion here while i think you're speaking on the msg lvl
20:42:15 <niftynei> ah nevermind. the onion use case is a bit different than the message level
20:42:17 <roasbeef> but even then, can't that be detected up front with featur ebits?
20:42:26 <roasbeef> (don't send if more than x MB)
20:43:13 <cdecker> Yeah, ok, but we're splitting things across multiple messages again
20:43:48 <cdecker> Anyway, seems we need to discuss this a bit further on the ML or the tracker
20:43:55 <bitconner> sgtm
20:44:37 <cdecker> #agreed The TPV types need to be message-specific, not global like in the current proposal, to maintain flexibility
20:45:08 <cdecker> #agreed Discussion of whether the even/odd rule should apply to TLV types as well is deferred to the ML/issue tracker
20:45:26 <cdecker> Does that sounds correct?
20:45:33 <bitconner> yep
20:45:38 <niftynei> thumbs-up
20:46:15 <cdecker> Shall we discuss #571? I think Rusty was going to split the PR in two anyway
20:48:38 <roasbeef> still not clear to me waht the goal is or what it achieves
20:49:07 <roasbeef> action item last time was concrete examples and motivating use cases
20:49:13 <cdecker> Just checked, Rusty will split the PR in two, hopefully making its goal a bit clearer (http://www.erisian.com.au/meetbot/lightning-dev/2019/lightning-dev.2019-04-15-20.06.html)
20:49:57 <cdecker> So I'd propose we jump straight to #593 instead
20:50:30 <cdecker> #topic bolt04: Multi-frame sphinx onion routing proposal #593
20:50:59 <cdecker> We had a concept ACK from last time, and t-bast mentioned he had a question about the proposal
20:51:13 <t-bast> yes I have one very small question
20:51:47 <t-bast> it's slightly unclear if we want to support the fact that the last payload can be multi-frame (because we mention that it needs to contain amt_to_forward and cltv)
20:51:56 <t-bast> I think we should support multi-frame last payload
20:52:00 <t-bast> just want to be sure
20:52:55 <cdecker> Personally my hope is that we can TLVify all onion payloads. That would mean we just assign TLV types to those 2 fields and wrap them in the TLV field
20:53:21 <t-bast> cool, in that case last payload can be multi-frame - I think it's more flexible to say that it could be multi-frame if we need it to be
20:53:23 <cdecker> In that case we'd treat the last payload just like any other
20:53:48 <cdecker> And it'd also be pretty much required for things like MPP and spontaneous payments, where we actually need the extra space
20:54:30 <t-bast> agreed then, I haven't seen anything strange implementing it in eclair, it's quite easy to integrate and should be flexible enough for routing updates
20:54:51 <cdecker> Yes, absolutely, we keep the old `hop_data` format for the case in which nodes are in the route (or indeed the final recipient) that don't know anything about TLV or multi-frame payloads
20:55:01 <cdecker> Awesome
20:55:36 <cdecker> @everybody: shall we wait with merging #593 until we also have an independent implementation in scala?
20:55:52 <t-bast> the implementation is ready :)
20:56:04 <t-bast> I sent the PR today and reference the #593
20:56:10 <cdecker> (because the C and Golang implementation were created by me, and I might have just repeated the same error) :-)
20:56:20 <cdecker> Oh, great
20:56:21 <t-bast> I think we need to add the test vectors though
20:56:38 <t-bast> how do you usually create them?
20:56:41 <cdecker> That PR is tiny :-)
20:56:49 <t-bast> oh yeah, scala is concise ;)
20:57:11 <cdecker> I wrote a cli tool for Go and C that just reads the JSON file and checks the output
20:57:13 <bitconner> reminder that we should have a separate change to commit the version number before actually adding a new onion version
20:57:35 <cdecker> Isn't that a breaking change?
20:57:40 <t-bast> that onion version is backwards-compatible with existing onions iiuc
20:57:47 <t-bast> so we don't need to bump to v1
20:58:04 <cdecker> Yep, #593 is perfectly backward compatible, no need for a version bump
20:58:18 <roasbeef> in any case the intermediate nodes need to know about it
20:58:29 <bitconner> oh okay, was just looking thru the pr and it sets a baseVersion so it looked like it could be something other than 0
20:59:13 <cdecker> roasbeef: what do you mean? We are telling the hops the payload type and the number of frames to read, what more do we need to tell them?
20:59:37 <t-bast> if we hit a node that doesn't understand multi-frame, we're kinda screwed
20:59:43 <t-bast> is that what you meant roasbeef?
20:59:58 <bitconner> isn't that signalled via a global feature bit?
21:00:00 <t-bast> but the route should fail immediately (ideally)
21:00:09 <t-bast> ah ok, if that's the case it would be nice
21:00:14 <bitconner> the i-support-multi-frame bit
21:00:28 <cdecker> Oh, yeah, that's needed of course
21:00:32 <cdecker> You're right
21:00:36 <bitconner> looks like it might be missing from the pr?
21:00:55 <cdecker> That'd be a global feature bit in the node_announcement
21:00:59 <t-bast> it is definitely missing, good catch
21:01:06 <cdecker> Yep, it's missing
21:01:08 <cdecker> Will amend
21:01:40 <cdecker> #action cdecker to assign a node_announcement feature bit to signal multi-frame support in the gossip
21:01:43 <roasbeef> yeah so if feature bit, we can commit to the version
21:02:04 <t-bast> sounds good
21:02:26 <bitconner> also noting that we call the payload format version 1, when it's actually version 0
21:02:29 <cdecker> No, there is no way for an onion to switch version along a path
21:02:45 <cdecker> And nodes will reject a version that is not 0
21:02:47 <bitconner> cdecker, why not?
21:02:57 <bitconner> isn't that only specific to the node that does the unwrapping?
21:03:35 <cdecker> Well, we need to tell the node that the version is committed in the HMAC, and we can't use a version bump to tell them because we can't switch version along a path
21:03:50 <bitconner> ah of course
21:05:11 <roasbeef> cdecker: won't them signalling the feature bit handle that?
21:05:37 <bitconner> we'd still have to bump the version to signal tho
21:05:45 <cdecker> Nope, if I signal I expect the version to be committed, and the sender doesn't understand then what?
21:05:57 <bitconner> there's no way to know the node constructing the onion ever got your node announcement anyway
21:05:57 <roasbeef> ofc it can switch, but ppl will just reject it today, but if we're using it to gate oteh things then someone can switch it out and cause a specific type of failure
21:06:01 <cdecker> (signal using a feature bit that is)
21:06:26 <roasbeef> cdecker: you can tell based on the version if you should check the mac or not
21:06:38 <roasbeef> yes so bump+signal+mac
21:07:13 <cdecker> roasbeef: go and check how you can receive a 0-version onion, and make sure the next hop gets 1-version onion despite you not understanding v1
21:07:58 <cdecker> We can do a 2-step deployment: TLV first, assign a TLV type "next-onion-version" and then use that to specify what version the next onion should have
21:08:19 <bitconner> we can't see what the next hops version is tho?
21:08:56 <roasbeef> confused as to how that 2 step addresses anything
21:09:10 <cdecker> Well that's what we're telling the processing node in the payload: "instead of the default version 0, you should serialize it with version 1"
21:09:11 <roasbeef> or that example
21:09:26 <roasbeef> why wouldn't they just serailize w/ the version they get in?
21:09:40 <bitconner> the bigger issue it seems is that the onion has a single version for the entire path, and not a per-hop version
21:10:34 <cdecker> roasbeef: that's exactly the issue, by default they will always serialize with the same version. And since already deployed nodes will reject any version that they don't know, we can't do a partial upgrade on a route
21:10:44 <cdecker> bitconner: exactly
21:11:24 <t-bast> you mean the version that's in the header (outside of the hops_data) right?
21:11:35 <cdecker> That and the fact that nodes reject versions they don't know means we always have to chose the lowest version supported by hops in the route
21:11:50 <cdecker> t-bast: yep, the first byte of the serialized onion packet
21:11:52 <t-bast> that seems to indicate that the multi-frame should stay in version 0 because in that case it works fine
21:11:54 <bitconner> could move the version into the hops data :)
21:12:26 <t-bast> if we keep version 0 to add the multi-frame support, everything should work fine
21:12:26 <cdecker> payload, yes, but that means we can't change the HMAC processing since we interpret the payload after verifyin the HMAC
21:12:41 <bitconner> o/w we will forever be stuck with the lowest common denominator
21:12:48 <t-bast> indeed
21:13:07 <cdecker> Yep, that's a bit of an oversight we had
21:13:24 <cdecker> But with the version switching in the TLV it could work
21:14:12 <cdecker> Anyway, let's not mix the two things, #593 is independent from any version switching (I purposefully made it that way)
21:14:17 <roasbeef> ah yeah i'm not talking about a partial upgrade, a full one, so no mixed route
21:14:27 <t-bast> agreed with cdecker
21:14:42 <cdecker> But you are right that we need to find a way to unlock the version numbering, otherwise we're stuck forever at v0
21:14:53 <cdecker> And can't change the processing of onions at all
21:15:03 <roasbeef> we already are changing the processing of them
21:15:16 <t-bast> yes but the current changes work as long as we stay on version 0
21:15:18 <cdecker> We are, but in a backwards compatible way
21:15:29 <t-bast> and that the sender knows who supports multi-frame in the path and who doesn't
21:15:30 <bitconner> cdecker, yes and it seems it will be rather large change and necessitate an overhaul of the onion packet anyway. best to do that in one fell swoop indepdently of these changs
21:16:05 <cdecker> Something that would require a version bump for example would be a change in the number of hops, HMAC computation, random bytes generation, that sort of thing
21:16:15 <cdecker> bitconner: agreed
21:16:16 <t-bast> yes, version 1 of the onion can fix that, and multi-frame can stay in version 0 :)
21:17:05 <bitconner> t-bast, well in theory we'd want multi-frame in version 1 as well
21:17:22 <t-bast> yes, what I meant is that we can do the multi-frame change while staying in version 0
21:17:30 <cdecker> Hehe
21:17:39 <t-bast> and when we move to version 1, we can fix versioning and keep multi-frame :)
21:17:44 <bitconner> i don't think there's any dispute there :)
21:18:08 <cdecker> Oh, we can do so much more trickery if we ever decide to revamp the entire onion construction xD
21:18:15 <t-bast> haha
21:18:19 <bitconner> okay, circling back. seems we just need a global feature bit?
21:18:25 <cdecker> Anyhow, seems we have my action item
21:18:37 <t-bast> bitconner: I think that's the only missing part in the spec indeed
21:18:42 <cdecker> Yep, will assign one asap and then we can merge next time.
21:18:50 <bitconner> sounds good
21:18:53 <t-bast> just last small point before we merge the PR
21:18:58 <t-bast> concerning the test vectors, I only saw the previous test vectors (single-frame) in cdecker's PR. I think we should add test vectors for multi-frame, do you want me to send a proposal?
21:19:02 <cdecker> Since the TLV PR is also stuck in limbo there is no real downside in deferring this one
21:19:12 <bitconner> sorry i didn't get a chance to look over the changes in depth since last meeting, but will do so today
21:19:18 <cdecker> Didn't I add multiframes as well?
21:19:31 <t-bast> I didn't see them, no, let me double-check
21:19:38 <cdecker> Apparently not, need to add them as well
21:19:47 <cdecker> #action cdecker to add multi-frame test-vector
21:19:54 <cdecker> Excellent, let's proceed
21:20:09 <cdecker> #topic BOLT2: rephrase cltv_expiry_delta text about accepting HTLCs #595
21:20:22 <cdecker> I just saw that I was supposed to merge this
21:20:24 <cdecker> Mea culpa
21:20:31 <cdecker> I'll just do so now
21:20:31 <bitconner> lgtm
21:20:56 <bitconner> we haz progress!
21:20:57 <cdecker> #action cdecker merged #595, based on decision from last meeting
21:21:05 <cdecker> OMG we better stop now
21:21:12 <cdecker> Otherwise what will we discuss next time?
21:21:23 <cdecker> #topic BOLT 5: Homogenize Local/Remote Commitment #598
21:22:06 <cdecker> Looks like it could fall under the spelling rule
21:22:52 <bitconner> looks straightforward
21:23:06 <cdecker> Ok, lgtm
21:23:19 <cdecker> #agreed cdecker to merge #598
21:23:26 <cdecker> Any objections?
21:23:55 <t-bast> lgtm
21:24:02 <bitconner> approved
21:24:17 <cdecker> Onwards, the last one for today
21:24:20 <cdecker> #topic BOLT 3: Explicit description of implicitly enforced timelocks on HTLC outputs #601
21:24:44 <cdecker> Adds more prose around the time-lock for HTLC outputs
21:25:24 <cdecker> Feels a bit hand-wavy to me
21:25:48 <cdecker> It mentions implicitly enforcing the timeout, but not how the timeout is specified
21:26:51 <bitconner> agree could probably use a little massaging
21:27:00 <cdecker> Should likely mention the difference in locktime between the two HTLC-spending transactions
21:27:06 <cdecker> Ok, will make a note of it
21:27:46 <cdecker> #action cdecker to add a bit more detail about how the timelock is committed to in the signature
21:27:59 <cdecker> Excellent, that concludes the official part
21:28:12 <cdecker> #topic Chit chat and gossip :-)
21:28:33 <cdecker> Anything we should discuss that was not on the agenda?
21:28:36 <bitconner> woo! thanks for chairing cdecker
21:28:51 <niftynei> :D
21:29:05 <cdecker> Thanks everybody for being excellent chairees :-)
21:29:10 <t-bast> thanks cdecker!
21:30:02 <m-schmoock> :D
21:30:44 <m-schmoock> i probably did understand like 30% of what was talked about, but im not that deep in RFC spec yet
21:30:45 <cdecker> If there's nothing else I think we can finish up
21:31:10 <cdecker> That's quite normal m-schmoock, I often have to go and look things up during these myself :-)
21:32:03 <cdecker> #endmeeting