19:18:37 <t-bast> #startmeeting
19:18:37 <lightningbot> Meeting started Mon Jan  6 19:18:37 2020 UTC.  The chair is t-bast. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:18:37 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:18:54 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/pull/719
19:19:11 <t-bast> This is a PR to clarify dependencies between features
19:19:35 <cdecker> #info Happy new year 2020, the year of the Lightening :-)
19:19:36 <t-bast> It also caught the fact that we left out var_onion_option from the dependencies of most new features
19:20:04 <roasbeef> t-bast: some ppl had issues scanning invoices made by pheonix from lnd
19:20:19 <roasbeef> iirc it was that they didn't specify the new onion format, but specified mpp
19:20:24 <t-bast> roasbeef: yep because we didn't include the var_onion_option bit because the spec explicitly left it out
19:20:38 <t-bast> currently the spec said that it was a feature bit that we could not put in invoices
19:20:51 <t-bast> we should fix the spec, which both conner and bluematt did in their PRs :)
19:21:11 <t-bast> BTW is lnd ready for complete compat' testing of MPP?
19:21:27 <roasbeef> yep master can recv, 0.9 (coming soon) will be able to recv as well
19:21:47 <roasbeef> you can send on teh RPC level, but only with multuiple nodes paying a single invoice, so like crowd funding style
19:22:01 <t-bast> great, I'll start doing some tests against master this week then!
19:22:03 <joostjgr> hi all
19:22:11 <t-bast> hi Joost
19:22:14 <joostjgr> what we can't do is sending multiple parts from a single node
19:22:32 <t-bast> good to know, thanks, I'll make sure I used two nodes
19:22:36 <joostjgr> it is possible to send multiple parts, but 'crowd sourced' from multiple instances
19:22:42 <roasbeef> so let's roll in #723 into #719?
19:23:13 <t-bast> #723 has another point about feature handling, but we can definitely merge those two if we ACK them all
19:23:23 <BlueMatt> fine by me. if you just take the second commit the first comit is redundant with 719
19:23:31 <t-bast> #719 needs an ACK from cdecker or niftynei
19:23:40 <t-bast> #723 needs an ack from someone LL-side
19:24:18 <t-bast> BlueMatt or ariard, do you have comments on #719?
19:24:25 <cdecker> I think we are already compliant with #719
19:24:42 <BlueMatt> 719 lgtm
19:25:04 <BlueMatt> cdecker: well you also have to reject on the receive side....like, reject the message
19:25:30 <BlueMatt> and disconnect them
19:25:45 <BlueMatt> actually, 719 should specify what you should do
19:25:57 <BlueMatt> it just says "MUST validate" but that doesnt mean anything
19:26:10 <t-bast> actually on #719 it seems like conner made a mistake, and added `9` to `gossip_queries` instead of `var_onion_optin`
19:26:31 <cdecker> Yes, I don't see any prescribed action for recipients in there
19:27:16 <cdecker> You're right t-bast allowing gossip signalling in the invoice is not correct imho
19:27:41 <BlueMatt> alright, so 719 clearly needs work, lets boot conversation to github.
19:27:44 <t-bast> Then let's let conner fix that and add some requirements
19:27:54 <t-bast> Let's move to 723 which we can probably merge
19:28:11 <t-bast> #action Fix a few comments on #719 before merge
19:28:18 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/pull/723
19:28:45 <ariard> a strict definition of transitive feature dependency could be laid out before the "MUST set all transitive feature dependencies" tho
19:29:06 <roasbeef> ariard: #719 attempts to do that
19:29:19 <niftynei> 723 lgtm, other than the fact that "should in general override" is too weak of language imo
19:29:45 <BlueMatt> niftynei: that's a non-normative note for future spec changes
19:29:57 <BlueMatt> hence it being in the "Rationale" section.
19:30:11 <ariard> roasbeef: that just a nit for clarity but #719 tells you what are actual dependencies but not formal definition
19:30:36 <niftynei> i get that, i'm in favor of making it stricter and letting future spec revisions relax it, rather than hedging in the present
19:30:52 <cdecker> btw I'm removing the meeting discussion tags as we address them, so the issues don't accumulate over time
19:31:00 <BlueMatt> niftynei: hm? it being in the Rationale section is a note *for us* for future changes. not for implementors
19:31:20 <roasbeef> ariard: def of transitive deps? what's the advantage over just enumerating them all?
19:31:33 <t-bast> cdecker: thanks!
19:31:45 <BlueMatt> ie "in the future, as spec changes are made, keep in mind that you should probably let node_annoucement be overridden by invoices, but also as an implementor note that this is likely to be how things work, so keep that in mind"
19:31:53 <roasbeef> cdecker: well some get discussed, then just sit aroudn, so they should retain the label until things are either "dropped" or they move forward
19:32:14 <BlueMatt> Rationale is not the place to put "strict" things, cause they have no strict meaning
19:33:01 <cdecker> roasbeef: I think of the tag as "to be discussed in the next meeting", so auto-adding them to the meeting list is probably not the desired effect, i.e., if the required changes have not been done, we just lose time asserting that the problem persists :-)
19:33:10 <niftynei> right, my preference is that the rationale is more opinionated, which you can do by removing the 'in general' :P
19:33:50 <ariard> roasbeef: bolt clarity but nevermind
19:34:01 <niftynei> the 'in general' makes me think that you've already got an exempting case in mind or that exists, which afaik is not the case
19:34:14 <cdecker> Either make the point weaker ("notice that contradictions may arise") or make it normative
19:34:49 <BlueMatt> ok, done, removed "in general"
19:35:01 <niftynei> awesome. thanks
19:35:09 <cdecker> SGTM
19:35:26 <t-bast> ACK
19:35:42 <t-bast> roasbeef, joost, you ok with merging #723?
19:36:11 <roasbeef> would want to check it out in detail first, and also give conner a chance to look over it since he's been doing a lot of feature bit clean up in lnd lately
19:36:30 <BlueMatt> roasbeef: its like 100 chars different, you can read it quick :p
19:36:32 <t-bast> It's quite a small one honestly
19:36:35 <roasbeef> i think we already use the onion bit in the invoice
19:36:51 <BlueMatt> t-bast: last I heard we only require two implementation acks to merge something anyway :p
19:37:10 <t-bast> Heh ;)
19:37:46 <cdecker> Well, there's no hurry is there? We can count ACKs on the issue, so why not let Conner weigh in?
19:38:19 <BlueMatt> cdecker: cause we have a ton of tiny trivial changes piling up, waiting for an ack from someone who proposed the same change in another pr........
19:38:21 <BlueMatt> whatever.
19:38:30 <t-bast> SGTM, let's merge this once conner acks
19:38:42 <t-bast> #action ping conner for ACK then merge
19:39:13 <t-bast> I agree with BlueMatt though that overall those small PRs aren't looked at and pile up. I think we could do a better job at reviewing them outside of the meetings to speed up the meeting itself...
19:39:48 <cdecker> Definitely, I know I could do a better job working through the tiny/uncontroversial stuff before the meeting
19:40:31 <t-bast> Let's move to two issues that a bit of discussion
19:40:35 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/issues/721
19:40:54 <t-bast> This is a remark on a potential attack on MPP that's not explicitly handled by the spec
19:41:24 <t-bast> I think we could fix this with a small spec change, are implementations vulnerable to that today?
19:41:29 <cdecker> imho this is pure receiver policy and shouldn't normative
19:42:34 <t-bast> but at least we should mention it then, otherwise the receiver can simply lose everything because the implementer hadn't thought about that attack
19:42:46 <cdecker> You can also steer this pretty easily through the number of channels you publicize (multiple tiny htlcs on a single channel aggregate, unless the attacker happens to peer directly with the recipient).
19:42:48 <roasbeef> it's also related to the min htlc value on the final channel
19:43:14 <BlueMatt> right, this can be handled with a min htlc value/max_htlcs_in_flight
19:43:19 <roasbeef> if you set that high enough, they you guard against this as the sender shouldn't route a payment too s mall, and the penultimiate node should reject and not forward on the outgoing channel
19:43:22 <BlueMatt> ultimately its the same risk number
19:43:30 <t-bast> It's true that it's mostly an issue for big nodes with many channels
19:44:03 <BlueMatt> if you're running a big node taking lots of payments, the risk from a number of payments is the same as the risk from one payment across a number of MPPs
19:44:11 <BlueMatt> err MPs
19:44:53 <t-bast> IIUC your overall feeling is that this isn't a big concern in practice?
19:45:01 <BlueMatt> now I wouldnt object to, eg, MPP may be paid with no more than 128 HTLCs, but its not due to this attack, more a general nice thing
19:45:23 <roasbeef> t-bast: it can be mitigated by properly setting the minhtlc value in the channel policy, may be nice to make it more explicit in like a best practices section
19:45:34 <BlueMatt> this *is* a concern, but it has nothing to do with MPP, and is exactly why max_htlcs_in_flight and htlc_minium_msat exist.
19:45:40 <niftynei> yeah this is a specific case of a general problem, i think what roasbeef is saying about the minhtlc value makes sense
19:46:01 <cdecker> Yep, same here
19:46:18 <BlueMatt> sounds like we're all on the same page, yea.
19:46:25 <t-bast> gotcha
19:46:29 <cdecker> We need to be careful not to overload the spec with too many details that are just special cases of what we already handle imho
19:46:42 <BlueMatt> somewhat tangentially, it may make sense to have a general limit just to, eg, restrict things to sanity
19:46:50 <t-bast> let's move on then, I'll summarize those points on the issue for reference
19:47:09 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/issues/724
19:47:09 <BlueMatt> eg 128 chunks, but thats mostly cause like more than that is obviously someone doing something stupid (or over-wumbo-ing pre-wumbo)
19:48:05 <t-bast> I noticed that c-lightning changed its behavior to more aggressively re-send its node_announcement to work around what sstone describes in this issue
19:48:19 <roasbeef> t-bast: fwiw we're skipping over things that are marked as meeting discussion and adding things that weren't tagged label wise
19:48:28 <roasbeef> but yeh this is relevant, node ann's don't propagate super well rn
19:48:42 <roasbeef> seems this is solved by reviving the inv proposal
19:48:54 <roasbeef> which would need a way to fetch a node ann in isolation
19:48:59 <t-bast> roasbeef: is there something specific you want to discuss? It feels to me that apart from anchor outputs which hasn't changed, we only have small PRs that are all waiting review that can be done outside of the meeting...
19:49:26 <t-bast> That's not necessarily very easy to do with INV either...
19:49:38 <t-bast> Long term may be using minisketch to reconcile node_announcements
19:49:43 <roasbeef> t-bast: no nothing in particular, just ref'ing that
19:49:56 <roasbeef> t-bast: why isn't it easy to do with an inv?
19:50:08 <roasbeef> you don't want _all_ the node anns, only those that you may care about or frequently use also
19:50:22 <sstone> roasbeef: I don't think that node inv will let you quickly reconcile your node anns
19:50:32 <t-bast> Mostly because INV is more work than enriching the existing queries, so it would need to add more to be really meaningful IMO
19:50:35 <roasbeef> sstone: it depends on what the goal is
19:50:52 <roasbeef> do you want 100% up to date node anns at all time, or do you want to be able to get the node anns you care about
19:51:30 <sstone> roasbeef: the goal is to be able to quickly learn about new/updated node anns, possible with feature and/or timestamp based filters
19:51:41 <sstone> possibly
19:52:49 <sstone> I see INV as a way to reduce broadcast bandwidth, but we still need queries
19:52:52 <roasbeef> feature filter seems desirable
19:53:16 <roasbeef> yeh the inv would add a way to query for the node ann, otherwise it's only half operational
19:53:54 * BlueMatt -> meeting, see y'all.
19:53:55 <roasbeef> lack of propagation of node anns also seems to have been exacerbated by some impls no longer asking for any sort of gossip backlog when connecting
19:54:08 <cdecker> See you BlueMatt :-)
19:55:08 <t-bast> Do you feel working on a broader INV proposal that encompasses this is something we should do now? Or should we work on extending the existing queries to help node_ann propagation and implement some filters? What's the general feeling on the most urgent work needed on gossip right now?
19:55:16 * t-bast waves at BlueMatt
19:55:21 <roasbeef> sstone: the extended query stuff doesn't cover this as is today? or I guess it would be an avenue to address this?
19:55:48 <t-bast> we could extend the existing extended queries to add node_announcement and filters
19:56:02 <t-bast> if people feel that it's useful we can work on that
19:56:26 <sstone> roasbeef: not well. you can ask for node anns but it's the same issue as with chan. updates: we need additional info such as timestamps
19:56:29 <t-bast> we'll call them super-extended-queries
19:56:48 <roasbeef> in the past I wasn't a huge fan of the checksum approach in the extended queries, which is why I mostly stopped commenting on it, lack of granularity meant that you shoudl be forced to always fetch in case of a change of which you may not have cared about
19:57:22 <niftynei> it seems that there's a general case about being able to query for 'newer gossip' via timestamps?
19:58:05 <sstone> niftynei: yes. we can already do that for updates but not node anns because it was not needed until now
19:58:11 <roasbeef> there's that, then also possibly being able to filter based on feature bits
19:58:26 <roasbeef> main thing is that node anns weren't very important til we started to actually use the global feature bits namespace
19:58:34 <ariard> did anyone explore using minisketch with multiple reconciliation sets per-feature or object of interest?
19:58:41 <niftynei> yep, that sounds about right
19:58:55 <niftynei> i know rusty  has been working on a minisketch compaction/filter for gossip
19:59:08 <roasbeef> ariard: i think rusty looked into basic integration, but idk feels like too big of a hammer for this simple case
19:59:19 <t-bast> ariard: we had an intern working on that for a few months, and I know rusty has been doing some experimentation too - but I think it's a longer term solution, we'll need something before
19:59:30 <niftynei> (i might be using the wrong term here, i'm not totally clear on what minisketch is composed of)
20:00:10 <t-bast> it's not trivial to correctly use minisketch for lightning - done naively it ends up being worse than IBLT
20:00:35 <t-bast> definitely doable, but more long-term
20:00:38 <ariard> okay good to know, well I'm following how it's going to be deployed on the base layer and learn from that before going further
20:00:53 <t-bast> great we'll learn a lot from that
20:01:07 <niftynei> it sounds like a extending query extensions is a reasonable move
20:01:12 <roasbeef> yeh we don't need to use every new shiny hammer just because it exists or was produced for other use cases
20:01:28 <t-bast> My proposal is that we could start drafting something to add filters to queries and node_announcement capabilities
20:01:28 <roasbeef> yeh seems so, iirc there's a PR that half way adds node anns to them? or maybe was just a todo left over
20:01:41 <t-bast> sstone and I will work on that for next meeting, how does that sound?
20:01:42 <ariard> right right, but having already a minisketch production library is cool too
20:01:49 <niftynei> sounds great t-bast
20:02:36 <t-bast> roasbeef: there's already *some* node_anns capabilities, but not enough :)
20:02:38 <roasbeef> t-bast: could be good to maybe first to an ML post to state the goals of w/e the new additions would be, and greater context in the network, etc
20:02:40 <sstone> sgtm. you can already query node anns but it useful only for channels you had not seen yet
20:02:53 <roasbeef> what's missin is feature bit filtering?
20:03:05 <t-bast> roasbeef: good idea, we'll start with the mailing list
20:03:18 <t-bast> not only, right now you can't know if a node_ann has changed compared to what you have
20:03:26 <t-bast> we need to use checksum or something else
20:03:34 <roasbeef> gotcha, it just comes along with w/e other chanenls you were asking for
20:03:55 <t-bast> exactly you can opt-in to receive the node_ann with the channel_udpate
20:04:01 <roasbeef> how do y'all handle reconcilling the feature bits inteh invoice w/ the node ann in practice today?
20:04:06 <t-bast> but right now you'd have to refetch it even if it hasn't changed
20:04:07 <roasbeef> like if they're conflicting somehow
20:04:34 <roasbeef> example being the node ann of the dest doesn't signal var onion, but the invoice doe
20:04:36 <t-bast> for your own feature bits or when reading some else's?
20:04:37 <roasbeef> s
20:04:52 <t-bast> ok, when paying invoice takes precedence
20:04:54 <roasbeef> so you're about to send out, and need to know how to pack the onion
20:05:00 <roasbeef> gotcha, I think we do the same
20:05:32 <t-bast> #action sstone and t-bast to post to the mailing list and work on a draft PR before next meeting
20:05:43 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/pull/697
20:05:56 <t-bast> It looks like we still haven't merged that sphinx fix :)
20:06:06 <t-bast> is it already in lnd and c-lightning's master branches?
20:06:11 <roasbeef> yeh have been meaning to get around to it
20:06:18 <roasbeef> i think CL has merged a fix, but we haven't
20:06:31 <cdecker> It is in c-lightning, but I haven't validated the new test vectors yet
20:06:33 <roasbeef> final thing was if we shoudl jsut use random bytes, or try to extract more bytes from teh existing csprng key stream
20:06:57 <t-bast> yep, I quite like the deterministic approach proposed by cdecker, that's what I currently implemented
20:07:03 <t-bast> but I don't feel very strongly about that
20:07:16 <roasbeef> have been meaning to update to juse use a new key, as the test vectors then can be deterministic
20:07:26 <cdecker> Yeah, the only advantage is the test-vectors are reproducible afaik
20:07:49 <roasbeef> sgtm, I'll update the spec PR
20:07:57 <roasbeef> and then can re-gen the test vectors
20:07:57 <t-bast> I think that's useful for new implementers/newcomers
20:08:06 <t-bast> great thanks!
20:08:07 <roasbeef> yeh one less thing to have to make a decision w.r.t
20:08:20 <t-bast> #action roasbeef update PR to use deterministic derivation
20:08:39 <t-bast> Is there a specific PR you'd like to do next? Or should we stop there?
20:09:16 <t-bast> It would be awesome if everyone could have a quick look outside of the meeting at the PRs marked "discuss at next meeting", most of them can be handled on Github directly to save time
20:10:57 <cdecker> I need to drop off, got some more reviews for a conference to do and the deadline's approaching ^^
20:11:02 * cdecker waves
20:11:06 <t-bast> cdecker: is that for CES?
20:13:30 <roasbeef> #672 can land, it needs to be rebased tho
20:14:01 <roasbeef> i think rusty is out for another meeting or two, so i'll just resolve the conflict and psuh it out
20:14:35 <roasbeef> t-bast: was the issue w/ CL and elcair resolved in #682? the chain innit msgs
20:14:45 <t-bast> thanks roasbeef, sgtm
20:15:16 <t-bast> re #682 CL has fixed that indeed, so it should be good to go too
20:15:36 <cdecker> t-bast: yes
20:16:27 <t-bast> cdecker: I'll very likely attend, see you there! Good luck with the reviews
20:16:55 <cdecker> I'll not be able to attend myself, just doing the work, not getting the upside xD
20:17:20 <t-bast> heh ;)
20:17:45 <t-bast> roasbeef: re 682 do you know if conner or you ack-ed it?
20:22:35 <niftynei> i know we're out of time, but would there be any interest in putting together a small working group for going through 'non-active feature' PR changes? small things like https://github.com/lightningnetwork/lightning-rfc/pull/703/files that are clarification and grammar related
20:23:24 <niftynei> i really like talking grammatical and symmantic minutiae but this meeting doesn't seem like a great forum for those kinds of discussions
20:23:48 <roasbeef> niftynei: I commented on that one, but the OP never replied, current guidelines for like typo/grammar stuff is a main champion for sanity checks
20:24:00 <roasbeef> t-bast: don't think so, we haven't implemented it either, but at a glance looks mostly good to me
20:24:36 <t-bast> niftynei: I agree, I think we should look at those on Github and use the "spelling" label
20:24:58 <t-bast> regarding 703 specifically I think this clarification isn't needed and bloats the spec a bit
20:25:51 <niftynei> agreed. this seems like something you might write in a blog post or 3rd party explainer doc
20:27:00 <t-bast> roasbeef: cool, we can merge it if you want or keep it like this if you plan to implement it soon-ish
20:27:15 <t-bast> roasbeef: let me know what you prefer
20:30:19 <niftynei> "spelling" doesn't seem like exactly the right label for some of them; in some cases it is a subtantive addition or change but won't have an impact on current implementations
20:31:36 <t-bast> true, in that case I think it's worth a brief mention at meeting and review from different people. But I agree it feels like it wastes some meeting time, we should strive to keep it short but that's hard :)
20:32:50 <niftynei> right, it feels like something that needs attention but ultimately would require more time, so another dedicated time to go through them seemed like not a bad idea
20:33:59 <niftynei> anyway, just an idea. ok, i need to go find some lunch
20:34:05 <niftynei> are we wrapping up shortly?
20:34:24 <t-bast> yes, I'm heading off too, let's stop there
20:34:28 <t-bast> #stopmeeting
20:34:33 <t-bast> #endmeeting