19:18:37 #startmeeting 19:18:37 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 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:18:54 #topic https://github.com/lightningnetwork/lightning-rfc/pull/719 19:19:11 This is a PR to clarify dependencies between features 19:19:35 #info Happy new year 2020, the year of the Lightening :-) 19:19:36 It also caught the fact that we left out var_onion_option from the dependencies of most new features 19:20:04 t-bast: some ppl had issues scanning invoices made by pheonix from lnd 19:20:19 iirc it was that they didn't specify the new onion format, but specified mpp 19:20:24 roasbeef: yep because we didn't include the var_onion_option bit because the spec explicitly left it out 19:20:38 currently the spec said that it was a feature bit that we could not put in invoices 19:20:51 we should fix the spec, which both conner and bluematt did in their PRs :) 19:21:11 BTW is lnd ready for complete compat' testing of MPP? 19:21:27 yep master can recv, 0.9 (coming soon) will be able to recv as well 19:21:47 you can send on teh RPC level, but only with multuiple nodes paying a single invoice, so like crowd funding style 19:22:01 great, I'll start doing some tests against master this week then! 19:22:03 hi all 19:22:11 hi Joost 19:22:14 what we can't do is sending multiple parts from a single node 19:22:32 good to know, thanks, I'll make sure I used two nodes 19:22:36 it is possible to send multiple parts, but 'crowd sourced' from multiple instances 19:22:42 so let's roll in #723 into #719? 19:23:13 #723 has another point about feature handling, but we can definitely merge those two if we ACK them all 19:23:23 fine by me. if you just take the second commit the first comit is redundant with 719 19:23:31 #719 needs an ACK from cdecker or niftynei 19:23:40 #723 needs an ack from someone LL-side 19:24:18 BlueMatt or ariard, do you have comments on #719? 19:24:25 I think we are already compliant with #719 19:24:42 719 lgtm 19:25:04 cdecker: well you also have to reject on the receive side....like, reject the message 19:25:30 and disconnect them 19:25:45 actually, 719 should specify what you should do 19:25:57 it just says "MUST validate" but that doesnt mean anything 19:26:10 actually on #719 it seems like conner made a mistake, and added `9` to `gossip_queries` instead of `var_onion_optin` 19:26:31 Yes, I don't see any prescribed action for recipients in there 19:27:16 You're right t-bast allowing gossip signalling in the invoice is not correct imho 19:27:41 alright, so 719 clearly needs work, lets boot conversation to github. 19:27:44 Then let's let conner fix that and add some requirements 19:27:54 Let's move to 723 which we can probably merge 19:28:11 #action Fix a few comments on #719 before merge 19:28:18 #topic https://github.com/lightningnetwork/lightning-rfc/pull/723 19:28:45 a strict definition of transitive feature dependency could be laid out before the "MUST set all transitive feature dependencies" tho 19:29:06 ariard: #719 attempts to do that 19:29:19 723 lgtm, other than the fact that "should in general override" is too weak of language imo 19:29:45 niftynei: that's a non-normative note for future spec changes 19:29:57 hence it being in the "Rationale" section. 19:30:11 roasbeef: that just a nit for clarity but #719 tells you what are actual dependencies but not formal definition 19:30:36 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 btw I'm removing the meeting discussion tags as we address them, so the issues don't accumulate over time 19:31:00 niftynei: hm? it being in the Rationale section is a note *for us* for future changes. not for implementors 19:31:20 ariard: def of transitive deps? what's the advantage over just enumerating them all? 19:31:33 cdecker: thanks! 19:31:45 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 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 Rationale is not the place to put "strict" things, cause they have no strict meaning 19:33:01 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 right, my preference is that the rationale is more opinionated, which you can do by removing the 'in general' :P 19:33:50 roasbeef: bolt clarity but nevermind 19:34:01 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 Either make the point weaker ("notice that contradictions may arise") or make it normative 19:34:49 ok, done, removed "in general" 19:35:01 awesome. thanks 19:35:09 SGTM 19:35:26 ACK 19:35:42 roasbeef, joost, you ok with merging #723? 19:36:11 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 roasbeef: its like 100 chars different, you can read it quick :p 19:36:32 It's quite a small one honestly 19:36:35 i think we already use the onion bit in the invoice 19:36:51 t-bast: last I heard we only require two implementation acks to merge something anyway :p 19:37:10 Heh ;) 19:37:46 Well, there's no hurry is there? We can count ACKs on the issue, so why not let Conner weigh in? 19:38:19 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 whatever. 19:38:30 SGTM, let's merge this once conner acks 19:38:42 #action ping conner for ACK then merge 19:39:13 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 Definitely, I know I could do a better job working through the tiny/uncontroversial stuff before the meeting 19:40:31 Let's move to two issues that a bit of discussion 19:40:35 #topic https://github.com/lightningnetwork/lightning-rfc/issues/721 19:40:54 This is a remark on a potential attack on MPP that's not explicitly handled by the spec 19:41:24 I think we could fix this with a small spec change, are implementations vulnerable to that today? 19:41:29 imho this is pure receiver policy and shouldn't normative 19:42:34 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 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 it's also related to the min htlc value on the final channel 19:43:14 right, this can be handled with a min htlc value/max_htlcs_in_flight 19:43:19 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 ultimately its the same risk number 19:43:30 It's true that it's mostly an issue for big nodes with many channels 19:44:03 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 err MPs 19:44:53 IIUC your overall feeling is that this isn't a big concern in practice? 19:45:01 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 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 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 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 Yep, same here 19:46:18 sounds like we're all on the same page, yea. 19:46:25 gotcha 19:46:29 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 somewhat tangentially, it may make sense to have a general limit just to, eg, restrict things to sanity 19:46:50 let's move on then, I'll summarize those points on the issue for reference 19:47:09 #topic https://github.com/lightningnetwork/lightning-rfc/issues/724 19:47:09 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 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 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 but yeh this is relevant, node ann's don't propagate super well rn 19:48:42 seems this is solved by reviving the inv proposal 19:48:54 which would need a way to fetch a node ann in isolation 19:48:59 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 That's not necessarily very easy to do with INV either... 19:49:38 Long term may be using minisketch to reconcile node_announcements 19:49:43 t-bast: no nothing in particular, just ref'ing that 19:49:56 t-bast: why isn't it easy to do with an inv? 19:50:08 you don't want _all_ the node anns, only those that you may care about or frequently use also 19:50:22 roasbeef: I don't think that node inv will let you quickly reconcile your node anns 19:50:32 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 sstone: it depends on what the goal is 19:50:52 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 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 possibly 19:52:49 I see INV as a way to reduce broadcast bandwidth, but we still need queries 19:52:52 feature filter seems desirable 19:53:16 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 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 See you BlueMatt :-) 19:55:08 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 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 we could extend the existing extended queries to add node_announcement and filters 19:56:02 if people feel that it's useful we can work on that 19:56:26 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 we'll call them super-extended-queries 19:56:48 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 it seems that there's a general case about being able to query for 'newer gossip' via timestamps? 19:58:05 niftynei: yes. we can already do that for updates but not node anns because it was not needed until now 19:58:11 there's that, then also possibly being able to filter based on feature bits 19:58:26 main thing is that node anns weren't very important til we started to actually use the global feature bits namespace 19:58:34 did anyone explore using minisketch with multiple reconciliation sets per-feature or object of interest? 19:58:41 yep, that sounds about right 19:58:55 i know rusty has been working on a minisketch compaction/filter for gossip 19:59:08 ariard: i think rusty looked into basic integration, but idk feels like too big of a hammer for this simple case 19:59:19 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 (i might be using the wrong term here, i'm not totally clear on what minisketch is composed of) 20:00:10 it's not trivial to correctly use minisketch for lightning - done naively it ends up being worse than IBLT 20:00:35 definitely doable, but more long-term 20:00:38 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 great we'll learn a lot from that 20:01:07 it sounds like a extending query extensions is a reasonable move 20:01:12 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 My proposal is that we could start drafting something to add filters to queries and node_announcement capabilities 20:01:28 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 sstone and I will work on that for next meeting, how does that sound? 20:01:42 right right, but having already a minisketch production library is cool too 20:01:49 sounds great t-bast 20:02:36 roasbeef: there's already *some* node_anns capabilities, but not enough :) 20:02:38 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 sgtm. you can already query node anns but it useful only for channels you had not seen yet 20:02:53 what's missin is feature bit filtering? 20:03:05 roasbeef: good idea, we'll start with the mailing list 20:03:18 not only, right now you can't know if a node_ann has changed compared to what you have 20:03:26 we need to use checksum or something else 20:03:34 gotcha, it just comes along with w/e other chanenls you were asking for 20:03:55 exactly you can opt-in to receive the node_ann with the channel_udpate 20:04:01 how do y'all handle reconcilling the feature bits inteh invoice w/ the node ann in practice today? 20:04:06 but right now you'd have to refetch it even if it hasn't changed 20:04:07 like if they're conflicting somehow 20:04:34 example being the node ann of the dest doesn't signal var onion, but the invoice doe 20:04:36 for your own feature bits or when reading some else's? 20:04:37 s 20:04:52 ok, when paying invoice takes precedence 20:04:54 so you're about to send out, and need to know how to pack the onion 20:05:00 gotcha, I think we do the same 20:05:32 #action sstone and t-bast to post to the mailing list and work on a draft PR before next meeting 20:05:43 #topic https://github.com/lightningnetwork/lightning-rfc/pull/697 20:05:56 It looks like we still haven't merged that sphinx fix :) 20:06:06 is it already in lnd and c-lightning's master branches? 20:06:11 yeh have been meaning to get around to it 20:06:18 i think CL has merged a fix, but we haven't 20:06:31 It is in c-lightning, but I haven't validated the new test vectors yet 20:06:33 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 yep, I quite like the deterministic approach proposed by cdecker, that's what I currently implemented 20:07:03 but I don't feel very strongly about that 20:07:16 have been meaning to update to juse use a new key, as the test vectors then can be deterministic 20:07:26 Yeah, the only advantage is the test-vectors are reproducible afaik 20:07:49 sgtm, I'll update the spec PR 20:07:57 and then can re-gen the test vectors 20:07:57 I think that's useful for new implementers/newcomers 20:08:06 great thanks! 20:08:07 yeh one less thing to have to make a decision w.r.t 20:08:20 #action roasbeef update PR to use deterministic derivation 20:08:39 Is there a specific PR you'd like to do next? Or should we stop there? 20:09:16 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 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 cdecker: is that for CES? 20:13:30 #672 can land, it needs to be rebased tho 20:14:01 i think rusty is out for another meeting or two, so i'll just resolve the conflict and psuh it out 20:14:35 t-bast: was the issue w/ CL and elcair resolved in #682? the chain innit msgs 20:14:45 thanks roasbeef, sgtm 20:15:16 re #682 CL has fixed that indeed, so it should be good to go too 20:15:36 t-bast: yes 20:16:27 cdecker: I'll very likely attend, see you there! Good luck with the reviews 20:16:55 I'll not be able to attend myself, just doing the work, not getting the upside xD 20:17:20 heh ;) 20:17:45 roasbeef: re 682 do you know if conner or you ack-ed it? 20:22:35 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 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 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 t-bast: don't think so, we haven't implemented it either, but at a glance looks mostly good to me 20:24:36 niftynei: I agree, I think we should look at those on Github and use the "spelling" label 20:24:58 regarding 703 specifically I think this clarification isn't needed and bloats the spec a bit 20:25:51 agreed. this seems like something you might write in a blog post or 3rd party explainer doc 20:27:00 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 roasbeef: let me know what you prefer 20:30:19 "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 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 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 anyway, just an idea. ok, i need to go find some lunch 20:34:05 are we wrapping up shortly? 20:34:24 yes, I'm heading off too, let's stop there 20:34:28 #stopmeeting 20:34:33 #endmeeting