20:06:16 <cdecker> #startmeeting
20:06:16 <lightningbot> Meeting started Mon Apr 15 20:06:16 2019 UTC.  The chair is cdecker. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:06:16 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
20:06:50 <cdecker> Shall we jump right into reviewing the pending things or are there any important announcements to get in first?
20:07:02 <bitconner> present, your honor
20:07:11 <cdecker> Congrats to the LL team for the launch of Loop first of all:-)
20:07:45 <lndbot> <alexbosworth> moli just did the first clightning loop
20:07:55 <cdecker> Awesome :-)
20:08:19 <cdecker> #topic BOLT7: extend channel range queries with optional fields #557
20:09:01 <cdecker> As far as I can see the last few changes were mostly typo fixes
20:09:14 <rusty> OK, I have been working on converting this to TLV.  I have a commit, but I'm not happy with it.  See my TLV extension q. from before the meeting:
20:09:14 <cdecker> Did someone have an objection last time?
20:09:39 <rusty> "should we just assert that any extra bytes in msgs is the TLV, or have an explicit length then TLVs field in each msg..."
20:09:53 <bitconner> i think the primary objection from the last meeting was the to requiredness of the checksums
20:09:54 <cdecker> Ok, shall we tackle TLVs first? Seems they block a couple of things (including extensions on the new onion format)
20:10:30 <rusty> bitconner: well, also TLV, which makes it more optional.
20:10:53 <cdecker> Right, so the idea is to go fully flexible with TLV right away
20:10:56 <rusty> My plan was to implement and test against ACINQ then get back.
20:11:18 <sstone> with what we have or with changes to TLV format ?
20:11:50 <bitconner> was thinking it could also be done by defining option_extended_query_flag as a bitfield, tlv would also make that easier
20:12:04 <sstone> I'm not completely sure about TVL for this specific use-case
20:12:07 <sstone> TLV
20:13:03 <rusty> sstone: it's hitting the problem that it's the first one, so if we're going TLV we should do it for this.  Sorry...
20:13:11 <cdecker> I see your point, assigning types for query_short_channel_ids(chain_hash) seems weird
20:13:53 <sstone> rusty: I understand, it just looks a bit weird.
20:13:54 <rusty> I *believe* we had consensus on TLV being the way forward, and so it seems simpler to make everything not already defined in messages to be a TLV; i.e. any leftover bytes.
20:14:10 <cdecker> A message (chain_hash|encoded_channel_ids|TLV_with_optional_parts) does seems sensible though
20:14:47 <sstone> ok. does everyone agree that we need the extra timestamps ?
20:14:54 <bitconner> yes
20:14:59 <cdecker> :+1:
20:15:20 <rusty> (niftynei has a PR which actually defines and parses the TLV format, FWIW, so I'll be making sure that's in sync)
20:15:21 <roasbeef> cdecker: thanks!
20:15:30 <roasbeef> my q in #557 is how the checksum willbe modified in the future
20:15:32 <bitconner> sstone, correct me if i'm wrong, but does the current proposal specify any sort of negotation?
20:15:49 <roasbeef> like if we get more fields, or is that what we're saying above to just checksum all the optional fields as well?
20:16:36 <cdecker> IIRC we must checksum the entire message (sans timestamp and signatures), otherwise we might miss updates that are important for our peers
20:16:58 <rusty> roasbeef: yeah, it's defined to skip TS and signature only...
20:17:07 <roasbeef> ok, but if you're using a new tlv field, I don't know of, then how can we arrive at the same checksum? unless there's some upfront signalling
20:17:12 <sstone> well sending optional fields is a very basic form of neg ?
20:17:13 <cdecker> (partial broadcasts that just end whenever they encounter a node that doesn't know how to parse has weird semantics)
20:17:52 <bitconner> sstone, no we'd making the same mistake we made w/ the encoding options, which forces everyone to understand it
20:17:57 <cdecker> roasbeef: the length gives you how many bytes to skip for that field, so just skip them when parsing and you can just resume parsing after that with the next field
20:18:02 <rusty> roasbeef: if channel_update contains new appended fields (TLV, presumably), they'll get included in checksum, just like they get incldued in signature?
20:18:37 <roasbeef> is there an assumed upon ordering for the types in the TLV?
20:18:45 <sstone> bitconner: sorry I don't understand your point
20:18:48 <rusty> roasbeef: increasing, yes.
20:19:00 <cdecker> increasing by type
20:19:29 <cdecker> With length and lexicographic content as tiebreaker for duplicate fields
20:20:05 <roasbeef> gotcha, but even with that, lets say I have the old channel update on disk w/o the new field how am I able to compute your new check sum?
20:21:10 <cdecker> Well, your checksum just includes the message you have on disk, and since your peer's will not match you will download and see the new fields
20:21:18 <bitconner> sstone, let's send query_flag=1 in a query_channel_range, if the remote peer doesn't support it are they allowed to send back reply_channel_range with query_flag=0?
20:21:44 <bitconner> and not include the extra data
20:22:07 <cdecker> Sure, why not?
20:22:32 <cdecker> You can always send back the old format, it's just an optimization for peers that know how to speak the new format
20:22:55 <sstone> you would just not include the optional flag
20:23:06 <bitconner> so then my proposal is to make option_extended_query_flag a bit field, allocating a bit for timestamps and another for checksums
20:23:24 <bitconner> it doesn't make sense to have checksums w/o timestamps, but vice versa is fine
20:23:49 <bitconner> then B can respond to A by masking it's supported feature w/ what is requrest
20:23:56 <bitconner> s/requrest/requested
20:24:13 <cdecker> Why not go TLV then? It's 2 additional bytes and allows us to swap out any part of the extra info in future without assigning a feature bit
20:24:15 <sstone> there is no specific feature bit in the current PR
20:25:11 <roasbeef> cdecker: gotcha that makes sense
20:25:52 <sstone> cdecker: ok one more argument for TLV
20:26:02 <cdecker> Yay :-)
20:26:27 <bitconner> so then what is the purpose of returning query_flag in the reponse, if not to signal what is included?
20:26:41 <cdecker> Ok, seems we sort of are agreeing that we want the timestamp and checksum fields to be split and made into a TLV field, am I interpreting this correctly?
20:26:54 <rusty> BTW, the TLV-ized version is here: https://github.com/rustyrussell/lightning-rfc/tree/fun-with-tlvs
20:27:24 <rusty> It's currently a pile of commits on top of the original proposal, sstone and I should go through and clean it up.
20:27:49 <bitconner> i think it makes sense to split them because adding checksums is a much bigger change from timestamps
20:28:03 <bitconner> timestamps alone solves the initial problem, while checksums are an additional optimization
20:28:12 <cdecker> Ok, I'll need to review rusty's proposal first :-)
20:28:13 <sstone> rusty: having fun with TLV sounds ...fun :)
20:28:20 <rusty> bitconner: easily done, just another TLV field.  sstone, do you agree?
20:28:42 <bitconner> rusty, a TLV on channel updates themselves? or the reply_channel_range?
20:29:12 <rusty> bitconner: the query and reply... the query uses it to flag what it wants,the reply uses it for the actual ts & csums.
20:29:15 <cdecker> Uhm, is that two TLV fields or a TLV with two fields? (for reference I always think of TLVs as a dict in python, or a map in go)
20:30:14 <bitconner> cdecker, i think it would be two fields so that each can be parsed independently?
20:30:30 <rusty> cdecker: bikeshed... we can have a TLV field saying "here's a flags word representing what I want" or one for "gimme timestamps" and anotehr for "gimme checksums"
20:30:57 <rusty> ... latter is probably cleaner, if we treat them independent.
20:31:21 <cdecker> Well, let me rephrase: the former is {"timestamps": []} {"checksums": []} whereas the latter which I prefer, is {"timestamps": [], "checksums": []} in JSON notation
20:31:52 <cdecker> And given that these are type-length-value encoded you can easily skip fields that you don't know how to interpret
20:32:02 <cdecker> Anyway, sorry for getting sidetracked
20:32:17 <cdecker> Do we agree that we want timestamps and optionally checksums?
20:32:30 <sstone> I prefer one TLV with two fields too
20:33:13 <cdecker> #agreed PR #557 is to be amended to track timestamps and checksums separately, so one or the other can be omitted.
20:33:23 <rusty> cdecker: the latter is more {"timestamps_and_checksums": []}?  That's the current proposal, but if we split it becomes {"timestamps": []} {"checksums": []} ?
20:34:04 <rusty> But yes, let's move on...
20:34:20 <cdecker> No the current proposal would be {"timestamps_and_checksums": []} which is inseparable
20:34:42 <bitconner> sgtm, long as there is some method of doing timestamps-only, i think it will reduce time to initial deployment :)
20:35:02 <cdecker> The distinction between one TLV with multiple fields or multiple TLVs become important once we talk about serialization of TLVs
20:35:12 <cdecker> It's not important here
20:35:35 <cdecker> Do we also agree that we want to investigate the TLV encoding for the fields??
20:35:39 <roasbeef> cdecker: what's 1 with multiple fields entail?
20:35:51 <roasbeef> so the value is an array?
20:36:29 <rusty> I'm still confused.  There's one "tlvs" field in the message.  It  can contain multuple T's.  In this case 1 == timestamps, 2 == checksums, say.
20:36:31 <cdecker> Possibly, or just back to back serialized timestamps or checksums
20:37:04 <cdecker> Oh yeah, that's exactly what I was looking for, thanks rusty
20:37:13 <rusty> OK, good.
20:37:39 <cdecker> So I count sstone and rusty in favor of TLV encoding the optional fields, do I get an ack from LL?
20:37:40 <roasbeef> ok I think that clarifies it, ihave some other q's w.r.t why this particualr checksum algo was selected and the impact of it, but can push that to the PR itself
20:38:31 <cdecker> #agreed PR #557 should be amended to use the TLV encoding for optional fields
20:38:44 <bitconner> sgtm, thanks for clarification rusty
20:39:01 <cdecker> Ok, I think that covers #557, right?
20:39:06 <roasbeef> yep
20:39:20 <cdecker> Great, moving on
20:39:41 <cdecker> #topic Feature bit unification and assignment  #571
20:40:04 <cdecker> This one was skipped last time since it wasn't tagged, but it got dragged in through a later PR
20:41:46 <cdecker> I think roasbeef had an issue with it last time
20:42:11 <roasbeef> i was just never clear on what it was solving or why we wanted to do it, seems it just comes down to a rename?
20:42:32 <roasbeef> and iirc at the tail end last time, it was realzied that it could be done slighlty diff based on the discussion about the wumbo bit?
20:42:46 <cdecker> Yep, pretty much. It just means that talking about feature bit 8 is not context dependent
20:43:20 <rusty> roasbeef: yes, but also we don't overlap so we can spray them both over the node_announcement, so you can tell I support foobar-channels.
20:43:22 <roasbeef> maybe soem motivating uses cases or like user/nodes stories would make it more clear?
20:43:37 <roasbeef> like "bob wants to know if alice supports the new dlog htlcs, he looks at __ and __ to determine"
20:43:56 <rusty> roasbeef: agreed.  And I'll leave the renaming to a separate PR, since it just muddies the issue?
20:44:23 <rusty> (I'm coming around to "channel" features vs "node" features, TBH, but that's a separate discussion).
20:44:53 <rusty> I think action is for me to clean that up to just state the overlap, give a good usage description, drop the renaming (for now), and have the feature assignments in a separate PR?
20:45:34 <cdecker> #action rusty to separate the PR into two PRs, one for rename and one for the overlap avoidance
20:45:36 <cdecker> :-)
20:45:52 <rusty> Great, let's move on...
20:46:18 <cdecker> #topic Specify tlv format, initial MPP #572
20:48:02 <rusty> Let's defer this... the rules for TLV need to be updated to specify varint, and that we're simply appending them to msgs.  The others, again, need to be split.
20:48:28 <rusty> (varint for type, that is: len is already varint)
20:48:32 <cdecker> Hm, too bad I really like the proposall
20:48:51 <cdecker> #action everybody to review the PR on Github
20:48:57 <rusty> Oh, I like it too, but it needs work on the PR.
20:49:17 <cdecker> #topic bolt04: Multi-frame sphinx onion routing proposal #593
20:50:02 <cdecker> I hope people had some time to look into the proposal since the last time
20:50:26 <cdecker> Sorry for posting it so late the last time, but I procrastinated on the writeup a bit :-)
20:50:30 <bitconner> had a little bit of time, i do like it reduces the number of crypto operations :)
20:50:40 <roasbeef> have some unpublished comments, but I've come around to the unification this achives between the two approaches
20:51:00 <cdecker> Awesome, thanks :-)
20:51:17 <rusty> :hooray:
20:51:18 <cdecker> roasbeef: can you still publish those comments? I'm curious ^^
20:51:32 <cdecker> sstone: did you have time to take a look too?
20:51:47 <bitconner> iirc, one thing that was unresolved from last time was how to signal a partially complete payload?
20:52:24 <sstone> yes but I did not have time to write a prototype :(
20:52:24 <cdecker> You mean how to use only part of a frame if the payload does not fit perfectly?
20:52:49 <bitconner> cdecker, yes, tho maybe that is done implicitly when using a TLV payload?
20:53:09 <cdecker> Yep, the TLV proposal defines type 0 as padding basically :-)
20:53:33 <cdecker> And for the old style payload we have the same convention anyway
20:53:44 <cdecker> (must be 0-padded)
20:54:19 <bitconner> sgtm
20:54:37 <cdecker> Ok, everybody happy with the proposal then?
20:56:18 <cdecker> Letting people finish writing their objection :-)
20:56:51 <bitconner> yep i'm happy. do we also agree that committing to the version number is a prerequesite, now that we'll have something other than 0? :)
20:57:19 <bitconner> will leave other notes on the pr, but approach is solid imo
20:57:32 <cdecker> You mean add the version number to the associated data?
20:57:35 <cdecker> Sure
20:58:12 <cdecker> Though that might be backwards incompatible wouldn't it?
20:58:13 <bitconner> yes, it isn't necessary atm since everyone only knows 0
20:58:27 <rusty> Dumb q: is there a decryption oracle problem now we can flip "realm byte" bits and measure how long it take to check hmac?
20:58:48 <cdecker> Wait, #593 doesn't change the onion version at all
20:59:26 <cdecker> rusty: hmac is still computed only once before we decrypt, and before we read the realm bits, so no
20:59:42 <bitconner> rusty, as in a timing orcale?
20:59:49 <rusty> cdecker: we could do some hack where we add realm byte to AD IFF it's != 0, but that seems icky...
21:00:23 <m-schmoock> good evening :D
21:00:24 <rusty> bitconner: yeah, but it was a half-baked thought.
21:00:27 <cdecker> rusty: yes, but that's not the realm byte its the onion serialization version which is currently not covered by hmac
21:00:46 <cdecker> Anyway, seems only minor things to hammer out
21:01:04 <rusty> cdecker: ah!  RIght.... hmm, bumping onion version is a diff change, needs signalling across path.
21:01:06 <cdecker> #agreed Conceptually accepted PR #593
21:01:29 <bitconner> yay!!
21:01:30 <cdecker> rusty: exactly, hence we only touch the realm byte which is covered by the hmac :-)
21:01:34 <cdecker> Awesome
21:01:46 <cdecker> #topic BOLT2: rephrase cltv_expiry_delta text about accepting HTLCs #595
21:02:13 <cdecker> A small rephrase from harding, which I thought makes a lot of sense
21:02:32 <rusty> Yep, Ack.
21:02:34 * harding waves
21:02:37 <bitconner> ack
21:02:43 <sstone> ack
21:02:46 <cdecker> It's unwinding a few contortions from the original format
21:02:48 <bitconner> thanks harding!
21:02:57 <cdecker> #action cdecker to merge asap!
21:03:05 <rusty> Yeah, who wrote that original crap!  Oh...
21:03:17 <cdecker> #topic Miscelaneous and chit chat :-)
21:03:39 <roasbeef> our 0.6 should be dropping this week, should reduce some of the enable/disable spam we see on the network
21:03:48 <rusty> roasbeef: w00t!
21:03:49 <roasbeef> have seen it go down a bit with nodes upgrading to the latest release candidate
21:03:55 <cdecker> Great ^^
21:04:01 <rusty> I'll be missing next meeting: apparently there are these things called vacations?
21:04:17 <roasbeef> one thing we've started to do is just not sync everything from all peers, so if you have 1k peers, we'll only actively get updates for 3 peers or so whcih makes a big diff w.r.t bandwidth usage
21:04:18 <cdecker> I know, strange right?
21:04:23 <roasbeef> inv stuff should clear up the remaining redundnacy
21:04:50 <cdecker> Yeah, we might implement that soon too (2 weeks...)
21:05:08 <rusty> roasbeef: nice!  My own efforts to reduce gossip have stalled due to other random crap, but I've got a plan in a GH issue somewhere, so that's a start!
21:05:35 <cdecker> Have you guys seen the Turbo channels from bitrefill? They're 0-conf channels that are usable in one direction until they confirm
21:05:54 <m-schmoock> yep, didnt try thou
21:06:26 <cdecker> I was shocked initially, but after thinking about them a bit more it makes sense since they have all the capacity on their side
21:06:37 <cdecker> Anyway, we should probably end the meeting here.
21:06:55 <cdecker> Hoping we have some more discussion on trampolines for next time :-)
21:06:58 <rusty> cdecker: are they only usable in 1 dir?
21:07:21 <cdecker> Yep, they seem to refuse HTLCs in the other direction
21:07:26 <rusty> I bought a channel off bitrefill, but didn't try a turbo one... interesting...
21:07:50 <cdecker> Any famous last words?
21:08:42 <cdecker> #endmeeting