20:06:28 <bitconner> #startmeeting
20:06:28 <lightningbot> Meeting started Mon Jun 24 20:06:28 2019 UTC.  The chair is bitconner. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:06:28 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
20:07:07 <bitconner> alright, should we start with 619?
20:07:19 <cdecker> We have both pending and accepted
20:07:28 <cdecker> Awesome
20:07:42 <t-bast> Let's go for onion ;)
20:07:51 <cdecker> I think we have a number of things to discuss on this:
20:07:51 <bitconner> #topic https://github.com/lightningnetwork/lightning-rfc/pull/619
20:08:08 <cdecker> 1) Potential future payload formats, or fixing on TLV for now
20:08:37 <cdecker> 2) Keep the legacy format description as is, or generalize the overall structure and move legacy into a more specific section
20:09:05 <cdecker> I think all other things are addressed
20:09:23 <t-bast> concerning 1), I think keeping the realm byte is a fair proposal to provide more flexibility at the cost of only one byte per hop, wdyt?
20:09:43 <t-bast> it also allows defining a termination condition via a special realm value instead of using the last hmac
20:10:00 <cdecker> So keep the realm byte, define 0x01 as TLV, and then have a varint after that to determine the payload length?
20:10:06 <t-bast> exactly
20:10:17 <cdecker> t-bast: I disagree on encoding the last hop in the realm
20:10:20 <t-bast> at least that's how I understood it, roasbeef & bitconner wdyt?
20:10:26 <cdecker> That really should be part of the TLV
20:10:36 <bitconner> t-bast, sounds right to me
20:10:44 <bitconner> cdecker, why is that?
20:10:55 <t-bast> cdecker: but if it's part of the TLV that's a bit of a layering violation though, the onion parser needs to start interpreting the TLV payloads doesn't it?
20:11:05 <bitconner> it seems more complicated to require/not-require fields conditional on the presence of others
20:11:06 <roasbeef> sounds good to be up until the var int, at that point we can still use only 3 bytes by using a fixed sized 2 byte integer there
20:11:49 <cdecker> Oh, I see for the hmac on terminal nodes
20:12:07 <cdecker> Problem is that the HMAC is already being speacial-cased when processing the onion
20:12:23 <t-bast> exactly, instead of signaling termination via 32 0x00 bytes, we signal it via realm = 255. We change the hmac special case to be a realm special case
20:12:55 <cdecker> No, we are not actually acting on the HMAC at all, we just hand it up
20:13:07 <bitconner> t-bast, my original ideaa was to signal with the high order bit, and keep the remaining 7 bits for other versions
20:13:10 <cdecker> Remember that the HMAC inside the payload is destined for the next hop, not ourselves
20:13:22 <bitconner> even 255 is 8x more info than we need :)
20:13:27 <cdecker> I don't see a layering violation with that setup at all
20:13:28 <t-bast> cdecker: oh right, indeed
20:13:30 <roasbeef> yeah it's a diff layer imo, like onion just does parsing, then the thing that actually makes the forwarding decisions is at the channel/link layer
20:13:39 <t-bast> bitconner: definitely, it was just a visual argument :)
20:14:10 <cdecker> TLV has a "terminal" type and then we know that the HMAC being handed up (that would be used when serializing for the next hop) is just garbage like usual
20:14:46 <t-bast> yeah that does work too and is quite simple
20:14:59 <cdecker> Since we are not doing anything with the garbage HMAC in case of a terminal payload, I don't see a layering violation here
20:15:17 <rusty> BTW, Re: realm. 0 is an even type, so future implementations that drop legacy handling will Just Work in that they'll reject it.
20:16:01 <cdecker> Ok, can we agree that using signalling final hop in the realm is not needed?
20:16:21 <t-bast> I agree that we can do it using TLV
20:16:27 <bitconner> cdecker, so then can't we signal the final hop via whether or not there is a short_chan_id present?
20:16:41 <cdecker> Sure, that works as well
20:16:44 <t-bast> and after thinking about it a bit more, it's not the onion parser who should decide termination but rather the layer that interprets the TLV so it makes sense
20:16:53 <cdecker> But I think explicit signalling is better bitconner
20:17:15 <cdecker> Reason being that we might find other ways of specifying the next hop, which also omits the scid
20:17:45 <t-bast> we do agree though that we can choose to signal termination at the TLV interpretation level, do we all agree on that?
20:17:52 <cdecker> It's 2 bytes (1 byte type, 1 byte 0x00 length)
20:17:56 <rusty> t-bast: ack
20:18:00 <bitconner> sgtm, yeah i can see that we may want them distinct
20:18:01 <cdecker> t-bast: ack
20:18:17 <t-bast> cool I think the other question was keeping the realm byte to move away from TLV if needed in the future
20:18:34 <t-bast> and have real 0x01 be TLV stream as currently specified
20:18:47 <cdecker> Ok, then for (1) we just need to decide whether we want to save 1 byte by keeping the realm and add the varint or smash them together
20:18:48 <bitconner> #agreed signal termination via TLV record
20:19:06 <t-bast> I don't have a very strong opinion on that, but it does feel safer to keep the realm as an additional upgrade mechanism
20:19:08 <rusty> t-bast: Hacky as it is, just not defining a type 0 and having code to peek "is the first byte 0?" works.
20:19:46 <bitconner> my preference would be to keep the realm for versioning
20:19:49 <t-bast> rusty: agreed for version 0, but if we want to update to use something else than TLV (like TLV 2.0) we need a way to signal this right? while still supporting tlv 1.0?
20:20:06 <rusty> I think we're fairly wedded to the idea of TLV being the future upgrade mechanism.  We still have the onion version if we want to ugprade.
20:20:35 <roasbeef> i'm for realm and 2 byte fixed integer (no need for var int for signalling, but it's more interesting in the onion for values since we can save some space)
20:20:40 <bitconner> rusty, would say upgrade path on the outer version isn't super clear atm
20:20:40 <t-bast> if we're fully committed to TLV no need for the realm. If we're having doubts I think we need it :)
20:20:49 <roasbeef> yeh i'd like to keep the real upgrade path in tact, juuust in case
20:20:52 <cdecker> Just so I mentioned this: the current writeup special-cases realm 0x00 and uses all others as the byte length of the payload
20:20:57 <t-bast> my first comment was also that we still have the outer version if needed
20:21:33 <cdecker> roasbeef: I still want the varint for the length, since 90+% will use <253 byte payloads we can really safe quite a bit there
20:21:56 <t-bast> roasbeef: I'm not sure it saves some space, because it means each payload always needs 2 bytes for length whereas most payloads will be less than 253 bytes so would need a single byte...I'd say we should optimize for the most common case so I'm not a big fan of fixed 2 bytes length
20:22:03 <rusty> cdecker: ah, yes, of course.  So the byte is not redundant.  While I'm all about future compat, I think this is a bit too far.
20:22:30 <roasbeef> t-bast: the var int type can use a single byte for smaller values? iirc the bitcoin var int is also 2 bytes minimal
20:22:47 <t-bast> roasbeef: no, varint is 1 byte for values less than 253
20:22:51 <cdecker> roasbeef: values <253 are represented in a single byte
20:22:53 <roasbeef> gotcha
20:22:53 <t-bast> that's why I think it's nice
20:22:58 <cdecker> Yep
20:23:01 <t-bast> (for onion)
20:23:12 <roasbeef> ok seems more worth it in that case, we're talking about the length prefix here right?
20:23:21 <t-bast> yes exactly
20:23:21 <cdecker> Yep
20:23:31 <rusty> We can also reserve length 1, since that's impossible too, if we want.
20:23:44 <t-bast> good point
20:23:55 <cdecker> rusty: pretty much any value below 8 is likely to never be used for the length
20:24:09 <bitconner> since we're saving one byte on the length, we can reinstate realm for free ;)
20:24:15 <niftynei> lol
20:24:16 <cdecker> So we could reserve realm/packet_type 0-7 for other formats
20:24:17 <t-bast> :)
20:24:27 <t-bast> that sounds like a good compromise
20:24:35 <rusty> cdecker: not if clever people end up encoding everything in the shared secret :)
20:25:11 <t-bast> rusty: that sounds fancy, I want the logs of such a proposal :)
20:25:12 <cdecker> That's going to be hard rusty, basically breaks the crypto assumptions we have if we try to pack fixed information in there :-)
20:25:54 <t-bast> roasbeef, bitconner, what do you think about that compromise?
20:26:22 <cdecker> Compromise being realm byte intact + varint for payload length?
20:26:24 <t-bast> we keep the current proposal by cdecker (no realm byte) but still have values 0-7 as an escape hatch if we really want to get rid of tlv
20:26:38 <cdecker> Oh, I see, thanks t-bast
20:27:21 <t-bast> we can explicitly disallow length values beneath 8, or just assume that no-one is going to be using those anyway
20:27:51 <bitconner> would still lean towards realm+varint, overloading seems less than desirable
20:27:53 <rusty> t-bast: I'm uncomfortable with that.  len=1 doesn't need to be special-cased.
20:28:18 <cdecker> I mean, I pretty much like the idea of keeping the realm byte in place and not be as stingy with space, but I also like the current writeup and want it merged :-)
20:28:35 * cdecker flipflops a lot when he's jetlagged
20:29:03 <rusty> cdecker: I think it's worth thinking of what this looks like in a post-legacy world.  The realm byte there will look odd, IMHO.
20:29:28 <roasbeef> so this comnbines both relam and legnth? why not clean separation?
20:29:55 <rusty> roasbeef: because, since length < 2 is impossible, it is a clean separation :)
20:30:22 <t-bast> rusty: that means length = 1 would need to be treated as a completely new thing right? to keep interpreting the bytes that come after?
20:30:30 <cdecker> roasbeef: the question is much more whether we want to make legacy a special case and TLV the default or to keep both at the same level
20:31:10 <t-bast> cdecker: agreed, good way of clarifying it
20:31:15 <rusty> t-bast: sure, but that's beautiful.  We don't need to do *anything* until/unless we decide we need a new version.  Currently implementations will already fail to parse len=1.
20:32:25 <t-bast> rusty: I agree that it looks clean, I'm ok with that solution (as I'm not even sure this escape mechanism will ever be needed)
20:32:40 <t-bast> that would mean keeping the proposal as-is currently in the PR right?
20:33:07 <bitconner> we'd sill need to add the signaling condition
20:33:13 <roasbeef> wouldn't say having a double interpretation of the field is a clean separation, it's overloading the length
20:33:29 <t-bast> bitconner: yes of course, just on the realm vs no-realm discussion
20:34:08 <t-bast> roasbeef: it saves up to 20 bytes!!! maybe even a few more, I know you could squeeze many cool things in those bytes :)
20:34:26 <rusty> roasbeef: sure.  But it's even cleaner once we get rid of legacy support.  Y'know, when we're retired :)
20:36:51 <bitconner> as much as i'd like to think we will never need to upgrade the inner version, that isn't the conclusion experience would lead me to :)
20:37:15 <bitconner> so i think we want some versioning, and whether we want those upgrade paths to be hindered by the overloading is unclear
20:37:38 <bitconner> well, actually i'd say it'd be preferable that the inner versions are all on the same level as cdecker phrased it
20:38:03 <rusty> bitconner: my experience tells me the opposite. We went for a flexible format so we don't have to upgrade again, but we've still left an "out" if we need.  I'm happy.
20:38:37 <bitconner> the out being 0-7 bytes?
20:38:40 <t-bast> bitconner: that means you don't believe that much in TLV, even though you made the PR!
20:38:43 <bitconner> realm 0-7*
20:39:03 <rusty> bitconner: well, I don't like 0-7, but 0-1 I'm happy with since it happens automatically.
20:39:05 <bitconner> t-bast nothing to do with believing in tlv
20:39:14 <t-bast> bitconner: I know, just teasing ;)
20:39:37 <bitconner> tlv isn't a magic bullet :)
20:39:55 <t-bast> but we still always have the onion outer version explicitly there for upgrades
20:40:15 <t-bast> I don't understand why we're not feeling that as an upgrade safety net
20:40:49 <roasbeef> because the outer oino version change is a hard fork essentially, it isn't committed to at all
20:40:53 <bitconner> the outer version is more difficult to change, it requires a network-wide upgrade to be effective
20:41:15 <bitconner> all hops in the path must use the same outer version, so you're always constrained to the lowest common denominator
20:41:16 <t-bast> why? because intermediate nodes would override your version with a 0?
20:41:40 <t-bast> I think it would just mean that you wouldn't be routing through nodes that only support v0 indeed
20:41:55 <t-bast> but I think that would likely be the case anyway if you want to upgrade routing
20:42:07 <t-bast> regardless of whether you use the outer version or only inner versions
20:42:22 <t-bast> and you'll be looking at each node's feature flags anyway before including them in a route with new features
20:44:12 <rusty> ... and that's only once we've exhausted the len=1 upgrade option.  My experience leads me to believe that we are not likely to use either, FWIW.
20:44:27 <t-bast> I think so too
20:45:20 <bitconner> okay so is there agreement on len=1 upgrade path being sufficient?
20:45:31 <t-bast> sgtm
20:45:55 <rusty> ack
20:46:59 <bitconner> #agreed no-realm, signal future versions with low-value lengths
20:47:31 <rusty> This means, aside from perhaps a note that we can use len=1 in future for upgrades, we can apply as-is?
20:47:56 <bitconner> once the signaling mechanism is added i think so
20:48:20 <t-bast> I think so, only a few nits to fix and termination signaling to describe and it should be ready
20:48:24 <t-bast> decker, wdyt?
20:48:33 <t-bast> *cdecker
20:48:37 <bitconner> #topic https://github.com/lightningnetwork/lightning-rfc/pull/557
20:48:52 <t-bast> shouldn't we do 607 before 557?
20:49:01 <t-bast> I think 557 builds on 607
20:49:01 <bitconner> i think since last time, extended gossip queries replaced Adler32 with CRC32
20:49:14 <bitconner> t-bast, i think 607 was good just need to add test vectors
20:49:19 <rusty> Yes, sorry, I've been distracted, so I need to revisit 557, with tests.
20:49:21 <t-bast> bitconner: perfect, sgtm
20:49:35 <bitconner> wanted to get this in before time is up
20:49:42 <t-bast> cool thx
20:49:55 <bitconner> anyone else have comments on 557?
20:50:25 <sstone> the test vectors I added for 557 should be valid with the latest TLV PR
20:51:00 <bitconner> sstone: great!
20:51:25 <rusty> bitconner: I am happy to ack it, and have the test vectors added as a separate commit.
20:51:35 <rusty> (After I've tested them of course!)
20:51:43 <bitconner> rusty: we talking about 557 or 607?
20:51:57 <rusty> Sorry, 607.
20:52:38 <bitconner> ah okay, well looks like both could use committed test vectors actually :)
20:52:57 <rusty> 557 I'm still working on our implementation.  Apologies, but it should now go much smoother since we have TLV defined.  Also, it's our first use of TLV in the spec, so I need to make sure we're happy with the exact format.'
20:53:31 <bitconner> rusty: yes that reminds me i had a comment on 619 regarding the tlv field formatting
20:53:32 <rusty> (I did want to ask about tools/extract-formats.py -- do other people care or can niftynei and I go nuts in there?)
20:53:58 <bitconner> sure don't see why no
20:54:01 <bitconner> not
20:54:04 <niftynei> :D
20:54:06 <t-bast> have fun ;)
20:54:35 <bitconner> okay so seems 557 still good from design perspective, waiting on impls?
20:54:44 <t-bast> sgtm
20:54:57 <rusty> Good.... I've got a very early set of conversational test (finally!) running, which use that to parse the spec to create messages.  It's pretty sweet, hope a topic for next mtg.
20:55:01 <rusty> bitconner: ack.
20:55:43 <bitconner> #agreed design still solid, waiting on feedback from second impl
20:56:09 <rusty> sstone: note, with CRC we should specify the Castagnoli CRC-32C polynomial used in iSCSI or SCTP.  We can include a ref to the iSCSI test vectors, too.  THat's what Intel implemented, FWIW.
20:56:59 <cdecker> Sorry for not replying on 619, I'll amend by adding the terminal signalling and the special-case for realm=0x01 as an escape hatch
20:57:05 <sstone> rusty: thanks I'll update the PR
20:57:24 <t-bast> Do we have some time to discuss feature bits unification and large channels? #571 and #596. I think those are pretty small and simple but useful.
20:57:28 <rusty> cdecker: you don't specify 0x01, since it's already going to fail parsing.  Just note it.
20:57:56 <bitconner> sure t-bast
20:58:13 <rusty> YEah, 571 is almost entirely janatorial.
20:58:27 <bitconner> #topic https://github.com/lightningnetwork/lightning-rfc/pull/571
20:58:51 <rusty> Renaming, and making sure feature bits don't overlap.  Removing some cases where we said we should reject msgs, which was short-sighted.
20:59:15 <bitconner> rusty, why do we refer to them as channelfeatures? and not something like connfeatures?
20:59:28 <bitconner> since there can be more than one channel, and the features may apply to more than just channel operation
21:00:07 <t-bast> do you have an example on something else than a channel operation which wouldn't be a nodefeature?
21:00:12 <rusty> bitconner: *channel* features are things people need to know about the channel.  ie. externally visible (prev global).
21:00:48 <rusty> bitconner: node features are much more common (we don't have any channel features yet)
21:01:17 <bitconner> oh i see, misread that as the renaming for localfeatures
21:01:32 <bitconner> still though, that can be used to signal other node-related things that don't apply to channels
21:01:44 <bitconner> like option_i_haz_channel_proofs
21:01:59 <rusty> bitconner: it's advertized in channel_annoucement...
21:02:29 <rusty> bitconner: option_i_haz_channel_proofs is a node feature, no?
21:02:46 <rusty> (Depends on what channel proofs are I guess?)
21:05:30 <roasbeef> yeh would say that's a node feature, but it depends on how we roll it out: is it attached to all chan anns, or do you query for them on a per channel basis?
21:06:03 <araspitzu> #590 uses the global features atm, with #571 should it become a channel feature? If so then using `channel_update.htlc_maximum_msat` for routing information makes the large channel option kinda redundant in the new channel features
21:06:05 <rusty> roasbeef: since a third party can attach it, I assumed it'd be for all channel_anns.
21:06:20 <rusty> araspitzu: indeed.
21:07:07 <araspitzu> how about encoding 'this node supports opening channel with feature X'? it seems a better fit for node features
21:07:09 <rusty> roasbeef: I don't think we want the channel_announce to commit to it, that'd be weird.  It's like an addendum, a new "channel_announcement_plus" message, I think.
21:07:11 <roasbeef> would potentially be a lot of bandwidth if it's on all the channel anns though
21:07:29 <roasbeef> especially for nodes that don't need them at all
21:08:34 <bitconner> for me i think the naming is confusing bc it's the opposite of how it reason about local vs global sets
21:08:36 <rusty> roasbeef: indeed.  We could waste a RTT and ask for proofs explicitly, or have a feature or msg to say "attach proofs to all channel_annnouncements please".
21:09:42 <rusty> bitconner: yeah, I dislike the naming too.  Hence, the original global or local, but people confused that as well.  Perhaps 'routing' and um... ?
21:10:02 <rusty> Shall we bikeshed over names before next meeting?  That worked well before....
21:10:08 <bitconner> lol
21:11:41 <bitconner> do people have a strong desire to proceed with the clean up? or do we think we need more bikeshed?
21:11:42 <rusty> OK, so I think the distinction is fairly clear, but naming needs work.  Next issue?
21:11:49 <bitconner> sgtm
21:12:00 <bitconner> #agreed work out naming of feature bit groups
21:12:04 <t-bast> sgtm
21:12:07 <araspitzu> sgtm
21:13:01 <bitconner> #topic https://github.com/lightningnetwork/lightning-rfc/pull/596
21:13:29 <bitconner> major comment here looked to have been whether we start at 0 or 16
21:13:30 <araspitzu> do we want to merge the large channel bit with the current features or we bikeshed it until #571?
21:13:50 <t-bast> I think we can merge with the current features, right?
21:14:03 <t-bast> if we agree on the name for this feature :)
21:14:05 <bitconner> araspitzu, that was going to be my suggestion as well. possible to proceed with them as disjoint sets
21:14:22 <araspitzu> yes it sounds right to me too :)
21:14:38 <t-bast> the comment about namespace was to allow the feature bit unification (and combining local/global features)
21:14:58 <t-bast> it feels to me that it's safer to use a bit that's disjoint from local feature bits, wdyt?
21:15:12 <araspitzu> if we want them completely disjoint then it should start using globalFeatures from 0
21:15:53 <araspitzu> and the unification would reorg the features accordingly (and avoiding overlap)
21:15:54 <rusty> araspitzu: I like the idea, I'd like to give some minor textual feedback on the issue itself.  Concept ack from me.
21:16:15 <rusty> (For technical reasons I think this should be merged after #571)
21:17:06 <bitconner> araspitzu: a slight naming suggestion: option_remove_funding_limit since it's more specific. up to you though :)
21:17:29 <araspitzu> rusty: isn't #571 backward compatible?
21:17:45 <rusty> araspitzu: option_what_the_hell_is_wumbo works too....
21:17:51 <t-bast> araspitzu: I think my sentence ended up saying the opposite of what I meant: I think it's safer to use feature bit values that are disjoint between local and global to be able to merge them in a single byte array (what's currently done in the PR)
21:17:52 <araspitzu> eheh
21:18:22 <rusty> araspitzu: it is, just want to avoid merge hell :)
21:18:30 <bitconner> t-bast: yes i agree
21:18:47 <rusty> I've acked it, with some minor comments, now.
21:19:21 <bitconner> question: will this also be set in the localfeatures?
21:19:35 <t-bast> great, so the decision is a concept ack but we wait for 571 to be merged (hopefully next meeting) before merging 596?
21:19:37 <bitconner> or is the receiver supposed to just reject if they don't want the channel
21:19:52 <rusty> t-bast: ack
21:20:02 <t-bast> bitconner: I think you should reject if you don't want that channel indeed
21:20:24 <t-bast> bitconner: I don't think it needs to be reflected in the local features, but maybe someone has a good reason to reflect it?
21:20:44 <rusty> bitconner: it's a local feature, yes.  But with #571 we advertize those in node_announcement too, so we can see them without having to connect to find it.
21:20:48 <bitconner> do we have a way of signaling that the reason for the failure is that i don't want a large channel?
21:21:18 <rusty> bitconner: we use generic error to reject too-small, too-large, too-salty channels...
21:21:32 <rusty> bitconner: though if I don't offer the feature, you shouldn't even try...
21:21:33 <araspitzu> t-bast: ack (i'll rebase that on #571)
21:22:31 <bitconner> rusty: yes but what you support could beb different from the latest node_announcement i have, setting it on the localfeatures upon connection would clarify support at the time of the request
21:22:49 <rusty> bitconner: that's always true, yes.
21:23:12 <rusty> node_announcement is always kind of best effort.
21:23:37 <bitconner> but since it's set in globalfeatures, we should be okay i think
21:23:47 <bitconner> cool then ack from me
21:23:53 <rusty> bitconner: ?  No it's not.
21:24:12 <bitconner> the text says the feature bits can be sent in init?
21:24:31 <rusty> bitconner: both global and local are sent in init
21:24:43 <bitconner> right, so why wouldn't we set it there?
21:25:01 <rusty> bitconner: we would.  I'm confused...
21:25:49 <bitconner> my question is: is option_support_large_channel set in init upon each connection, in addition to the node_announcement
21:26:08 <araspitzu> bitconner: yes it is
21:26:11 <bitconner> great
21:26:17 <bitconner> thanks!
21:26:44 <bitconner> i think that clarifies all of my questions
21:27:31 <bitconner> #agreed rebase on 571 and await merge
21:27:32 <rusty> OK, I have to go, but good progress.  Let's bikeshed later, OK!
21:27:40 <bitconner> sounds good, later rusty!
21:27:41 <t-bast> ack!
21:27:42 * cdecker wants to point out that #619 was updated with the requested changes before we close the meeting :-)
21:27:52 <t-bast> cdecker: faster than lightning
21:27:55 <bitconner> awesome thanks cdecker!
21:28:07 <bitconner> any other topics we'd like to discuss?
21:28:23 <lndbot> <joost.jager> 608?
21:29:02 <lndbot> <joost.jager> proposal has been improved since last time it was discussed. it should fix the probe vector without sacrificing the info that we get from the failure
21:29:33 <t-bast> joost: ack
21:29:52 <cdecker> I don't think it addresses probing at all tbh
21:30:20 <cdecker> Oh, wait I didn't see that it only protects against intermediate nodes
21:30:50 <cdecker> ack
21:30:55 <lndbot> <joost.jager> it is similar to the previous merging of `incorrect_payment_amount` into invalid payment details
21:31:13 <bitconner> #topic https://github.com/lightningnetwork/lightning-rfc/pull/608
21:31:24 <bitconner> joost: ack from me
21:31:27 <lndbot> <joost.jager> the new thing compared to previous time it was discussed, is that the final hop now returns its current height. that way we, as a sender, can distinguish between intermediates delaying on the forward path vs us using the wrong payment details
21:32:20 <lndbot> <joost.jager> otherwise we'd get an `incorrect_or_unknown_payment_details` without knowing whether to retry via another route or stop trying
21:33:31 <bitconner> makes sense to me
21:33:59 <bitconner> we have some acks from various impls
21:34:38 <bitconner> might be good to have rusty chime in as well, but the meeting is quite a bit over
21:34:48 <bitconner> will mark as tentative approval
21:34:52 <t-bast> maybe tag him on github to ask for a review?
21:35:11 <bitconner> #agreed tnetative ACK, looking for rusty's feedback before final approval
21:35:20 <t-bast> joost: btw I discussed with some Tor people and it looks like they managed to achieve what you want to achieve for error message authentication with a fixed-length. They referred me to Tor proposal 295 for details, it's probably worth having a close look at how they do it. Anyone familiar with this already?
21:35:45 <lndbot> <joost.jager> tentative ack, sounds good
21:36:18 <bitconner> t-bast, not familiiar with 295 myself, but should read up on it
21:36:38 <lndbot> <joost.jager> tor parallels sounds interesting. do you have a link?
21:36:56 <t-bast> https://github.com/torproject/torspec/blob/master/proposals/295-relay-crypto-with-adl.txt
21:37:13 <t-bast> at first glance it might be tightly coupled with AES-GCM so it would not work for us
21:37:18 <t-bast> but worth investigating
21:38:15 <bitconner> thanks t-bast
21:38:18 <t-bast> ah no AES-CTR, strange
21:38:25 <bitconner> #endmeeting