20:06:20 <rusty> #startmeeting Not the marketing committee
20:06:20 <lndev-bot`> Meeting started Mon Sep 13 20:06:20 2021 UTC and is due to finish in 60 minutes.  The chair is rusty. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:06:20 <lndev-bot`> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
20:06:20 <lndev-bot`> The meeting name has been set to 'not_the_marketing_committee'
20:06:20 <lndev-bot> Meeting started Mon Sep 13 20:06:20 2021 UTC and is due to finish in 60 minutes.  The chair is rusty. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:06:20 <lndev-bot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
20:06:20 <lndev-bot> The meeting name has been set to 'not_the_marketing_committee'
20:06:22 <niftynei> two bots for the price of one, of this is exciting
20:06:50 <cdecker[m]> Hurray
20:07:09 <rusty> #topic https://github.com/lightningnetwork/lightning-rfc/pull/834
20:07:13 <cdecker[m]> Stopped one of them, I think it was trying to make up for last time
20:07:28 <niftynei> looks like  lndev-botimous prime has been chosen as the survivor lol ;)
20:08:14 <cdecker[m]> Shouldn't matter, logs and agenda will still work
20:08:35 <t-bast> Go #834, go! I think it's high time this gets merged, we've seen it being useful many times already in the real world (just found a c-lightning issue today thanks to it)
20:08:38 <rusty> OK, I can rebase this.  roasbeef mentioned wanting a feature bit, which in this case seems overkill (you can always still send an erorr)
20:08:40 <BlueMatt> I think the only thing left to resolve on warnings is https://github.com/lightningnetwork/lightning-rfc/pull/834#discussion_r702330608 ?
20:09:28 <BlueMatt> oh, wait, no, still need to resolve the removal of the all-0s error message
20:09:35 <BlueMatt> and whether we want to do that
20:10:51 <BlueMatt> thoughts?
20:10:53 <roasbeef> see my comment on that other PR: there was a new change that depended on this new warning message, but in that case how is the peer meant to even know to expect the warning message if there's no feature bit?
20:11:01 <roasbeef> I don't understand this new aversion to feature bits
20:11:13 <rusty> BlueMatt: yes, I believe you're the only one to support it, and I think you fail them one at a time as reestablish is sent,
20:11:14 <roasbeef> unless you always send the warning _and_ error in all scenarios?
20:11:17 <t-bast> I'm very neutral on whether we should keep all-0s error message or not, I don't mind either way
20:11:26 <BlueMatt> the peer cant "expect a warning message", warning messages are required to be "ignored"
20:11:31 <roasbeef> still as is, I don't really see what this PR does
20:11:37 <roasbeef> it's just the old error message w/ a new name
20:11:48 <rusty> BlueMatt: but if they don't set the feature, you can ignore errors more.
20:11:55 <BlueMatt> rusty: ok, let me rephrase, I think we should *add* an all-0s error message :)
20:12:13 <t-bast> roasbeef: it's an odd message, they've been explicitly created to allow optional things that you don't need to support, there's really 0 reason to have a feature bit for warning messages
20:12:14 <rusty> BlueMatt: OK, I disagree.  It's another implementation details which won't be implemented.
20:12:27 <BlueMatt> why wouldn't you implement it?
20:12:38 <BlueMatt> that's a useful way to say "go away, close the channels, I'm not gonna talk to you anymore"
20:12:44 <BlueMatt> without having to have a reconnect loop
20:12:45 <roasbeef> comment re PR: https://github.com/lightningnetwork/lightning-rfc/pull/904#issuecomment-909815114
20:12:56 <rusty> BlueMatt: I haven't yet.  I never send such a thing, and I can't see why I would.
20:13:05 <t-bast> roasbeef: it helped a lot figure out many bugs in the wild already on our side, so I'd say it has already proven its worth as-is
20:13:06 <roasbeef> so rather than use the explicit feature bit dep system we built in, we now rely on implicit TLV deps?
20:13:17 <BlueMatt> roasbeef: I believe your comment there, really, is more about quick close feature bit, less about warning message feature bit
20:13:50 <rusty> I agree with roasbeef, we should use a feature bit.  And if set, that means errors are *errors*, are genuinely unrecoverable, and you should really drop to chain.
20:13:52 <roasbeef> BlueMatt: no it's about both, the quick close depends on the warning message the way it's written, and we have a natural way to indicate dependent protocol features: the feature bit deps
20:14:09 <niftynei> rusty: "close all channels" is probably more useful if there's more than one channel btw peers :P
20:14:12 <BlueMatt> rusty: ah, as a backwards compat for the implementations that always ignored errors?
20:14:17 <rusty> (And we can drop all our "wait, this is a spurious error, reconnec and try again" logic)
20:14:40 <BlueMatt> roasbeef: outside of the scope of quick close, is there ever a reason why you'd seek out a peer that will send warning messages?
20:14:41 <rusty> BlueMatt: yeah, e.g. LND used to send an error if you were too slow to handshake, and we used to close channels...
20:14:46 <t-bast> Agreed with BlueMatt, we can argue about whether quick close needs a feature bits, but warning messages are really optional messages you're free to ignore but that may help you diagnose issues in the wild, and that doesn't need a feature bit at all, that's why we split messages in odd vs even
20:14:52 <roasbeef> t-bast: bugs related to the warning message? or bugs in new feature that assume the warning message?
20:15:10 <BlueMatt> I do think rusty has a point here, though, I would be in favor of a feature bit that means "yes, errors are errors, follow the spec, stop ignoring them"
20:15:15 <t-bast> roasbeef: bugs in new and old features that now include warning messages which help us figure out what's wrong on the other side
20:15:25 <rusty> t-bast: I disagree, I'd really like to get rid of the cases where c-lightning ignores errors because implementations used to send them.
20:15:40 <roasbeef> t-bast: figruing it out is all manual tho right? given you have no way to programmatically handle the new message as it's just a string?
20:15:58 <BlueMatt> rusty: re: all-0s, I guess I'm still vaguely of the thinking that reconnect loops absolutely suck, and we should stop throwing our hands up and assuming they're ok.
20:16:00 <roasbeef> in which cases would the warning message help you uniquely dianose something that the existing error message would't?
20:16:25 <BlueMatt> roasbeef: yes, because unlike error messages, warning messages dont close the channel.
20:16:34 <t-bast> rusty: we still don't ignore errors and don't plan on starting to ignore them
20:16:58 <BlueMatt> t-bast: do y'all implement the all-0s close-all-channels error logic?
20:17:10 <rusty> t-bast: ouch, we had several cases we had to, to avoid forced closing.  Maybe safe to remove them now...
20:17:14 <roasbeef> BlueMatt: re why would seek them out, assuming it has error codes, then yeah as you're able to programtically handle certain denegrate cases, and also know that you have strucutred data in the errors
20:17:28 <t-bast> roasbeef: even manual, it's a net improvement compared to what we had before, that's the best argument I can have for a feature - it solves a problem we actually have, not a theoretical one
20:17:32 <roasbeef> but it's less about seeking them out, and making it explicit that two protocol features are dependent on each other
20:17:50 <roasbeef> this is the PR that adds that dep btw: https://github.com/lightningnetwork/lightning-rfc/pull/904
20:17:54 <BlueMatt> roasbeef: I'd understand if you added a feature bit for when warnings got a programatic handling support, but that's not what we're talking about here
20:18:02 <BlueMatt> roasbeef: the warning messages in this pr *MUST* be ignored
20:18:20 <BlueMatt> roasbeef: we can revisit the feature bit discussion when y'all add the "programatic why we warned" logic in the tlv
20:18:29 <t-bast> BlueMatt: we did have the close-all-channels on all-0s error, but we may have removed it recently, I'll need to check
20:18:42 <roasbeef> BlueMatt: which PR #904?
20:19:08 <BlueMatt> roasbeef: no, 834. 904 would again be a conversation about quick close feature bits, which seems separate from just warning messages?
20:19:09 <roasbeef> yeah we're moving forward w/ the TLVs on our end, likely just in the old error message though, given the warning message is identical
20:19:33 <BlueMatt> roasbeef: I mean error already has programatic handling assigned to it - close the channel.
20:19:36 <niftynei> but it's not identical
20:19:37 <roasbeef> must be ignored? totally confused
20:19:39 <BlueMatt> roasbeef: not sure what else you'd want to add?
20:20:06 <BlueMatt> the current warning message, because it has nothing but a string, must be ignored from a logic perspective. you can go log it, tell the user, whatever, but you can't *do* something
20:20:09 <roasbeef> the close channel clause is essentially deprecated, no one does it
20:20:13 <t-bast> rusty: yeah I think nowadays you shouldn't have too may issues re-activating the logic of force-closing when you receive an error, it's not biting us at all on our node
20:20:24 <BlueMatt> roasbeef: we do it, eclair does it, and c-lightning is planning on doing it, maybe with a feature bit
20:20:29 <BlueMatt> so, no, its not deprecated
20:20:51 <t-bast> rusty: it was an issue at some point and we've had unwanted channel closure, but it's gotten much better in the last 12 months to be honest
20:21:10 <BlueMatt> I've *never* seen an unwanted channel closure as the result of an error message, fwiw :)
20:21:24 <roasbeef> it's ill advised imo, which is why it never really caught on
20:22:06 <rusty> roasbeef: wait, t-bast just said they're force-closing on all errors.  I double-checked c-lightning, we gave up and are ignoring all errors :(  We reconnect though
20:22:07 <t-bast> BlueMatt: you arrived on mainnet at the right time, after the nastyness happened xD
20:22:15 <BlueMatt> t-bast: I'm aware
20:22:27 <BlueMatt> roasbeef: its ill advised to tell your peer that you're broadcasting the local state? that just seems like an incredibly helpful and nice thing to do.
20:22:32 <roasbeef> rusty: so then just eclair force closes?
20:22:47 <t-bast> roasbeef: yes, when we receive an error we do
20:22:48 <roasbeef> BlueMatt: you can do that w/ an error message, but doesn't mean they need to force close
20:22:49 <rusty> roasbeef: and LDK it seems.
20:23:28 <roasbeef> say "I broadcasted btw, it's in the mempool" is totally useful, but rn we don't have a way to allow nodes to properly handle that, since it's just a string
20:23:32 <BlueMatt> roasbeef: sure, I mean they dont have to broadcast, but its a really great way to say "dont try to route over this again, dont expect me to accept anything, you should give up on this channel"
20:23:45 <roasbeef> lnd doesn't watch the mempool though, so we'll only note that once it hits the chain
20:24:02 <roasbeef> yeh that sounds useful, but we don't have a way to do that atm w/ just the string
20:24:03 <BlueMatt> yes, we do have a way to tell a node that, the error message!
20:24:22 <BlueMatt> sending an error message with a channel_id set is *exactly* that, according to the spec, and the way eclair and LDK have implemented it
20:24:33 <BlueMatt> I understand historically that was not practical, but those nodes hardly exist anymore.
20:24:37 <rusty> I think warning is still useful, as there are cases which are recoverable (esp since reconnection does reset some state).
20:24:58 <BlueMatt> now, as an alternative to the current warning message pr, we *could* swap the message types
20:25:02 <rusty> But yes, you have to avoid looping too hard.
20:25:12 <BlueMatt> ie define a new "warning" message, with the type of current error, and then add a *new* error message
20:25:34 <BlueMatt> the new error message would have the semantics of error as-define, but not lnd/c-ln implemented.
20:26:18 <niftynei> do the msg type support that?
20:26:24 <BlueMatt> hmm?
20:26:35 <niftynei> isn't error msg a mandatory msg bit
20:26:50 <BlueMatt> probably, but its not like there are any implementations that *dont* understand it
20:27:03 <BlueMatt> like, yes, we'll redefine it to "you must understand how to deserialize this, but also you must then ignore it"
20:27:04 <niftynei> ah no it's 17
20:27:08 <BlueMatt> oh, even better
20:27:27 <niftynei> that sucks though from an implementation/upgrade standpoint
20:27:47 <BlueMatt> it does
20:28:00 <BlueMatt> but i suggested it as roasbeef seemed insistent on error messages being ignored going forward
20:28:05 <BlueMatt> but now roasbeef stopped responding :/
20:28:15 <rusty> That seems like a backward step.  Here are a short list of where we currently send a warning (and reconnect):
20:28:16 <niftynei> like .. sometimes warning messages are fatal and other times they aren't, depending on what version of CL you're talking to? idk
20:28:18 <roasbeef> no not that they should be ignored
20:28:24 <t-bast> to be fair, I get the feeling that roasbeef you're completely dismissing this PR on principle because you'd like something else (error codes automated *some* scenarios), which feels to me very orthogonal and can be done separately
20:28:36 <roasbeef> they're useful, we've used them to debug invalid sig issues in the past
20:29:03 <roasbeef> no I'm not, I'm just saying I don't find it useful really as is, and we prob won't implement it, ofc ppl can deploy it or w/e
20:29:23 <roasbeef> we're following thru w/ our error codes thing, since it solves a buncha problems for us
20:29:27 <niftynei> but your reason for finding it "unuseful" is that ... you've never enforced the semantics of errors meaning "close the channel"??
20:29:39 <roasbeef> maybe we'll bLIP it, since everyone doesn't think error codes are useful
20:29:54 <t-bast> it's already been useful for us many times in the wild, and feels like a net improvement that requires like 20 lines of code, so I'd really like to see it happen...
20:30:04 <niftynei> i mean honestly, it sounds like LND is in favor of upgrading all their error messages to warnings?
20:30:19 <niftynei> to match your current behavior to the spec
20:30:21 <BlueMatt> roasbeef: sheesh, no one is saying error codes are *not* useful
20:30:28 <BlueMatt> roasbeef: please stop putting words in others' mouths
20:30:32 <roasbeef> sure, seems you've already implemented it, but the lingering question re the dep w/ it and the quick close thing stands: if nodes don't impl warning, they can't do quick close or the other way around?
20:30:37 <rusty> - various experimental things (malformed STFU message, etc).
20:30:37 <rusty> - short_id mismatch in announcement_signatures
20:30:37 <rusty> - almost any malformed message (e.g. update_htlc which we can't decode)
20:30:37 <rusty> - unable to add htlc for any reason
20:30:37 <rusty> - update_fee from non-opener
20:30:38 <rusty> - update_fee unreasonable
20:30:39 <t-bast> I'm interested in the error codes as well, and would love to see a PR for that, but it feels completely additive to warnings to me, if we have both I'd be very happy
20:30:43 <roasbeef> idk that was said pretty plainly in #834
20:30:50 <BlueMatt> roasbeef: people have given you *feedback* on error codes, and suggested its a great thing to add as a tlv in a warning message, adding new logic to how you have to handle them
20:31:20 <roasbeef> maybe we should move onto something else?
20:31:24 <BlueMatt> roasbeef: and also said that warnings are useful on their own, and error codes are a nice extension, thus we should move forward with warnings then add stuff on top
20:31:27 <roasbeef> this has taken up a lot of time
20:31:28 <t-bast> roasbeef: of course they can do quick close without it, and that's a completely separate discussion...?
20:31:38 <BlueMatt> I'm not really sure how you have so misinterpreted the feedback given there.
20:31:45 <roasbeef> t-bast: but then #904 would need to be modified right? since it says send a warning if the quick close fails?
20:32:04 <t-bast> roasbeef: no, it's just that it's *nicer* with a warning, it's a small improvement
20:32:20 <BlueMatt> roasbeef: did you see my comment at https://github.com/lightningnetwork/lightning-rfc/pull/904#issuecomment-911982144 ?
20:32:20 <t-bast> roasbeef: doesn't *require* it at all
20:32:25 <BlueMatt> I tried to explain it a bit more verbosely
20:33:32 <rusty> OK, I think we're in circles now.
20:33:34 <rusty> #topic https://github.com/lightningnetwork/lightning-rfc/pull/906
20:33:50 <roasbeef> updated to bit 44/45
20:34:19 <rusty> I agree with roasbeef here, but I think the right semantic is "if you don't negotiate this, don't quickclose, if you do, you must"
20:34:35 <BlueMatt> rusty: this isnt about quickclose, this is channel type?
20:34:39 <rusty> Oops.
20:34:43 <t-bast> xD
20:34:58 <roasbeef> quick close was merged iirc
20:35:05 <roasbeef> this is chan type feature bit
20:35:20 <t-bast> I agree that the argument stands for both, you can argue that channel_type needs a feature bit, and you can use the same argument to say that quick close does as well
20:35:26 <rusty> I agree with roasbeef here, but I think the right semantic is "if you don't negotiate this, ignore channel_type and use default, if you do, you must send and use it"
20:35:37 <BlueMatt> feature bit for channel type seems like it would make the logic simpler, but I dunno if I get to have an opinion cause I haven't implemented it yet :)
20:36:21 <t-bast> TBH if you feel strongly about having a feature bit here, I don't mind. If we do, I agree with rusty that the condition must be updated
20:36:24 <rusty> (I actually think quickclose is a stronger candidate for feature bit, but that's separate)
20:36:26 <roasbeef> the way I see it, for features like this, we all prob have a config flag or build flag to be able to test versions of the software w/ and w/o it in our integration tests right? so this is just another conjunction there
20:36:36 <BlueMatt> tho i guess i dunno i dont care either way, and maybe it is more complicated? whatever.
20:37:08 <BlueMatt> roasbeef: no, we dont for most of this stuff, we just edit the message being delivered in testing to simulate older peers
20:37:21 <roasbeef> I think this whole saga has shown also we don't really have a strict criteria of when feature bit or not, maybe has been muddied by newer TLV fields in messages
20:37:22 <t-bast> That's really not an area I think is worth fighting for on my end, I don't think it solves what you think it does (because you can't just make it mandatory when you want to migrate to it because you have connected peers with channels that still may not support it)
20:37:24 <BlueMatt> so thats probably some of why I see these flags as spurious in some cases
20:37:26 <roasbeef> which is where some of this ambiguitiy is
20:37:39 <t-bast> But if it solves an issue for you, I'm happing to add a feature bit
20:37:42 <roasbeef> BlueMatt: do you then remove the older logic from the client?
20:37:42 <rusty> roasbeef: yeah, basically we would always send it, we'd just on recv do: if (negotiated(option_channel_feature)) { if (!tlvs->channel_type) fail } else { tlvs->channel_type = NULL' }
20:38:11 <BlueMatt> roasbeef: no, but we can simulate talking to an old peer whether there is a feature bit or not. so we can test old logic just fine with or without a feature bit
20:38:32 <BlueMatt> actually, we can test *easier* without a feature bit
20:38:37 <BlueMatt> one less thing to deal with
20:38:59 <roasbeef> you do the same even for stuff like ensuring you can open non-anchor channels after that's default?
20:39:03 <BlueMatt> of course my or your particular test infrastructure isn't a great reason to have a feature bit or not, imo, but whatever
20:39:03 <rusty> One nice feature of a feature bit is that when you make it compulsory your logic gets simpler.
20:39:20 <roasbeef> BlueMatt: not saying that should be the reason, but was a respone to "it's an extra if statement"
20:39:25 <roasbeef> rusty: +1
20:39:37 <t-bast> rusty: it's true that you can remove some code, but only if you also remove the option for your users to switch from mandatory to optional...
20:39:50 <rusty> t-bast: eventually, yeah.
20:39:57 <t-bast> rusty: we haven't dared do that yet (even though we'd love to start doing it soonish)
20:40:15 <BlueMatt> roasbeef: yea, again, all of our tests deal with unencrypted messages to be delivered, so we can do ~whatever we want in our protocol tests.
20:40:22 <roasbeef> we tried to do it for static remote key, but that didn't really work, hence the need for the chan type stuff in the first place
20:40:55 <rusty> Yeah, lnprototest tests this now (it assumes you allow downgrade, which we explicitly will not once static_remotekey becomes compulsory)
20:41:17 <roasbeef> BlueMatt: ah interesting, but was referring to stuff more like how the chain  handling conditions change for a peer due to diff chan types, stuff like the csv delay as an example changes, but maybe we just have really diff test set ups
20:41:30 <roasbeef> or like being able to parse the old onion format
20:41:42 <rusty> roasbeef: yes!!  Can we just kill legacy already?
20:41:48 <t-bast> roasbeef: how do you plan on using a channel_type feature bit if you have channels with a peer that doesn't support it (when you want to start making it mandatory)?
20:42:11 <roasbeef> rusty: legacy as in non static key? yes pls, I think the eclair version that supports static key is out now?
20:42:24 <crypt-iq> i think he meant legacy onion
20:42:35 <t-bast> roasbeef: as usual, not eclair, eclair-mobile only doesn't support it
20:42:39 <rusty> Anyway, I think there's agreement on this, but I want to reformulate it to:
20:42:40 <rusty> Sender: - if negotiated:
20:42:40 <rusty> - MUST set channel_type.
20:42:40 <rusty> ... Receiver:
20:42:40 <rusty> - if negotiated:
20:42:40 <rusty> - if channel_type not set, MUST fail
20:42:43 <BlueMatt> roasbeef: yea, if we force the messages to indicate old channel type, then we're testing an old channel type and then can test all the transactions against that :) I think we do have just very fundamentally different tests as a library that exposes messages vs a whole software program :)
20:43:17 <rusty> Happy to do that on-issue though.  Shall we move on?
20:43:20 <roasbeef> t-bast: so i think we'd need to continue to sidestep the issue w/ setting required bits for chan types w/ the old peer, but once they signalled this bit it wouldn't be an issue
20:43:25 <BlueMatt> rusty: yes, lets move on
20:43:25 <roasbeef> rusty: sgtm
20:43:46 <roasbeef> the feature bit name spacing we have rn is a lot better than the OG, but there're still some leaks like that imo t-bast
20:44:17 <t-bast> roasbeef: but if they never do? You'll keep accepting channels that dont use the channel type mechanism...so you'll want to eventually kick them out by making it mandatory and closing channels with them? Otherwise they'll keep opening new channels that don't use the feature
20:44:25 <rusty> #topic https://github.com/lightningnetwork/lightning-rfc/pull/894 and 905
20:44:50 <roasbeef> t-bast: yeah, so we'd need to reject them, or purposefully downgrade like we do now w/ older eclair nodes -- would rather not downgrade, so we'd prob jsut reject them all
20:45:10 <BlueMatt> roasbeef: hwats your thoughts on the options at https://github.com/lightningnetwork/lightning-rfc/issues/905 ?
20:45:32 <t-bast> roasbeef: ok, I don't really know how to handle the "now we close all channels with a peer because they don't support one of our mandatory features" and communicate it to node operators
20:45:34 <rusty> Pick a number, please, and I'll implement it :)  I admit I've been absent on this issue
20:45:37 <BlueMatt> specifically option 2, which t-bast and I suggested, and which I've implemented
20:46:31 <t-bast> Oh yeah for this dust limit thing, please spend a bit of time reading the issue, and be convinced with option 2 that basically says that while in practice option 1 is cleaner, it's really a lot of unnecessary hassle :D
20:46:44 <roasbeef> t-bast: not saying close, we just wouldn't let them open any new ones, the way I see it tho, all new types would just use this and we can even add an explicit feature bit dep on it
20:47:02 <roasbeef> I think we have an impl of option 1 w/o the bit, as it was just teasing out the changes somewhat
20:47:19 <roasbeef> 2 is def easier...and I guess theo nly ppl on the blast radius are those that used a non-segwit addr for upfront shutdown
20:47:29 <t-bast> I just wanted to raise awareness on this, and on the fact that I think #894 can be merged as-is independently of #905 (which we can argue on the issue directly, and I'll open a PR once I've gathered enough feedback / rough consensus)
20:47:35 <BlueMatt> right, that plus happen to close with a very low value
20:47:43 <BlueMatt> *and* have a low dust limit, which i think basically no nodes use today
20:47:51 <t-bast> roasbeef: ok, that's a good alternative, I'll think about it
20:48:07 <BlueMatt> so I'm not sure there's *anyone* in the blast radius for #2, actually
20:48:10 <rusty> Yeah, let's just ban non-sw closes, and grandfather in non-sw for upfront, iff dust limit high enough?
20:48:23 <crypt-iq> i think it may be good to explicitly state in the spec that both channel reserves need to be above both dust limits, which i dont think is explicit
20:48:31 <BlueMatt> rusty: that's basically what option 2 is, except with not just dust limit, but only the payout value at close
20:48:38 <rusty> Yeah, I'm not sure anyone will notice.  For us, if we do this and you ry to shutdown to non-sw, we'll warning you and hangup, then you can try again
20:48:46 <t-bast> BlueMatt: I tested on our node, we do have people closing to non-segwit, but none ever ran into that case where it would have lead to a force-close. So I feel pretty safe with option #2
20:48:55 <rusty> BlueMatt: ack, I misspoke.
20:49:18 <BlueMatt> t-bast: well, my point here is also that no one currently *sets* a dust limit too low for non-segwit closes
20:49:35 <BlueMatt> t-bast: in order for a node to get a force-close here they have to (a) have a dust limit at 330, and (b) use non-segwit close
20:49:40 <BlueMatt> or 354 i guess
20:49:41 <t-bast> BlueMatt: yes, but I tested even if they did, we never had a shutdown that would have run into this :)
20:49:48 <BlueMatt> (b) is true, but (a) I think basically isnt
20:50:01 <BlueMatt> right, and (c) actually close with a <540 sats output
20:50:05 <BlueMatt> so its ~nothing imo
20:50:28 <t-bast> Great, so if there's rough consensus about option 2, I'll open a PR for that
20:50:38 <t-bast> #action t-bast to open a PR for option 2 of issue 905
20:50:51 <roasbeef> jeremy was right, dust sucks, we should just elminiate it
20:50:52 <roasbeef> kek
20:50:58 <rusty> roasbeef: lol
20:51:00 <BlueMatt> lol
20:51:02 <t-bast> Or should I do it in a new commit in #894? Or merge #894 as-is?
20:51:06 <t-bast> roasbeef: xD
20:51:18 <ariard> i think #894 is mergable?
20:51:26 <BlueMatt> if we move forward with #2, I think we should mention in 894 that the dust limit is 354
20:51:37 <rusty> Yep...
20:51:41 <BlueMatt> i guess i dont care if its a followup
20:51:44 <BlueMatt> but it seems a waste
20:51:53 <BlueMatt> I'll leave it up to the author, the changes themselves there are fine
20:51:53 <roasbeef> 894 mentions 354 iirc
20:51:57 <niftynei> is it 354 or 330?
20:52:03 <roasbeef> fine w/ 894 as is
20:52:12 <BlueMatt> roasbeef: it does not say that the dust limit that nodes *MUST* check for is 354
20:52:16 <roasbeef> ahh good ol dust arithmetic
20:52:16 <BlueMatt> it mentions it is a dust limit
20:52:27 <BlueMatt> niftynei: 354 is anysegwit, 330 is segwit-taproot/v0
20:52:33 <niftynei> oic you're using the highest of segwit dust options, gotcha
20:52:44 <rusty> #topic https://github.com/lightningnetwork/lightning-rfc/pull/895
20:52:45 <t-bast> niftynei: 330 is for p2wsh but 354 is the max dust limit for all segwit versions, so it's a safe way to encompass all segwit
20:52:59 <niftynei> ty for the explanation t-bast!
20:53:30 <rusty> roasbeef: the self-assignment of fake ids, I don't think it's a problem.  I set my own aliases, so I make sure they don't clash.
20:53:35 <BlueMatt> rusty: you opened a replacement for this today, no?
20:53:51 <t-bast> I'll add a commit on #894, no need to rush merging that PR
20:54:07 <BlueMatt> https://github.com/lightningnetwork/lightning-rfc/pull/910
20:54:52 <roasbeef> rusty: yeh you can set your down, but unless I'm missing something, a collision can still happen which makes how you act based on info the onion ambiguous right? Alice and Bob both say their scid is 10, you get an HTLC from dave that wants to route thru scid 10, what do you do?
20:55:16 <rusty> roasbeef: you tell Alice and Bob what their scids are.
20:55:20 <roasbeef> if we say, you gotta reject any collisions (you force a conneciton recycle?) then it works
20:55:33 <BlueMatt> roasbeef: the scid aliases are unidirectional
20:55:34 <rusty> roasbeef: they tell you what they will use for HTLCs coming *from them*, sure.
20:55:36 <roasbeef> rusty: only if you're the funder of both?
20:55:46 <rusty> roasbeef: both sides give *different* aliases.
20:56:15 <cdecker[m]> Or we could make them deterministic, i.e., last 8 bytes of the funding txid
20:56:18 <BlueMatt> B tells A what scids A can put in invoices that B will use to forward from C. B cannot put that same scid in their own invoices as A only uses it for inbound from B, not relaying to
20:56:40 <rusty> You only need to know what scid they'll use when writing routehints for bolt 11.
20:57:13 <rusty> I tell Alice to use 10, Bob to use 11.
20:57:21 <roasbeef> ah ok, I had the like responsibilities flipped
20:57:30 <roasbeef> make sense now...I think
20:57:54 <rusty> Yeah... so BlueMatt wanted this to be a universal alias mechanism, not just a zeroconf open.  That's kinda different enough that I opened a competing PR.
20:57:54 <roasbeef> ok now 895 vs 910?
20:58:00 <t-bast> What exactly is the difference between the two PRs? Only the explicit mention of using the channel for the `push_msat` amount?
20:58:19 <roasbeef> t-bast: I think 910 carves out the scid thing for more general usage?
20:58:27 <BlueMatt> rusty: I have a hard time implementing it as *not* a universal alias mechanism
20:58:30 <t-bast> ok, interesting
20:58:45 <BlueMatt> like, I dont think you can implement it without it accidentally being one, so might as well make it explicit
20:58:46 <roasbeef> t-bast: so like the thing where ppl can use random stuff in the hop hints and not give away on-chain info
20:58:48 <BlueMatt> and its kinda cool to have anyway
20:58:50 <rusty> t-bast: 910 means you *always* get given an alias if you support this, even if you have a public channel, even if you don't intend to use it before conf.
20:59:14 <t-bast> Great, that sounds cool
20:59:51 <crypt-iq> now you have to maintain a mapping of all scids for a channel -> channel?
20:59:52 <rusty> i.e. you "funding locked" (now a misnomer, but whatev) immediately with some alias value.  You probably still fail incoming payments until confirmed, unless you trust.
21:00:02 <t-bast> I agree with BlueMatt on his comment about the `next_commitment_point` though, it can be misleading: if you send `funding_locked` at any time, what value do you put in here? Especially if you may send `funding_locked` in parallel to commitment updates?
21:00:05 <rusty> crypt-iq: yes, another alias.
21:00:08 <BlueMatt> crypt-iq: the sender has to maintain all the mappings they sent. the recipient only has to maintain one.
21:00:41 <BlueMatt> t-bast: its made a bit clearer in the latest pr, but the text as-is in the spec i think is clear enough, as long as we all agree
21:00:42 <rusty> t-bast: yeah, we could rename that field to commitment_point_1 or something.
21:00:57 <rusty> And agreed, it's redundant, but it's also simple.
21:01:00 <BlueMatt> it says "MUST set `next_per_commitment_point` to the per-commitment point to be used for the following commitment transaction, derived as specified in"
21:01:04 <BlueMatt> which is reasonably clear
21:01:07 <BlueMatt> I think
21:01:11 <BlueMatt> but worth confirming/pointing out
21:01:13 <crypt-iq> the short channel id field is 8 bytes long, chance of collision is 2^32 ?
21:01:27 <t-bast> rusty: ok, if it's the first commitment_point then that's all good
21:01:31 <roasbeef> woah, so 901 lets you just continue to send a new funding_locked to update the alias?
21:01:42 <roasbeef> like after normal chan operation has started?
21:01:56 <rusty> roasbeef: yeah, BlueMatt wanted that.  I won't be doing that, since you gotta respect all aliases ou gave out, forever.
21:02:05 <t-bast> I think what rusty and BlueMatt are saying for the commitment_point here is contradictory? Or am I understanding it wrong?
21:02:06 <rusty> And ofc I can just keep using the first alias.
21:02:26 <BlueMatt> t-bast: heh, yes, it seems we are now.
21:02:35 <t-bast> Rusty you're saying it should always be your initial commitment_point, BlueMatt is arguing the PR says it's the next one (which may be commitment_point N)?
21:02:44 <rusty> BlueMatt: it's no longer clear if you have sent commitment_signed already, which was previously not possible.
21:02:51 <BlueMatt> t-bast: to be fair, it doesnt matter which, we just gotta pick :)
21:03:02 <t-bast> I think always sending the initial one here is the simplest, as you don't actually use it (except for the first `funding_locked` you receive)
21:03:03 <BlueMatt> I'll let the author of the pr select one and clarify it in the pR :)
21:03:09 <rusty> t-bast: exactly.
21:03:17 <t-bast> ACK then
21:03:42 <t-bast> It needs to be clarified in the PR though, it's easy to misinterpret
21:03:59 <rusty> t-bast: agreed, good catch BlueMatt
21:05:00 <rusty> #action Rusty to fix PR to clarify next_per_commitment_point is #1 in #910
21:05:28 <rusty> OK, so is 910 the winner over the "just for trusted zeroconf" 895?
21:05:43 <BlueMatt> after thinking about it more, I dont feel super strongly, fwiw, about the ability to send the message over and over with new aliases, I think its cool and we might as well, cause the cost from 1 to many is low and there are use-cases for having many, but I do think we should include the text about always sending an alias cause that's important
21:06:11 <BlueMatt> the temporal shift over time of the scid alias i think is less useful than just having a pool, but in any case all of those details are up to the sender + recipient, the spec is generic
21:06:25 <t-bast> At first glance, it does look like a winner IMO. pm47 will review both on our end, I'll ask him to chime in this week.
21:06:51 <rusty> Yeah, I am tempted to upgrade the "- SHOULD initially set `alias` to value not related to the real `short_channel_id`." to "- MUST initially set `alias` to value not related to the real `short_channel_id`."
21:06:56 <roasbeef> yeh seems to work, can also ask the breez ppl to take a look at it, but I think it lines up w/ what they already do (they can keep their current scid generation scheme)
21:07:26 <t-bast> good idea to have breez take a look as well, the more reviews the merrier
21:07:51 <niftynei> does adding aliases here mean that there's other proposals for aliases that are now superceded?
21:08:11 <rusty> Oops, we need a feature bit for this!
21:08:16 <t-bast> Are there other proposals for aliases?
21:08:21 <BlueMatt> yes, feature bit for this is useful :)
21:08:43 <rusty> *If* you undersand this, we need to *not* route via the real scid for private channels, otherwise probing is still a thing.
21:08:49 <roasbeef> t-bast: iirc y'all had an older one a few years ago?
21:09:05 <rusty> (And if you don't, we need to, since you don't know about the alias)
21:09:18 <niftynei> yeah this feels similar to something else but i cant remember exactly. it's not important
21:09:20 <t-bast> roasbeef: on the contrary, I fought against rusty's alias proposal and instead proposed route blinding, which I don't think clashes with this proposal :)
21:09:21 <BlueMatt> rusty: good point, yea
21:09:24 <mschmoock> rusty: +1
21:09:30 <roasbeef> t-bast: ah yeh route blinding
21:09:52 <rusty> Yeah, routeblinding is a superset, but more ambitious.  THis is a minfix.
21:10:47 <rusty> #action rusty update #910 to add feature bit, make routing via non-announcable real scids invalid if negotiated.
21:11:27 <rusty> Oh, that means it's a channel_type, since we gotta remember whether we negotiated that bit at open...
21:11:28 <BlueMatt> so we're way over time, are there any short things to tackle?
21:11:46 <mschmoock> mission creep :D
21:12:11 <rusty> https://github.com/lightningnetwork/lightning-rfc/pull/898 ?  I think it just needs acks?
21:12:16 <rusty> #topic https://github.com/lightningnetwork/lightning-rfc/pull/898
21:12:40 <BlueMatt> I've acked, we merged the chnages already
21:13:14 <rusty> #action rusty to validate https://github.com/lightningnetwork/lightning-rfc/pull/898
21:13:37 <t-bast> Thanks! And we should be done with Bolt 11 test vectors for a small while
21:13:39 <rusty> Actually, with BlueMatt and t-bast, that's enough to merge.
21:13:47 <t-bast> yay!
21:13:59 <rusty> I rpropose we merge, and stragglers like me can catch up...
21:14:07 <rusty> Any objectsion?
21:14:08 <t-bast> But if you plan on validating the test vectors, no need to rush we can wait to see if you find an issue we missed
21:14:17 <roasbeef> test vectors good
21:14:18 <t-bast> Whatever you'd like
21:14:29 <rusty> #action t-bast to merge https://github.com/lightningnetwork/lightning-rfc/pull/898
21:14:41 <rusty> OK, I think we're done!
21:14:49 <t-bast> Neat, thanks guys!
21:14:57 <rusty> #endmeeting