19:05:45 #startmeeting 19:05:45 Meeting started Mon Dec 9 19:05:45 2019 UTC. The chair is t-bast. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:05:45 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:05:56 I just stole the chair 19:06:15 Let's start with two related small ones 19:06:17 No problem, Ikea has cheap ones... 19:06:30 ... 19:06:39 #topic Message extensions 19:06:43 #link https://github.com/lightningnetwork/lightning-rfc/pull/705 19:07:06 This is a proposal similar to what we did with onion TLV, to create a "safe range" of message types for experiments 19:07:39 There are two things to discuss: whether to assign a range like that and whether the range I chose makes sense :) 19:08:02 Ack from me, seems like a no-brainer. 19:08:15 Yep, sounds good 19:09:02 Cool, I'll let Joost or Conner chime in for LL 19:09:20 ack from me! 19:09:40 i am not so familiar with this area. 65535 is the max I assume? Maybe random is not enough to prevent collisions? 19:10:07 yes the message types are uint16s 19:10:09 Exactly, 65535 is the max. Random may not be enough indeed to avoid collisions, but I don't know if we can do much better 19:10:27 Do you have an idea on how to help people chose? 19:10:29 People manage to avoid stepping on each other with TCP port numbers, in practice. 19:10:33 it might be nice to have a wiki page where devs can 'signal intent' to use a type number, but i don't think that needs to go in the spec 19:10:38 rusty: xD 19:10:51 yes, maybe just the wording of 'random' could be something that hints to a little more coordination 19:10:51 niftynei: good idea, this can be a Github issue 19:10:58 similar to the feature bit waiting room 19:11:23 sgtm 19:11:32 Allright, thanks for the feedback I'll update that wording and create an issue to track signaling 19:11:34 I like the idea of a Wiki where we track these 19:11:43 #action t-bast update the wording on "random" choice 19:11:54 (an issue is also good, and users could at least comment there) 19:12:02 #action t-bast create a wiki or issue to track used message IDs 19:12:16 Thanks, let's do the second one that's linked to this one now :) 19:12:33 #topic Message TLV extensions 19:12:36 #link https://github.com/lightningnetwork/lightning-rfc/pull/714 19:13:02 SGTM 19:13:08 This PR adds to the spec that the ignored data at the end of messages must be a TLV stream 19:13:38 The main reason I'd like to get that in sooner than later is that I realized that if we let some people use a TLV stream there and others use something else, this might lead to nasty conflicts 19:13:50 Much nastier than if everyone just uses a TLV stream and the TLV rules 19:13:55 WDYT? 19:15:47 Well, this has always been the case. If you extend things in incompatible ways, you lose. 19:16:06 are we certain all of today's messages readily support this sort of extension? 19:16:32 stuff like the init message potentially can't 19:16:34 wondering if it has weird interactions with any of the optional fields we've added 19:16:36 Bolt 1 currently states that everyone should ignore the trailing bytes of all Lightning messages 19:16:48 It's a rather nice consolidation imho 19:16:49 init message already can, there's a PR that adds a TLV stream at the end awaiting interop 19:16:53 there's also an assumption that impls that follow this have already impleemented _all_ the current "optional" fields 19:17:01 t-bast: not really, it has two variable length fields 19:17:24 roasbeef: that's completely orthogonal to that...? 19:17:24 Yeah, there are some optional fields which make this trickier. 19:17:32 But both are length-prefixed roasbeef that's not an issue 19:17:46 True that current optional things may mess this up (maxHtlcAmount for example) 19:17:55 t-bast: i mean that if the feature bits are large enough, ther's no space left 19:18:00 It'd be nice to consolidate the optional trailing fields into TLVs 19:18:06 but also see the optional fields thing, until now, none of them have been optional 19:18:12 roasbeef: gotcha, true, you have to deal with only the remaining space left 19:18:17 so we need to gate the tlv extensions on observance of all the currently optional fields 19:18:20 roasbeef: but that's fully under the sender's control 19:18:22 for example, i think shutdown scripts may have to be mandatory in order to extend open_channel 19:18:35 as in, if you don't implement upfront shutdown for example, you can't add extensions to the open channel message 19:19:09 standardize on tlv for extensions sgtm. does this also translate cleanly to extending the htlc failure message with tlv? (realizing this is not exactly the scope of this pr) 19:19:12 Isn't the presence of the shutdown script gated on a feature bit? 19:19:32 joostjgr: that'd be indeed nice 19:19:34 cdecker: yes, so if you don't have that set, then you can't set extensions 19:19:34 that's a good point roasbeef, not sure how we should handle priority there 19:19:57 joostjgr: we can potentially do that indeed, I'll think about it 19:20:15 joostjgr: not really, unless there's a tuneling aspect to it 19:20:32 A quick grep indicates that if we assume static_commitment or dataloss_protect and option_upfront_shutdown_script, that's it. 19:20:46 cdecker, just because i say i support it doesn't mean the field is actually present 19:21:23 Right, how about we just re-bundle those fields into a TVL stream then? Might be painful for a while, but once we are upgraded we can simplify massively 19:21:38 cdecker: naah, just make them compulsory. 19:21:54 i think at this point everyone has implemented those three? 19:21:56 roasbeef: why can't you use extensions on open_channel if the upfront feature bit is not set ? 19:21:58 Also fine by me :-) 19:22:11 sstone_: will the other party check for the address, or try to parse the tlv extension? 19:22:22 while decoding the message 19:22:35 total aside, but the v2 of open_channel that i've been working on does exactly this (moves the optional fields into a tlv) 19:22:36 roasbeef: parse the tlv extension 19:22:38 sstone_: lnd objected to feature-dependent parsing before, which is why the redundant field is left there for option_simplified_commitment. 19:22:52 niftynei: if only you'd done that before we added these optional fields :D 19:22:53 (not really useful for existing msgs tho) 19:23:13 sstone_: why? it's possible to set the bit (optional), but then not set any addr at all, however I signal that I can parse yours to extract the addr 19:23:33 it's a side effect of the prior optional fields not actually being optional at all to build on top of 19:23:39 niftynei: perhaps put all fields in tlv to start, optional or otherwise? 19:24:00 roasbeef: ok I see :( 19:24:03 roasbeef: yes i think that's the case 19:24:08 Do you think we should do a full pass on all those optional fields and decide on a case-by-case whether they should become mandatory or be inside an extension stream? 19:24:11 +1 on to the TLV-everything proposal :-) 19:24:17 :D 19:24:20 bitconner, not sure if trolling or... :P 19:24:33 niftynei: not trolling, promise :) 19:24:58 (sounds a lot like the protobuf 2 to protobuf 3 migration btw, where they decided that all fields must be optional) 19:25:09 I can spend some time on all relevant fields and send a recap', and we decide what we do for each 19:25:20 SGTM t-bast 19:25:33 t-bast: sgtm, hopefully there aren't too many instances 19:25:35 t-bast: there are only a few fields, see above. We should simply make them all compulsory and move on, making all extra data TLV. 19:25:57 Allright, I'll do a new pass on that and will update the PR 19:26:04 There's another small point in this PR 19:26:10 (dataloss_protect is already compulsory on the network in practice, and everyone support upfront_shutdown too) 19:26:13 Is the stream length-prefixed or not? 19:26:19 no. 19:26:31 rusty: perfect, that's what I'm leaning towards too 19:26:44 Let's keep it simple and force TLV extensions only in the future -> no length prefix 19:26:48 especially if we don't need to allow a secondary extension mechanism, I don't see a reason for length prefix 19:27:00 At some point you have to fish or cut bait. We can always renumber the messages if we want Fancy New Format. 19:27:06 bitconner, roasbeef, joostjgr, do you agree with that? 19:27:13 agreed rusty 19:28:08 if no length, there's a hard conflict with the prior extensions mechanism of "jsut add stuff to the end" 19:28:13 is there a possibility that we mant to not set any of those three in the future? 19:28:23 if so, it would break all extensions 19:28:38 (or at least the ones on the relevant messages) 19:28:51 roasbeef: true, it effectively replaces that by "just add tlv records to the end" 19:29:08 in general tho i like the change 19:29:19 It's a minor restriction, but makes things so much better behaved and less hand-wavy 19:29:34 "add stuff to the end" could then again be a special field in the tlv stream... 19:29:44 bitconner: I'll try to summarize for each of those the reasons why we may want to turn them off in the future and we'll see 19:29:48 TBH, this is what "we can append stuff" was *for*: when we knew how to use it. 19:29:49 joostjgr: exactly! 19:29:51 was brought up last time, but I think we need to step back and think about how we extend the spec and make it more readable in light of the steady changes, so basically just making git tags to snapshot things, with the current feature bits basically being implictly assumed for each version, and points to prior versions for existing behavior 19:30:59 that's a good point roasbeef: do we want to have some of us draft a proposal for next meeting? 19:31:32 I think that's how it would be most efficient: let some of us prepare a proposal we can chew on before discussing it next time 19:31:52 *proposals (better if many do it in parallel) 19:32:01 not sure if it requires immediate action, just something we should be cognizant over, it really became apparent to me when reviewing the anchor outputs pr 19:32:01 I agree with roasbeef: once a feature is made compulsory, all mention of it should be cleaned up as if it were always compulsory. 19:32:21 rusty: if we basically have a psuedo-programming-markup langauge in there right now lol 19:32:25 +1 19:33:31 #action t-bast add proposal on how to handle current optional fields (probably make them all mandatory) 19:33:37 #action t-bast remove the length prefix 19:33:46 (to 'versioning by commit/current rev only") 19:33:56 #action all keep thinking about ways to clean-up the spec 19:33:59 roasbeef: yeah, specs kinda always get this way unless you remove obsolete support. But I agree at some point you go "look at the git history for reasons", and some features are no-brainers. 19:34:38 I'd still like to move to a proposal -> changes -> living document process for the specs 19:34:42 the days of initial_routing_sync are numbered 19:35:03 cdecker: what's living doc mean? 19:35:08 cdecker: what do you mean? 19:35:13 isn't tha what we have rn? no strong bounds between "versions" 19:35:17 Each change that is substantial needs a document accompanying it that explains the rationale in a single place, and then has pointers into the spec where changes were applied 19:35:32 also there's a question of bolt extension or new docs all together 19:35:41 Reconstructing the rationale from the git history and the few breadcrumbs left in the various rationale blocks is terribly confusing 19:35:46 then like stich them all together using some script to make a mega doc 19:35:55 bitconner: +1! 19:35:59 For significant changes I agree: we're missing having the rationale high-level view in a single place -> maybe we should add wiki pages for that 19:36:13 cdecker: so new proposals would be their own documents more like bips? 19:37:24 Yes, but once the logic of the change has been debated we can then apply changes to the bolts, and link the rationale in, so that there is a doc to read the big-picture of why certain things were done 19:37:53 And the bolts would be the living documents (basically a materialization of all the proposals being applied) 19:38:10 I'd like that, especially since we'll be adding more "big" features now that the basics are there 19:38:26 still leaves the question of how to handle extension of existing bolts, or creating new ones 19:38:32 Personally, I like git commit logs for this, but they've got to be more verbose than currently. 19:38:42 For anchor outputs I think it would make a lot of sense to have such a doc for newcomers 19:38:42 the current cut itself was never really examined for consistency and independence of presentation 19:38:49 Just bolting on additional bullets to the rationale sections in the bolts without ever giving the bigger pictures is really confusing (at least to me) 19:38:55 by cut I mean the split of stuff between the docs, we kinda just all ran w/ it 19:39:10 roasbeef: that's true, some of them are kinda big now 19:39:15 rusty: you can't have a discussion in git commits 19:39:33 roasbeef: exactly 19:39:36 cdecker: no, but the final commit msg which goes into master should have it embedded IMHO. 19:39:52 Right, that can certainly be done 19:40:47 t-bast: should we continue with another pr? :) 19:40:57 ok for the sake of time, let's cap the meta discussion to move on? great points all around tho 19:40:57 Anyway, that's for the overall process if we are changing it up. Didn't want to sidetrack the discussion :-) 19:40:58 yep, great discussion let's keep going 19:41:08 t-bast, what is your time line for the irc part and the hangouts parts of this meeting? 19:41:31 (Looking fwd to snapshots of IRC discussions in future spec commit msgs....) 19:41:32 Is there another "smallish" PR some of you want to go over before we move to anchor outputs? 19:41:55 There are a bunch of trivial PRs over there, but maybe each of us can go through them outside of the meeting for the sake of time? 19:41:58 641? 19:42:02 711? 19:42:09 I think #697 can be applied (we all fixed it iirc) 19:42:09 is it time to revisit? 19:42:29 +1 for 697 19:42:56 Oh yeah we haven't implemented 697 yet but will be simple 19:43:00 ACK, can be applied 19:43:06 #topic https://github.com/lightningnetwork/lightning-rfc/pull/641 19:43:10 #link https://github.com/lightningnetwork/lightning-rfc/pull/641 19:43:38 I think 711 is a no-brainer, please just ACK it people :) 19:43:51 bitconner can you refresh our minds on 641? 19:43:57 sure 19:44:00 and then we move to anchor @joostjgr ;) 19:44:20 atm the spec says we _should_ reply with historical data when people set very wide gossip filters 19:44:22 Ack'd 711 :-) 19:44:40 this change removes that altogher, since it be pretty expensive 19:44:52 bitconner: so do we still support for the very first time they set filter? 19:44:52 it forces people to us the queries rather than simply asking for everything 19:45:20 the filter then is still applied to updates that we receive from other peers 19:45:23 Because we currently (if we have no gossip) pick a node and ask for filter from 0. 19:45:31 thx, sstone probably has more to say than me on that subject 19:45:56 rusty: and don't use the query mechanism? 19:46:03 Hm, asking for everything sounds legit though 19:46:07 bitconner: we'll fall back to it to double-check. 19:46:13 well it lgtm 19:46:26 the whole reason for adding gossip_queries was to avoid the DOS vector of intial_routing_sync 19:46:34 If the peer really needs all the gossip why not allow it to just query it this way rather than mandating multiple roundtrips? 19:46:48 bitconner: sure, but if you really *do* want to bootstrap, gossip_queries is way less efficient. 19:47:02 Yes, but in this case it's the recipient (which was getting DoSd) that opts in explicitly 19:47:03 rusty: it has rate limiting built in tho 19:47:05 idt 5 more control messages really is gonna make a difference 19:47:08 vs getting a massive dump on the wire 19:47:18 rusty: how much worse ? 19:47:54 again, we wanted to remove the inefficient sync method by adding gossip_queries. keeping this behavior just degrades it back to what we had before 19:47:58 For us, sending you all the gossip is literally scanning a file and sending it; it's pretty efficient. 19:48:09 the idea that dos should be reduced by telling the sender to slow down in-protocol seems to have missed...the entire point of tcp? 19:48:17 rusty: but what about the receiver? with the queries they can rate limit the resposnes they get 19:48:28 like, the sender should trivially be able to say "oh, tcp socket full, let me not send this low prio crap" 19:48:52 Yes, that's the point, if a node asks for everything with a really large timestamp range, they explicitly opt into getting a lot of messages 19:49:03 The sender of the gossip is free to throttle however they like 19:49:15 BlueMatt: yes but it's the same socket for everything 19:49:22 bitconner: there's an intermediate step here, where we should not go backwards: i.e. if you didn't specify a filter before, sure, we'll look from the start, but if you specify a new one we're going to send yo histortical records. 19:49:44 i'm more concerned with the seender side, this is the biggest allocation of bandwidth i see on my node by far 19:49:54 sstone_: sure, but sending different messages in different order is trivial. its not like you can load the whole thing into a buffer the second you connect anyway (if you do that on the sender side you will oom yourself) 19:50:23 bitconner: well, (recent) c-lightning will only do that if it doesn't have any gossip. 19:50:48 rusty: so that only prevents sending duplicates on the same connection? 19:51:33 bitconner: yeah, but in our implementation we scan all the gossip again if you send a new filter, which is kinda sucky, 19:52:10 all this scanning can be eliminated if we wish :) 19:52:12 i guess my question is: its a bunch of outbound bandwidth, but does that *matter* 19:52:28 like, outgoing bandwidth that doesnt interfere with other operation shouldnt matter 19:52:30 when you're connected to 300 peers, yes 19:52:36 why? 19:52:41 I need to go in 10 mins, so will say this on anchor outputs: would be nice if we can at least agree on scripts so if to use the miniscript optimizations or not (which change the witness), as that would let impls move forward. On the topic of the anchor output size, I'm not sure why we need strict conformance given that since the sender pays for them, they only need to sure the responder's values aren't below dust and "too high" (so a cap on size based 19:52:58 bitconner: it shouldnt ever interefere with other stuff, it just may be slow to send to peers? 19:53:23 and if its the case that you dos yourself by sending lots of data, gossip_queries doesnt fix that, it just makes the attacker have to work a tiny bit harder 19:53:57 * niftynei is a under the weather today, so is going to log off a bit early. looking forward to watching the anchor outputs recording later :wave: 19:53:58 roasbeef: noted 19:53:59 BlueMatt: not saying it does, but it's generally just wasteful 19:54:20 sure, definitely should hve nodes prefer to use things that use less bandwidth, but...well, what isnt at this point? 19:54:43 bitconner: so at startup we pick one peer and ask for everything from some starttime, which defaults to (last-changed-time-of-gossip-store minus 60 seconds) or 0 if we have no gossip at all. 19:55:57 That seems pretty optimal if you restart your node. We don't have a timestamp filter for other queries, so I'm not sure how to replace this other than by asking for, everything? 19:57:31 gotcha, i thought cl had switch entirely to gossip queries and not rely on the full dump at all. in that case this change could still be premature 19:57:58 Let's keep the discussion on Github then and move on to Anchor Outputs TM? 19:58:11 And please have a quick look at 711 which needs some love? 19:58:15 bitconner: This is the only case left: seemed a good fit. The rest of the time we got for "stream on" or "stream off". I'm happy to change, just got to figure out what we shuld do. 19:58:18 Well, we could always chunk the "dump everything on me" query into smaller ranges and incrementally ask for them, but it'd still be the same end-result with added latency for issuing the queries 19:58:54 rusty: sounds good we can discuss more on the pr :_) 19:59:08 t-bast: yes, let's move on to AO 19:59:08 cdecker: yeah, you need to query by scid range than ask about those scids. But that doesn't work for "I've been down 1 hour, what happened?' 19:59:10 I mean responses to a full sync query might as well already be the same as that, just done on the sending side 19:59:21 cause, like, you have to implement it that way to avoid dos'ing yourself anyway 19:59:29 and its the same amount of data sent 19:59:30 #action keep discussing on the PR to summarize pros and cons 19:59:43 #topic Anchor Outputs 19:59:46 #link https://github.com/lightningnetwork/lightning-rfc/pull/688 19:59:51 BlueMatt: Yeah, we already stream gossip reponses at low prio, though TCP buffering makes that less useful than you'd hope. 19:59:55 #link https://meet.google.com/vyt-qnjg-bnm?pli=1&authuser=0 20:00:16 Please join the hangout link for anchor outputs, I may need to manually accept each one of you 20:00:29 can you not make it "public"? 20:00:52 I don't think so... 20:01:28 Ok, just checked, Hangouts on the Air was discontinued, maybe we can setup something on Zoom next time? 20:27:41 #info ACK on the scripts with small changes (CLTV=16) 20:27:58 #info ACK on using a symmetric delay with max (using current negotiation) 20:37:00 #info leaning more towards a constant value for the anchor outputs 20:37:08 Thanks everyone for the progress made! 20:37:10 #endmeeting