20:03:03 <roasbeef> #startmeeting
20:03:03 <lightningbot> Meeting started Mon May 13 20:03:03 2019 UTC.  The chair is roasbeef. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:03:03 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
20:03:28 <cdecker> Yay :-)
20:04:06 <cdecker> List of topics scheduled for today: https://github.com/lightningnetwork/lightning-rfc/labels/Meeting%20Discussion
20:05:08 <roasbeef> just added two things sec
20:05:09 <t-bast> the first two should be fairly simple :)
20:06:34 <cdecker> You mean the first two from the top?
20:06:34 <roasbeef> #topic #609: unify redundant paragraphs
20:06:44 <roasbeef> order is a bit diff now since aded soem
20:06:59 <roasbeef> #609 prob falls under the typo rule
20:07:08 <bitconner> yeh
20:07:11 <t-bast> cdecker: yes exactly
20:07:12 <cdecker> Yep, added the spelling tag
20:07:16 <bitconner> would also say that we probably don't even need the first sentence at all
20:07:21 <bitconner> the second is more specific
20:07:34 <roasbeef> ok, so contest on merge? already has an ack from cdecker
20:07:46 <cdecker> lgtm
20:07:53 <roasbeef> #action merge 609
20:07:54 <bitconner> lgtm, still a good clean up
20:08:03 <t-bast> lgtm
20:08:15 <sstone> ack
20:08:25 <roasbeef> #topic 608 merge final expiry and unknown payment hash
20:08:40 <roasbeef> so this is similar to what happend with unknown payhash and incorrect amount
20:08:59 <roasbeef> rationale being that this should also be merged in order to not give back any identifiable information to nodes attempting to probe routes
20:09:02 <cdecker> Pretty close to a typo fix, but since it actually changes subject...
20:09:25 <roasbeef> cdecker: #608?
20:09:26 <cdecker> Oh, sorry got confused about the order
20:09:41 <roasbeef> ah np, this is 8 hrs old, so wouldn't expect ppl to have looked into it deeply yet
20:10:03 <bitconner> makes sense to me
20:11:12 <t-bast> sounds reasonable, it could leak some info with the current behavior
20:11:25 <t-bast> so lgtm to update
20:11:53 <sstone> what does "trying payments with the correct amount" mean in this context ?
20:11:53 <roasbeef> yeah that's the rationale behind the change, moving more towards a unified "that didn't work for me" error w/o giving intermediate nodes more info than they actually need
20:12:21 <roasbeef> sstone: is that in the PR body? or ref to something above?
20:12:30 <sstone> in the PR description
20:13:15 <roasbeef> I think he means payments with the proper amount, but the others modified in order to probe to see if a node will reject based on implementation specific params around grace periods
20:13:30 <roasbeef> but proper amount would be guessed in many cases if the next link isn't the actual destination
20:14:16 <roasbeef> i don't joost is on IRC rn, but I can forward some of these q's to him
20:14:36 <sstone> you mean by making up guesses as to the last hop and applying corresponding fees ?
20:14:41 <sstone> hops
20:14:42 <lndbot> <joost.jager> hi all
20:14:57 <roasbeef> a wild joost appears!
20:15:02 <roasbeef> but he's on slack, so that's cheating ;)
20:15:15 <lndbot> <joost.jager> sorry, other client didn't work. you apparently can't read my msgs
20:15:18 <lndbot> <joost.jager> correct amount as in an amount that exceeds at least the invoice amt
20:15:23 <lndbot> <joost.jager> it needs the proper amount, otherwise it would trigger the incorrect_or_unknown_payment error. the idea is to force the final_expiry_too_soon error to reveal that that node is the destination
20:16:56 <sstone> if proper means exactly what the destination is expecting Id' say guessing would not be that easy ?
20:17:19 <lndbot> <joost.jager> amount doesn't need to be exact, but enough. at least as long as overpayments are accepted
20:17:20 <bitconner> sstone: it only needs to be within 1-2x the invoice amount, iiuc
20:17:50 <bitconner> or any positive value if the invoice is zero-valued
20:18:19 <lndbot> <joost.jager> then the expiry needs to be lower than the final invoice cltv, that is more guesswork, but still doable
20:18:51 <sstone> ok
20:19:54 <lndbot> <joost.jager> the success of the attack also depends on implementation specifics, but the way it currently is, some info is leaking
20:20:45 <roasbeef> don't think we need to come to a general agreement rn, just wanted to intro the change, we can circle back on it next meeting to give ppl more time to digest perhaps (less than a day old), but similar to a change we've made in the past so i'm on board with the general idea
20:21:38 <bitconner> sgtm, not a very contentious change (i think) and we have a lot of topics today :)
20:21:53 <t-bast> it's going to be a trade-off between ease of debugging and security - early on in the network's life it's useful to be a bit specific in the errors, once everything works well errors should be as generic as possible to avoid leaking anything (think padding oracle attacks in crypto)
20:22:03 <roasbeef> t-bast: so the sender still has all the info
20:22:09 <roasbeef> but less info for the intermediate nodes
20:22:18 <t-bast> yeah in that case I don't think it even hurts debugability
20:22:20 <t-bast> so lgtm
20:22:56 <roasbeef> ok, sounds like it isn't too controversial, but we're missing a few ppl in this meeting, so maybe let's keep it on the agenda for the next one to give others a chance to check it out?
20:23:12 <sstone> sgtm
20:23:25 <cdecker> Sounds good to me, would like to dig in a bit more, but overall sounds reasonable
20:23:30 <roasbeef> #agreed circle back on #608 next week
20:23:42 <roasbeef> #topic #607 and #603
20:23:49 <roasbeef> combining them since they're proposing similar things
20:24:06 <roasbeef> bitconner: wanna give us an overview on the diff tradeoffs this makes compared to rusty's original/
20:24:09 <roasbeef> ?
20:24:10 <roasbeef> both on the topic of a TLV format
20:24:12 <bitconner> sure
20:24:19 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/607
20:24:23 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/603
20:24:35 <roasbeef> (first one is the new one)
20:24:45 <bitconner> the big question that i think needs to be answered here is whether or not we are okay with having two different wire formats, wire vs onion
20:25:03 <bitconner> to me, the tradeoffs are sufficient to warrant this, though i'm not sure if others feel the same
20:25:38 <bitconner> that being said, i took a stab at writing up a spec for a spec tailored more for wire specificaly
20:26:16 <roasbeef> what makes this more wire specific? diff sizes for type + length?
20:26:34 <bitconner> to me the biggest differences is enforcing canonical ordering, as wire-TLV may be used to extend gossip messages that need signature verification to function properly
20:26:40 <roasbeef> the additional ordering requirement deseriable in both contexts
20:27:10 <t-bast> can you detail why you think 2 wire formats are needed in the first place? I think canonical ordering for example could be desirable for both, isn't it?
20:27:21 <bitconner> it also has a requirement for readers to cache unknown fields, so that they can be reserialized properly
20:27:23 <cdecker> I was just about to ask the same thing
20:27:35 <roasbeef> #603 lacks an odering requirement iirc
20:27:41 <roasbeef> or possibly just doesn't make it explicit
20:27:56 <bitconner> cdecker t-bast, i'm not certain. i would be in favor of enforcing it in either/both
20:28:21 <t-bast> but how do you sign without a canonical ordering?
20:28:33 <sstone> I think #603 also specifies ordering
20:28:52 <bitconner> t-bast: we need to be more space concious in the onion tlv, since space is much more limited. thus using a varint to capture the savings of encoding small values makes sense
20:29:07 <cdecker> No, I'm not sure I get what you refer to when saying "having two different wire formats, wire vs onion"
20:29:41 <bitconner> t-bast, you can't, which is why this one requires enforcing canonical ordering
20:29:58 <sstone> bitconner: then why not use a varint for length also, limited to 2 bytes ?
20:30:05 <niftynei> there is canonical ordering on #603 " MUST order `tlv` in ascending type, length and value."
20:30:09 <t-bast> bitconner: ok I get why you propose that then. But shouldn't we then use varint and enforce canonical ordering, and we have 1 format that works everywhere? :)
20:31:27 <bitconner> sstone, i'm not opposed to using a varint, though at most it will save 256/65Kb =- 0.39% of the available space and fixed-size is slightly faster
20:32:01 <bitconner> niftynei, the requirement is only on the writer though?
20:32:22 <bitconner> i'm suggesting that the reader must also assert this
20:32:44 <roasbeef> t-bast: main diff being the size constraints, 65kb just at most 1kb and some change maximally but most cases using tens of bytes
20:33:29 <niftynei> oh i see. my understanding was that the reader is free to enforce if they want to (it is indicated as MUST)
20:34:24 <sstone> also #603 seems to imply that there could be multiple occurrences of the same type
20:34:29 <bitconner> niftynei, in order for signature checks to pass idt we can make it optional for the reader iiuc
20:34:55 <roasbeef> #607 also doesn't have the even/odd distinction
20:35:22 <roasbeef> in the past when it came up, from my PoV it can be captured using only feature bits, but depending on if wire vs onion that may not always be the case at first glance
20:35:29 <bitconner> another difference between the two is the even/odd rule, which isn't needed for wire messages afaik since this can be gated via feature bits
20:36:45 <cdecker> But the same is true for onion TLV where we signal support via the node_announcement.features
20:36:48 <bitconner> imo, the even odd rule should be applied at a different layer than the parsing requirements
20:38:02 <roasbeef> cdecker: meaning we do or don't need the even/odd rule?
20:38:06 <bitconner> cdecker, though not all nodes have nodeannouncements
20:38:22 <roasbeef> hmm yeah I guess some would need to leak into the invoice as well?
20:38:37 <niftynei> i like how the even/odd rule signals the intent of the type author as to strength of the requirement
20:38:44 <cdecker> Oh I see that you wrote something about the need to use a different TLV format for wire.
20:39:12 <cdecker> I get the feeling I need to read more on it before making an informed decision (sorry for slowing this up)
20:39:29 <niftynei> it makes it easier to check on a spec/inclusion level intent wrt feature bit additions etc but that's more of a process check than code level check
20:39:37 <bitconner> niftynei, i think we can still use the even/odd rule in the onion-specific namespace
20:39:45 <roasbeef> niftynei: depends on the context, why would you put non essential data in the onion? for the wire may apply more
20:39:53 <bitconner> my point is that it doesn't need to be inherited by all namespaces
20:41:53 <cdecker> I'm just worried about my mental capacity of keeping the context rules separate when reading the spec :-)
20:42:16 <roasbeef> if distinct, ideally they'd be in distinct documents
20:42:29 <jtimon> re #609 Happy to remove the first sentence and the "In other words". Also, I guess "remote node's responsibility" was more specific than "the offerer's responsibility" too, so I guess I should keep that version instead.
20:42:36 <jtimon> sorry if it's not the right time and it's better to discuss it at the end since I'm talking about minor details
20:42:50 <roasbeef> ~20 mins left, few other topics still, perhaps we should comment directly on the TLV PRs? open questions seems to be: if distinct wire and onion, and if even/odd should apply to all (or just one if unified), requirements for decoder
20:42:59 <cdecker> sgtm
20:43:01 <niftynei> cool.
20:43:03 <t-bast> sgtm
20:43:05 <sstone> sgtm
20:43:08 <roasbeef> jtimon: so there's rough agreement for acceptance, have moved onto other topics, but you can change up your PR if you'd like
20:43:22 <bitconner> sgtm
20:43:28 <niftynei> thanks for the second write up bitconner, it's good to see the tradeoffs laid out
20:43:41 <roasbeef> #agreed discuss #603 and #607 on PR, open q's to resolve: if distinct wire and onion, and if even/odd should apply to all (or just one if unified), requirements for decoder, <insert other tradeoffs here>
20:43:46 <roasbeef> #topic 606
20:43:57 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/606
20:44:01 <jtimon> yeah, no hurry, as said it's minor details, just trying to be coherent with the "more specific is beetter" comment
20:44:08 <roasbeef> has two approvals rn
20:44:19 <roasbeef> changes connection to channel
20:44:29 <roasbeef> I think for soem implementations a connection and channel are 1:1
20:44:34 <roasbeef> while others allow more than one channel over a connection
20:44:39 <cdecker> Was really close to calling this a typo fix, but subject changes :-)
20:44:48 <bitconner> niftynei, thanks! was tyring to get a feel for how much they did/didn't intersect :)
20:45:20 <cdecker> This is HTLCs that are specific to a channel so I think channel is the correct scope to fail here
20:45:41 <roasbeef> ah ok, expanded the diff, that's clear now
20:45:42 <t-bast> agreed
20:45:44 <bitconner> iirc, there are probably more cases where connection is used synonmously with channel, but perhaps my memory evades me
20:45:54 <roasbeef> so we're good with this?
20:45:57 <bitconner> yep
20:46:02 <roasbeef> #action merge #606
20:46:02 <t-bast> lgtm
20:46:14 <roasbeef> #topic #601
20:46:20 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/601
20:46:56 <roasbeef> > BOLT 3: Explicit description of implicitly enforced timelocks on HTLC outputs
20:46:59 <cdecker> This is a followup on a clarification we discussed last time
20:47:18 <roasbeef> ah since it's a locktime rather than cltv for second levels?
20:47:31 <cdecker> Our comment last time was that he talks about timeout enforcement but never mentioned the locktime, so now he fixed it up
20:47:39 <cdecker> Yep
20:49:04 <roasbeef> not sure the portion of the diff on bolt2 is well placed
20:50:42 <roasbeef> it's not that it's to make the scripts smaller, instead it makes an off chian covenant since it's a multi-sig and both sides must explicilty sign off on redemption paths
20:50:51 <roasbeef> i'll comments this on the PR, the rest of it looks ok to me
20:51:00 <t-bast> agreed with roasbeef on that part which surprised me a bit
20:51:12 <roasbeef> #action roasbeef to comment on #601 about bolt2 portion of diff
20:51:16 <bitconner> sgtm, and can also figure out it's final resting place
20:51:27 <t-bast> otherwise lgtm
20:51:27 <roasbeef> ppl ok if we move on?
20:51:30 <t-bast> yep
20:51:31 <roasbeef> t-bast: yeh same
20:51:34 <bitconner> yes
20:51:38 <sstone> yes
20:51:41 <cdecker> sgtm
20:51:50 <roasbeef> #topic #604 and #593
20:51:56 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/604
20:52:02 <roasbeef> #link https://github.com/lightningnetwork/lightning-rfc/pull/593
20:52:12 <roasbeef> these are about deciding on the framing/parsing of the unused onion space
20:52:27 <roasbeef> #604 is newer proposed by rusty to compromise on discussion in #593
20:52:42 <roasbeef> the biggest diff is that it defines the payload entirely to use tlv everywhere
20:52:49 <cdecker> So I added the last commit on #593 for the multi-frame signaling. If we decide in favor of #604 we need to cherry pick that over
20:53:05 <roasbeef> this cuts off some redundant data for the final hop, and imo makes this a bit more consistent (no hybrid of old and new payload)
20:53:21 <jtimon> bitconner: yeah, I grep'ed "fail the connection" and it felt like probably more should be revised, but that one just felt clearer because of the "fail the channel" for the offering node
20:53:24 <roasbeef> recent discussion in #593 centered around if ppl should be able to use all available space and also how many possibl tlv/packet types were "enough"
20:53:38 <cdecker> My only concern is that we now have a mapping of realm -> num_frames that needs to be added to the onion processing code
20:53:53 <roasbeef> cdecker: am i correct in that #593 leaves that old 13 padding bytes unused? not sure if just wasn't made explicit that it's part of the greater payload
20:54:13 <roasbeef> cdecker: wasn't it already there with #593? just needed to be parsed out?
20:54:18 <cdecker> roasbeef: the old format allows you to stick additional fields at the end, so you can use them as is now
20:54:39 <roasbeef> cdecker: but it expected them to be zero right? those 13 bytes
20:54:49 <cdecker> roasbeef: no, the feature flag was added to the node_announcement in the gossip
20:55:21 <roasbeef> not sure we're talking about the same thing here, I see that added commit to add the feature bit, would need to be included independent of which approach we go with
20:55:25 <t-bast> I think the mapping isn't more necessary, we can just do 0x00ff & payload[0] to get the number of frames (like we would do 0xff00 & payload[0]) in 593
20:55:26 <cdecker> Not really, it's just treated as a normal wire-packet so if we were to define an additional field to the legacy hop data we could add that
20:56:15 <bitconner> 16 namespaces seems like plenty to me
20:56:19 <cdecker> t-bast: that assumes we only ever have 21 realms? And all realms > 0 are TLV
20:56:28 <t-bast> cdecker: you're right, got it
20:56:31 <bitconner> well, 15 i guess?
20:56:41 <cdecker> bitconner: where does the "namespace" come from?
20:56:52 <t-bast> for other realms we'd need a different parsing logic which is a bit cumbersome
20:56:55 <roasbeef> bitconner: so #593 has 16 namespaces, #604 kinda defers that and makes tese new realm bytes use a new paylaod format that's all tlv so just opauqe blobs passed u p
20:57:06 <roasbeef> cdecker: i think he means packet type
20:57:19 <cdecker> Ok, yes
20:57:24 <cdecker> Got it
20:57:27 <roasbeef> t-bast: so you're saying remove the bit packing and use the first byte?
20:57:34 <roasbeef> in #593
20:57:44 <bitconner> cdecker, yes packet type, since each packet type could map to distinct namepsaces :)
20:57:51 <bitconner> but packet type is more correct
20:58:09 <roasbeef> #604 merges the packet type and the number of frames used
20:58:23 <bitconner> anyway, we will have another chance to consider how many packet types are needed when we redo the onion format to commit to a per-hop version
20:58:23 <roasbeef> biggest thing as mentioned above is that the entire onion payload is now tlv, so the old fields like amount to forward now have a tag
20:58:25 <t-bast> roasbeef: for the first 4 bits (realms 0 to 15) that works but as cdecker points out, for bigger realms the mapping realm -> number of frames might be different
20:58:40 <cdecker> tbh I don't like namespacing since it creates composite types that, spreading the type logic over multiple fields, and they are just coarse-grained
20:59:06 <cdecker> The counterproposal in #593 to the whole namespacing idea is to have var_int types in TLV
20:59:40 <t-bast> bitconner: totally agree that we'll have another opportunity to make more changes when we re-work onion to support per-hop version and we can evaluate at that time if we need more available packet types or something
20:59:59 <cdecker> Which allows us to use much more types, and avoid composite types altogether
21:00:41 <bitconner> t-bast, yeah i don't think we'll be able to exhaust all of the ones we have before that happens (assuming it doesn't take a decade :)
21:01:00 <cdecker> To be clear: all packet-types in #604 would use the same TLV type namespace
21:01:18 <cdecker> The only difference between the packet-types is how many frames the current hop should consume
21:01:28 <t-bast> I think that I slightly prefer #593 because the parsing of the number of frames is a bit cleaner / doesn't leak into type namespacing. But honestly I'm fine with both and I think they would unlock things like trampoline/rendezvous routing which is nice
21:01:32 <roasbeef> but also the "type" is defiend in that realm also
21:02:03 <roasbeef> i lean towards #604 mostly due to the unification there, no old+new just new (speaking of how the payload is interpreted)
21:02:36 <bitconner> cdecker, why not defer specifying the format of those packet types until we have a use case?
21:03:06 <roasbeef> yeah both kinda do that already iiuc
21:03:07 <bitconner> we coulddd use a different namespace if we have a reason to after seeing how the initial deployment of version 1 goex
21:03:10 <bitconner> goes*
21:03:12 <cdecker> bitconner: the multi-frame proposal is concerned solely with how we signal how many frames we need to consume
21:03:55 <cdecker> Mixing in the namespacing for TLVs can be deferred, which is what I'm proposing
21:04:21 <cdecker> The two are orthogonal really, which is why I cam up with the 4 bit / 4 bit split initially
21:04:23 <bitconner> cdecker, oh i see, thanks for the clarifying
21:04:51 <t-bast> the two are orthogonal really -> completely agree
21:05:15 <roasbeef> yeh the bigger thing imo is the clean slate payload in #604
21:05:18 <cdecker> roasbeef: #593 would also allow old and new payload format and has them cleanly separated (lsb 4 bits)
21:05:49 <roasbeef> does it? i find that kinda murky as it's written atm
21:05:57 <t-bast> which is why I think this is enough to unblock many new scenario until we work on another update for per-hop versioning - where we will likely revisit that and have the opportunity of a cleaner slate
21:05:59 <roasbeef> so i need to have 13 bytes between the new and old when I encode?
21:06:02 <roasbeef> do*
21:06:05 <cdecker> I think our main disagreement comes from the fact that #593 as written would allow legacy payloads with multiple frames which may not be sensible since we haven't defined additional fields for that format
21:06:17 <roasbeef> there's that too
21:06:37 <cdecker> roasbeef: you want to mix legacy with an optional TLV at the end?
21:06:44 <cdecker> Sure we can do that easily
21:07:24 <cdecker> But then we merge #593 and then we define that we can have an optional TLV appended to the legacy payload, and we're done
21:07:38 <cdecker> No need for 13 0x00 bytes inbetween
21:07:51 <roasbeef> not that I want to really, but that it wasn't clear to me upon reading if that was allowed or not, and then how the parser would react to that: it'd pass up the first 65 or so as the legacy payload, then the rest opque
21:07:52 <t-bast> sgtm
21:08:00 <cdecker> Since the legacy payload is parsed identically to wire messages we can append anything at the end really
21:08:18 <roasbeef> but then we'd be modifying how the legacy paylaod is parsed?
21:08:24 <roasbeef> as in don't read those 13 bytes
21:08:39 <cdecker> Oh, I see. In that case I'd expect the onion processor to hand up the legacy payload unparsed and have the caller parse it
21:08:56 <bitconner> mixing legacy w/ optional tlv makes sense to me, shaves a few bytes that could keep certain applications from overflowing into an additional frames
21:09:34 <roasbeef> bitconner: #604 adds the overhead for signalling the payload as distinct types, but then since it's a new format entirely it drops the short chan iD for the final hop
21:09:51 <roasbeef> so the code would pass teh blob up, and parsing for the types would be unified
21:09:55 <roasbeef> rather than parse old then new
21:10:03 <cdecker> From what I see that would be perfectly doable: keep legacy payload around, open a new PR that adds an optional TLV in the place of the 13 padding bytes and voila
21:10:22 <cdecker> roasbeef: exactly :-)
21:10:50 <bitconner> awesome, sgtm
21:11:18 <roasbeef> don't follow the second part of that cdecker, why this over the clean slate approach?
21:11:35 <cdecker> Both are doable is what I'm saying
21:11:42 <roasbeef> gotcha
21:11:49 <cdecker> fwiw I'm planning on using TLV exclusively going forward
21:12:19 <roasbeef> ok thx i understand implications of #593 now better, but still lean towards #604 myself, with the undertone that I think we'd all like this to move forward so we can get to the fun stuff it enables
21:12:28 <cdecker> But like bitconner says it might be useful to keep legacy around just to save on bytes and still have the TLV flxibility added
21:12:45 <t-bast> roasbeef: agreed ;)
21:13:01 <cdecker> I have a slight preference for #593 (no suprise there really) but I'm happy with either
21:13:07 <jtimon> what is TLV? where can I read more about that?
21:13:09 <roasbeef> it only saves on bytes for intermediate nodes fwiw, i don't think we lose the flexibility
21:13:26 <roasbeef> jtimon: type length value, a way for forwards/backwards compat when adding new fields to messages
21:13:26 <bitconner> jtimon: check out the open PRs for TLV :)
21:13:37 <jtimon> thanks
21:14:06 <bitconner> sure thing!
21:14:35 <bitconner> okay, so can someone quickly enumerate the multi-frame items that remain to be solidified?
21:14:36 <cdecker> roasbeef: we also have two variants for #593 btw (4bits + 4bits vs 5bits + 3bits) :-)
21:14:42 <cdecker> Just to add a bit of complexity
21:14:43 <roasbeef> kek
21:15:51 <cdecker> Ok, so #604 explicitly enumerates 21 types (I'm assuming we want 20 TLV multi-frame types here), but mixes types and num_frames in an explicit map
21:16:19 <cdecker> i.e., the code processing the onion needs to know the map of packet-type to num_frames to return
21:16:57 <cdecker> #593 separates them into two separate regions of the former realm-byte, but loses a bit of expressiveness
21:17:06 <roasbeef> along side it also makes the entire thing just tlv and drops the old aprior framing (#604 does)
21:17:31 <cdecker> roasbeef: packet-type 0 is still a single frame with the legacy format even in #593
21:17:49 <cdecker> roasbeef: literally nothing changes for that packet-type and we keep it working
21:17:59 <roasbeef> yeh, #604 does away with the legacy all together
21:18:38 <cdecker> packet-type 0 translates into 1 frame (0x00 & 0xF0 additional frames => 1 frame total) and realm 0 (0x00 & 0x0F => realm 0)
21:18:40 <roasbeef> #604 _is_ higher code impact though imo
21:18:58 <roasbeef> cdecker: yeh i get that
21:20:05 <niftynei> what is a "code impact"?
21:20:20 <cdecker> So both proposals keep backward compatibility (legacy payload format is maintained and it has matching packet type 0x00), it literally just in how we decide on how many frames to return :-)
21:20:22 <roasbeef> niftynei: so code to parse the first 65 bytes under the new and old format
21:20:35 <roasbeef> since depending on realm, it'll be diff
21:21:10 <niftynei> also this may be a bit of a naive question, but doesn't doing away with the legacy entirely make it such that onions can't be routed through nodes that haven't upgraded yet?
21:21:32 <cdecker> Hm, I'm actually just handing the blob up to the caller, and the caller can then decode the blob depending on the packet type
21:21:45 <roasbeef> yeah so you wouldn't send it to them based on the feature bit niftynei, it's also signalled in a new realm
21:21:47 <cdecker> (this is the layering thingy I was talking about in the PRs btw)
21:21:50 <roasbeef> or a diff looking realm
21:22:47 <t-bast> non upgraded nodes should fail on unknown realms - but new messages can have some hops that use the old payload and route through non-upgraded nodes
21:23:02 <t-bast> so I think we should be good with both proposals on that front
21:23:45 <niftynei> kk
21:23:47 <cdecker> Absolutely, #604 just goes ahead and also deprecates the legacy format
21:24:08 <cdecker> And that's where the added code complexity comes from
21:25:03 <t-bast> so should we go ahead with #593?
21:25:07 <cdecker> Well actually it just specifies numeric types to be used to recreate the legacy payload in the TLV format, which I would have split into a separate PR
21:26:19 <roasbeef> feels a bit unfair to move on one without giving the author of the other proposal a chime in on any additional context (rusty not here)
21:26:27 <roasbeef> unless one of y'all wants to speak for him? ;)
21:26:40 <roasbeef> i think we're making progress though...
21:26:43 <t-bast> agreed, we can wait for rusty's input (maybe in PR comments?)
21:26:46 <roasbeef> info wise at least
21:26:58 <cdecker> Yep, sounds like we're getting there
21:27:13 <cdecker> We could record an action to move on either once we hear from Rusty
21:27:19 <t-bast> sgtm
21:27:36 <bitconner> pRoGrEsS!?
21:27:42 <cdecker> Shall we count ranked choices so we can trigger once Rusty gives his?
21:27:58 <roasbeef> #action wait for comments/feedback from rusty w.r.t #593 vs #604 on PR
21:27:59 <bitconner> cdecker, sgtm
21:28:38 <roasbeef> cdecker: on teh PR on in these notes? we can add them as #info
21:28:41 <roasbeef> if these notes
21:28:42 <cdecker> Oh, and I'd like to make the split in #593 5bits and 3bits officially so we can use all 20 frames instead of just the 17 we had so far
21:29:21 <t-bast> :thumbs_up:
21:29:34 <t-bast> (lack of slack icons...;))
21:29:45 <roasbeef> kek
21:29:50 <roasbeef> ok, so done here for now?
21:30:28 <t-bast> I'm good with the progress we made :)
21:30:31 <roasbeef> #endmeeting