19:05:45 <t-bast> #startmeeting
19:05:45 <lightningbot> 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 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:05:56 <t-bast> I just stole the chair
19:06:15 <t-bast> Let's start with two related small ones
19:06:17 <cdecker> No problem, Ikea has cheap ones...
19:06:30 <t-bast> ...
19:06:39 <t-bast> #topic Message extensions
19:06:43 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/705
19:07:06 <t-bast> 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 <t-bast> There are two things to discuss: whether to assign a range like that and whether the range I chose makes sense :)
19:08:02 <rusty> Ack from me, seems like a no-brainer.
19:08:15 <cdecker> Yep, sounds good
19:09:02 <t-bast> Cool, I'll let Joost or Conner chime in for LL
19:09:20 <bitconner> ack from me!
19:09:40 <joostjgr> 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 <bitconner> yes the message types are uint16s
19:10:09 <t-bast> 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 <t-bast> Do you have an idea on how to help people chose?
19:10:29 <rusty> People manage to avoid stepping on each other with TCP port numbers, in practice.
19:10:33 <niftynei> 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 <t-bast> rusty: xD
19:10:51 <joostjgr> yes, maybe just the wording of 'random' could be something that hints to a little more coordination
19:10:51 <t-bast> niftynei: good idea, this can be a Github issue
19:10:58 <t-bast> similar to the feature bit waiting room
19:11:23 <niftynei> sgtm
19:11:32 <t-bast> Allright, thanks for the feedback I'll update that wording and create an issue to track signaling
19:11:34 <cdecker> I like the idea of a Wiki where we track these
19:11:43 <t-bast> #action t-bast update the wording on "random" choice
19:11:54 <cdecker> (an issue is also good, and users could at least comment there)
19:12:02 <t-bast> #action t-bast create a wiki or issue to track used message IDs
19:12:16 <t-bast> Thanks, let's do the second one that's linked to this one now :)
19:12:33 <t-bast> #topic Message TLV extensions
19:12:36 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/714
19:13:02 <cdecker> SGTM
19:13:08 <t-bast> This PR adds to the spec that the ignored data at the end of messages must be a TLV stream
19:13:38 <t-bast> 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 <t-bast> Much nastier than if everyone just uses a TLV stream and the TLV rules
19:13:55 <t-bast> WDYT?
19:15:47 <rusty> Well, this has always been the case.  If you extend things in incompatible ways, you lose.
19:16:06 <bitconner> are we certain all of today's messages readily support this sort of extension?
19:16:32 <roasbeef> stuff like the init message potentially can't
19:16:34 <bitconner> wondering if it has weird interactions with any of the optional fields we've added
19:16:36 <t-bast> Bolt 1 currently states that everyone should ignore the trailing bytes of all Lightning messages
19:16:48 <cdecker> It's a rather nice consolidation imho
19:16:49 <t-bast> init message already can, there's a PR that adds a TLV stream at the end awaiting interop
19:16:53 <roasbeef> there's also an assumption that impls that follow this have already impleemented _all_ the current "optional" fields
19:17:01 <roasbeef> t-bast: not really, it has two variable length fields
19:17:24 <t-bast> roasbeef: that's completely orthogonal to that...?
19:17:24 <rusty> Yeah, there are some optional fields which make this trickier.
19:17:32 <cdecker> But both are length-prefixed roasbeef that's not an issue
19:17:46 <t-bast> True that current optional things may mess this up (maxHtlcAmount for example)
19:17:55 <roasbeef> t-bast: i mean that if the feature bits are large enough, ther's no space left
19:18:00 <cdecker> It'd be nice to consolidate the optional trailing fields into TLVs
19:18:06 <roasbeef> but also see the optional fields thing, until now, none of them have been optional
19:18:12 <t-bast> roasbeef: gotcha, true, you have to deal with only the remaining space left
19:18:17 <roasbeef> so we need to gate the tlv extensions on observance of all the currently optional fields
19:18:20 <cdecker> roasbeef: but that's fully under the sender's control
19:18:22 <bitconner> for example, i think shutdown scripts may have to be mandatory in order to extend open_channel
19:18:35 <roasbeef> as in, if you don't implement upfront shutdown for example, you can't add extensions to the open channel message
19:19:09 <joostjgr> 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 <cdecker> Isn't the presence of the shutdown script gated on a feature bit?
19:19:32 <cdecker> joostjgr: that'd be indeed nice
19:19:34 <roasbeef> cdecker: yes, so if you don't have that set, then you can't set extensions
19:19:34 <t-bast> that's a good point roasbeef, not sure how we should handle priority there
19:19:57 <t-bast> joostjgr: we can potentially do that indeed, I'll think about it
19:20:15 <roasbeef> joostjgr: not really, unless there's a tuneling aspect to it
19:20:32 <rusty> A quick grep indicates that if we assume static_commitment or dataloss_protect and option_upfront_shutdown_script, that's it.
19:20:46 <bitconner> cdecker, just because i say i support it doesn't mean the field is actually present
19:21:23 <cdecker> 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 <rusty> cdecker: naah, just make them compulsory.
19:21:54 <bitconner> i think at this point everyone has implemented those three?
19:21:56 <sstone_> roasbeef: why can't you use extensions on open_channel if the upfront feature bit is not set ?
19:21:58 <cdecker> Also fine by me :-)
19:22:11 <roasbeef> sstone_: will the other party check for the address, or try to parse the tlv extension?
19:22:22 <roasbeef> while decoding the message
19:22:35 <niftynei> 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 <sstone_> roasbeef: parse the tlv extension
19:22:38 <rusty> sstone_: lnd objected to feature-dependent parsing before, which is why the redundant field is left there for option_simplified_commitment.
19:22:52 <t-bast> niftynei: if only you'd done that before we added these optional fields :D
19:22:53 <niftynei> (not really useful for existing msgs tho)
19:23:13 <roasbeef> 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 <roasbeef> it's a side effect of the prior optional fields not actually being optional at all to build on top of
19:23:39 <bitconner> niftynei: perhaps put all fields in tlv to start, optional or otherwise?
19:24:00 <sstone_> roasbeef: ok I see :(
19:24:03 <bitconner> roasbeef: yes i think that's the case
19:24:08 <t-bast> 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 <cdecker> +1 on to the TLV-everything proposal :-)
19:24:17 <bitconner> :D
19:24:20 <niftynei> bitconner, not sure if trolling or... :P
19:24:33 <bitconner> niftynei: not trolling, promise :)
19:24:58 <cdecker> (sounds a lot like the protobuf 2 to protobuf 3 migration btw, where they decided that all fields must be optional)
19:25:09 <t-bast> I can spend some time on all relevant fields and send a recap', and we decide what we do for each
19:25:20 <cdecker> SGTM t-bast
19:25:33 <bitconner> t-bast: sgtm, hopefully there aren't too many instances
19:25:35 <rusty> 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 <t-bast> Allright, I'll do a new pass on that and will update the PR
19:26:04 <t-bast> There's another small point in this PR
19:26:10 <rusty> (dataloss_protect is already compulsory on the network in practice, and everyone support upfront_shutdown too)
19:26:13 <t-bast> Is the stream length-prefixed or not?
19:26:19 <rusty> no.
19:26:31 <t-bast> rusty: perfect, that's what I'm leaning towards too
19:26:44 <cdecker> Let's keep it simple and force TLV extensions only in the future -> no length prefix
19:26:48 <t-bast> especially if we don't need to allow a secondary extension mechanism, I don't see a reason for length prefix
19:27:00 <rusty> 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 <t-bast> bitconner, roasbeef, joostjgr, do you agree with that?
19:27:13 <t-bast> agreed rusty
19:28:08 <roasbeef> if no length, there's a hard conflict with the prior extensions mechanism of "jsut add stuff to the end"
19:28:13 <bitconner> is there a possibility that we mant to not set any of those three in the future?
19:28:23 <bitconner> if so, it would break all extensions
19:28:38 <bitconner> (or at least the ones on the relevant messages)
19:28:51 <t-bast> roasbeef: true, it effectively replaces that by "just add tlv records to the end"
19:29:08 <bitconner> in general tho i like the change
19:29:19 <cdecker> It's a minor restriction, but makes things so much better behaved and less hand-wavy
19:29:34 <joostjgr> "add stuff to the end" could then again be a special field in the tlv stream...
19:29:44 <t-bast> 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 <rusty> TBH, this is what "we can append stuff" was *for*: when we knew how to use it.
19:29:49 <t-bast> joostjgr: exactly!
19:29:51 <roasbeef> 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 <t-bast> that's a good point roasbeef: do we want to have some of us draft a proposal for next meeting?
19:31:32 <t-bast> 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 <t-bast> *proposals (better if many do it in parallel)
19:32:01 <roasbeef> 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 <rusty> 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 <roasbeef> rusty: if we basically have a psuedo-programming-markup langauge in there right now lol
19:32:25 <niftynei> +1
19:33:31 <t-bast> #action t-bast add proposal on how to handle current optional fields (probably make them all mandatory)
19:33:37 <t-bast> #action t-bast remove the length prefix
19:33:46 <niftynei> (to 'versioning by commit/current rev only")
19:33:56 <t-bast> #action all keep thinking about ways to clean-up the spec
19:33:59 <rusty> 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 <cdecker> I'd still like to move to a proposal -> changes -> living document process for the specs
19:34:42 <bitconner> the days of initial_routing_sync are numbered
19:35:03 <roasbeef> cdecker: what's living doc mean?
19:35:08 <t-bast> cdecker: what do you mean?
19:35:13 <roasbeef> isn't tha what we have rn? no strong bounds between "versions"
19:35:17 <cdecker> 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 <roasbeef> also there's a question of bolt extension or new docs all together
19:35:41 <cdecker> Reconstructing the rationale from the git history and the few breadcrumbs left in the various rationale blocks is terribly confusing
19:35:46 <roasbeef> then like stich them all together using some script to make a mega doc
19:35:55 <rusty> bitconner: +1!
19:35:59 <t-bast> 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 <bitconner> cdecker: so new proposals would be their own documents more like bips?
19:37:24 <cdecker> 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 <cdecker> And the bolts would be the living documents (basically a materialization of all the proposals being applied)
19:38:10 <t-bast> I'd like that, especially since we'll be adding more "big" features now that the basics are there
19:38:26 <roasbeef> still leaves the question of how to handle extension of existing bolts, or creating new ones
19:38:32 <rusty> Personally, I like git commit logs for this, but they've got to be more verbose than currently.
19:38:42 <t-bast> For anchor outputs I think it would make a lot of sense to have such a doc for newcomers
19:38:42 <roasbeef> the current cut itself was never really examined for consistency and independence of presentation
19:38:49 <cdecker> 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 <roasbeef> by cut I mean the split of stuff between the docs, we kinda just all ran w/ it
19:39:10 <t-bast> roasbeef: that's true, some of them are kinda big now
19:39:15 <cdecker> rusty: you can't have a discussion in git commits
19:39:33 <cdecker> roasbeef: exactly
19:39:36 <rusty> cdecker: no, but the final commit msg which goes into master should have it embedded IMHO.
19:39:52 <cdecker> Right, that can certainly be done
19:40:47 <bitconner> t-bast: should we continue with another pr? :)
19:40:57 <roasbeef> ok for the sake of time, let's cap the meta discussion to move on? great points all around tho
19:40:57 <cdecker> Anyway, that's for the overall process if we are changing it up. Didn't want to sidetrack the discussion :-)
19:40:58 <t-bast> yep, great discussion let's keep going
19:41:08 <joostjgr> t-bast, what is your time line for the irc part and the hangouts parts of this meeting?
19:41:31 <rusty> (Looking fwd to snapshots of IRC discussions in future spec commit msgs....)
19:41:32 <t-bast> Is there another "smallish" PR some of you want to go over before we move to anchor outputs?
19:41:55 <t-bast> 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 <bitconner> 641?
19:42:02 <rusty> 711?
19:42:09 <cdecker> I think #697 can be applied (we all fixed it iirc)
19:42:09 <bitconner> is it time to revisit?
19:42:29 <bitconner> +1 for 697
19:42:56 <t-bast> Oh yeah we haven't implemented 697 yet but will be simple
19:43:00 <t-bast> ACK, can be applied
19:43:06 <t-bast> #topic https://github.com/lightningnetwork/lightning-rfc/pull/641
19:43:10 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/641
19:43:38 <t-bast> I think 711 is a no-brainer, please just ACK it people :)
19:43:51 <t-bast> bitconner can you refresh our minds on 641?
19:43:57 <bitconner> sure
19:44:00 <t-bast> and then we move to anchor @joostjgr ;)
19:44:20 <bitconner> atm the spec says we _should_ reply with historical data when people set very wide gossip filters
19:44:22 <cdecker> Ack'd 711 :-)
19:44:40 <bitconner> this change removes that altogher, since it be pretty expensive
19:44:52 <rusty> bitconner: so do we still support for the very first time they set filter?
19:44:52 <bitconner> it forces people to us the queries rather than simply asking for everything
19:45:20 <bitconner> the filter then is still applied to updates that we receive from other peers
19:45:23 <rusty> Because we currently (if we have no gossip) pick a node and ask for filter from 0.
19:45:31 <t-bast> thx, sstone probably has more to say than me on that subject
19:45:56 <bitconner> rusty: and don't use the query mechanism?
19:46:03 <cdecker> Hm, asking for everything sounds legit though
19:46:07 <rusty> bitconner: we'll fall back to it to double-check.
19:46:13 <sstone_> well it lgtm
19:46:26 <bitconner> the whole reason for adding gossip_queries was to avoid the DOS vector of intial_routing_sync
19:46:34 <cdecker> 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 <rusty> bitconner: sure, but if you really *do* want to bootstrap, gossip_queries is way less efficient.
19:47:02 <cdecker> Yes, but in this case it's the recipient (which was getting DoSd) that opts in explicitly
19:47:03 <roasbeef> rusty: it has rate limiting built in tho
19:47:05 <bitconner> idt 5 more control messages really is gonna make a difference
19:47:08 <roasbeef> vs getting a massive dump on the wire
19:47:18 <sstone_> rusty: how much worse ?
19:47:54 <bitconner> 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 <rusty> For us, sending you all the gossip is literally scanning a file and sending it; it's pretty efficient.
19:48:09 <BlueMatt> 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 <roasbeef> rusty: but what about the receiver? with the queries they can rate limit the resposnes they get
19:48:28 <BlueMatt> like, the sender should trivially be able to say "oh, tcp socket full, let me not send this low prio crap"
19:48:52 <cdecker> 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 <cdecker> The sender of the gossip is free to throttle however they like
19:49:15 <sstone_> BlueMatt: yes but it's the same socket for everything
19:49:22 <rusty> 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 <bitconner> 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 <BlueMatt> 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 <rusty> bitconner: well, (recent) c-lightning will only do that if it doesn't have any gossip.
19:50:48 <bitconner> rusty: so that only prevents sending duplicates on the same connection?
19:51:33 <rusty> 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 <bitconner> all this scanning can be eliminated if we wish :)
19:52:12 <BlueMatt> i guess my question is: its a bunch of outbound bandwidth, but does that *matter*
19:52:28 <BlueMatt> like, outgoing bandwidth that doesnt interfere with other operation shouldnt matter
19:52:30 <bitconner> when you're connected to 300 peers, yes
19:52:36 <BlueMatt> why?
19:52:41 <roasbeef> 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 <BlueMatt> bitconner: it shouldnt ever interefere with other stuff, it just may be slow to send to peers?
19:53:23 <BlueMatt> 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 <t-bast> roasbeef: noted
19:53:59 <bitconner> BlueMatt: not saying it does, but it's generally just wasteful
19:54:20 <BlueMatt> sure, definitely should hve nodes prefer to use things that use less bandwidth, but...well, what isnt at this point?
19:54:43 <rusty> 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 <rusty> 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 <bitconner> 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 <t-bast> Let's keep the discussion on Github then and move on to Anchor Outputs TM?
19:58:11 <t-bast> And please have a quick look at 711 which needs some love?
19:58:15 <rusty> 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 <cdecker> 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 <bitconner> rusty: sounds good we can discuss more on the pr :_)
19:59:08 <cdecker> t-bast: yes, let's move on to AO
19:59:08 <rusty> 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 <BlueMatt> 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 <BlueMatt> cause, like, you have to implement it that way to avoid dos'ing yourself anyway
19:59:29 <BlueMatt> and its the same amount of data sent
19:59:30 <t-bast> #action keep discussing on the PR to summarize pros and cons
19:59:43 <t-bast> #topic Anchor Outputs
19:59:46 <t-bast> #link https://github.com/lightningnetwork/lightning-rfc/pull/688
19:59:51 <rusty> BlueMatt: Yeah, we already stream gossip reponses at low prio, though TCP buffering makes that less useful than you'd hope.
19:59:55 <t-bast> #link https://meet.google.com/vyt-qnjg-bnm?pli=1&authuser=0
20:00:16 <t-bast> Please join the hangout link for anchor outputs, I may need to manually accept each one of you
20:00:29 <BlueMatt> can you not make it "public"?
20:00:52 <t-bast> I don't think so...
20:01:28 <cdecker> Ok, just checked, Hangouts on the Air was discontinued, maybe we can setup something on Zoom next time?
20:27:41 <t-bast> #info ACK on the scripts with small changes (CLTV=16)
20:27:58 <t-bast> #info ACK on using a symmetric delay with max (using current negotiation)
20:37:00 <t-bast> #info leaning more towards a constant value for the anchor outputs
20:37:08 <t-bast> Thanks everyone for the progress made!
20:37:10 <t-bast> #endmeeting