Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lightning Specification Meeting 2021/02/01 #838

Closed
6 of 21 tasks
t-bast opened this issue Jan 27, 2021 · 1 comment
Closed
6 of 21 tasks

Lightning Specification Meeting 2021/02/01 #838

t-bast opened this issue Jan 27, 2021 · 1 comment

Comments

@t-bast
Copy link
Collaborator

t-bast commented Jan 27, 2021

The meeting will take place on Monday 2021/02/01 at 7pm UTC on IRC #lightning-dev. It is open to the public.

Pending from last meetings

Pull Request Review

Issues

Long Term Updates

Backlog

The following are topics that we should discuss at some point, so if we have time to discuss them great, otherwise they slip to the next meeting.


Post-Meeting notes:

Action items

@t-bast t-bast pinned this issue Jan 27, 2021
@t-bast
Copy link
Collaborator Author

t-bast commented Feb 2, 2021

<jkczyz> #startmeeting
<cdecker> Nope doesn't work, I'll set one up for next time
<jkczyz> will just use them for ease of parsing then
<jkczyz> #topic rustyrussell to validate #539 test vectors in c-lightning
<cdecker> rusty isn't joining today, so let's skip that one
<jkczyz> ok!
<jkczyz> #topic: all implementations to correctly set sync_complete as per #826
<jkczyz> How's the progress on this?
<roasbeef> lnd already does it, and has this the start 
<t-bast> On that front, I have a PR for eclair, it just depends on another PR related to gossip queries so it's not integrated yet, but will be done soon
<roasbeef> as the prior usage of this field wasn't really used, and didn't add much 
<ariard> still on the todo list on the RL side
<roasbeef> since the start*
<roasbeef> we just need to update newer nodes to start reading the value again
<jkczyz> cdecker: How about c-lightning?
<jkczyz> going to presume cdecker stepped away. Shall we move on to the next topic or is there more to discuss here?
<cdecker> Had to check
<jkczyz> ah, sorry for the rush :)
<cdecker> c-lightning still uses the "are we queriable for all gossip" meaning. Will fix that up these days
<cdecker> Sorry for the delau
<cdecker> s/delau/delay/
<jkczyz> ok, let's move on then
<jkczyz> #topic second implementation of #672 for compatibility testing with c-lightning
<jkczyz> This is for BOLT 2 opt_shutdown_anysegwit
<jkczyz> Looks like cdecker and roasbeef were looking into this for their impls?
* ThomasV (~ThomasV@unaffiliated/thomasv) has joined
<t-bast> No progress on this one for eclair, we'll do this at some point but not sure when
<cdecker> Should be really trivial to implement
<roasbeef> we accept it already, just need to add the feature bit 
<cdecker> Excellent 👍
<jkczyz> should we add an action item to follow-up? or is there a better way to go about completing this?
<t-bast> I think we can keep this item in the pending list until we've had cross-compatibility between implementations tested?
<ariard> just to test interops CL/lnd ?
<t-bast> Once we do we can merge the PR
<jkczyz> #action follow up
<jkczyz> let's move on then
<jkczyz> #topic second implementation of #824 for compatibility testing with lnd
<jkczyz> defining option_zero_htlc_tx_fee
<jkczyz> ACKs from ariard and t-bast 
<jkczyz> Any implementations have this ready for iterop testing?
<t-bast> Not yet for eclair, it should be ready in a few weeks (hopefully)
<cdecker> I see the definition of option_anchors has been added, very good 👍
<ariard> once I'm done with LDK sample, should be easy to tweak my anchor implem to test interops
<roasbeef> lnd is ready at 0.12, you toggle it w/ a flag --protocol.anchors 
<ariard> but same few weeks
<jkczyz> great, let's follow-up on progress next meeting then
<cdecker> cl doesn't have a nursery yet, so we'll need to implement that first before we can implement `option_anchors_zero_fee_htlc_tx`
<jkczyz> cdecker: nursery?
<cdecker> Tracking (partial) transactions, recombining them when needed, and bumping fees if needed
<jkczyz> ah, thanks for explaining
<roasbeef> yeh we've been working on our version of that in lnd for a while, it's pretty advanced now and the API is just "yo sweep this, lemmie bump it, dont' include this one w/ that one, etc" and it batches+aggregates+republishes in the background 
<ariard> cdecker: think about reorg or remote side confirming first and thus unlocking bumping utxo
<cdecker> Yep, that's pretty much what we aspire to as well
<roasbeef> yeh we watch for all spends, and then update what we're doing accordingly 
<roasbeef> negative confs, etc 
<cdecker> That's not a problem ariard, we support reorgs and races, just not re-arranging transactions, or bumping fees
<jkczyz> ok, I move that we go onto pending PR reviews unless there is more to discuss here
<jkczyz> #action follow up
<jkczyz> #topic Compression algorithm gossip #825
<jkczyz> seems like most are happy with the state of this?
<t-bast> I think only RL commented on this PR, right?
<t-bast> It would be worth having another implementation look into it IMO
<jkczyz> t-bast: ah right! you were the one who proposed it :P
<t-bast> :D
<jkczyz> cdecker roasbeef, any input from your sides?
<roasbeef> on the lnd side we've been interested in something like this for sometime now, but haven't impl'd it yet 
<roasbeef> what happens if there's a non overlap? 
<roasbeef> so you have the bit set, they have the bit set, but you both want diff algos 
<roasbeef> so intersection of algos is the empty set 
<cdecker> Sounds good to me, the only point of contention was how we can signal we don't support zlib, and in that case we just kill the connection if the peer asks for it and we don't
<cdecker> roasbeef: empty intersection = no gossip exchange, easy
<t-bast> exactly
<bitconner> one q i had: how do you indicate support for both raw _and_ zlib if there's only a bit assigned to zlib?
<bitconner> or is raw just assumed?
<t-bast> cdecker: now you can signal you don't support zlib (if you have that feature bit activated and in `init` don't send support for zlib)
<jkczyz> t-bast: no lingua franca
<t-bast> bitconner: yes I assumed raw
<bitconner> does it not make sense to have the option to disable raw? 
<cdecker> t-bast: I know, but nodes that don't understand the feature bit will still ask for it
<t-bast> cdecker: yes that's right, hopefully this will be temporary and people will upgrade soon-ish because of all the other goodies that are coming
<t-bast> bitconner: maybe, it could be reasonable to want to only allow compressed if you're worried about bandwidth
<t-bast> bitconner: I could use one of these bits to indicate raw, what do the others think?
<cdecker> Nah, raw MUST be supported imho
<bitconner> i think that makes sense, since we are trying to rectify the mistake of assuming some default encoding support. if we're implementing encoding negotiation imo it makes sense to have all encoding treated quially
<bitconner> equally*
<roasbeef> oh so you're saying uncompressed isn't explicit? 
<roasbeef> ahh I see what you're saying, being able to signal support for both in an explicit manner? 
<bitconner> yeah, raw isn't opt-in
<t-bast> cdecker: why do you think it can't be opt-out?
<t-bast> to ensure we have something that allows all nodes to communicate?
<t-bast> and avoid splitting based on incompatible encoding support?
<t-bast> (splitting the network)
<cdecker> That for one, but it is also trivial to opt out if you don't want to support it: don't ask for it, and if asked return an empty one
<bitconner> if you don't want to split, then you'd signal both? but if you are actually constrained then idk if it makes sense to force the fallback
<cdecker> But I don't really care that much, it's just weird to have raw as a "compression algorithm" :-)
<bitconner> cdecker: that would break some assumptions, bc the node is execpting to get all the updates
<roasbeef> no compression is best compression 
<bitconner> maybe encoding algorithm would be more appropriate :P
<roasbeef> ^
<bitconner> you're telling others that "i know nothing" when in reality you just don't want to tell them 
<t-bast> TBH there are pros and cons for both, I'm not sure which solution I'd prefer
<t-bast> No opt-out of uncompressed format ensures there's always an intersection between two nodes (a way to exchange gossip queries) which is good
<roasbeef> yep
<roasbeef> so bit 0 is uncompressed, bit 1 is zlib?
<cdecker> Ok
<cdecker> Let's make it explicit then :-0
<cdecker> :-)
<t-bast> okay let's do this
<cdecker> Gotta run in a couple of minutes btw
<t-bast> #action t-bast to add uncompressed as an explicit option
<bitconner> yay cipher suite negotiation 😂
<roasbeef> ok just commented thaton the spec 
<t-bast> :+1:
<jkczyz> great, let's move on
<jkczyz> #topic Warning message #834
<t-bast> If rusty isn't there, maybe we'll wait for him before we discuss this warning message since he created the PR?
<jkczyz> yeah, that seems reasonable given the discussion on the PR
<cdecker> Sure that works as well, he should be here for the extended discussion
<jkczyz> #action follow-up when rusty is here
<cdecker> But we think this is really an important improvement, since it makes soft failures explicit (the ones that don't panic and go to chain)
* jb55 (~jb55@gateway/tor-sasl/jb55) has joined
<cdecker> lnd had a couple of them, and we needed to work around them by special casing, so making this explicit removes that under-definedness from the spec
<t-bast> I agree, it's desirable
<cdecker> Implementing it is really just adding the message, and then going through all the error cases and deciding whether to kill or not the underlying channel and switching to the appropriate message variant
<t-bast> that sounds really boring though xD
<t-bast> but it's quite useful
<ariard> we already have soft-errors on the RL-side, good move
<roasbeef> yeh next step is to add in error codes so we can all handle them more programmatically 
<roasbeef> then the payloads can have more structure as well 
<roasbeef> like "error code 5 is invalid sig, there're 3 tlv of the past state, htlcs, etc" 
<BlueMatt> what can an implementation do with any of that data?
<roasbeef> debug? 
<ariard> apart of helping debug
<roasbeef> just an example 
<BlueMatt> if we add explicit error codes, we should have clear definitions of exactly what an implementation should do in response to each
<BlueMatt> otherwise it can just be a string.
<roasbeef> other stuff is when you reject an incoming funding req, you can tell them what your actual min amt is, etc 
<ariard> but yeah that sounds more like dyna m mic negotation
<roasbeef> string parsing isn't really helpful as ppl then all have diff strings 
<roasbeef> this is just abotu structure errors for debugging and also to give more insight into disconnects, funding rejections, etc 
<BlueMatt> right, my point is if you want to impose the effort of something more than strings, there needs to be a clear "if you get error code 1, you should do X"
<roasbeef> like rn, we prob all send diff errors when we recv an invalid commitment sig or htlc sig 
<BlueMatt> otherwise its all debugging anyway and debugging requires humans which can use strings.
<roasbeef> might as well have better/standardized input to your debugging, but that's just one class of errors 
<ariard> we might inject structure and then let implementation experiment a bit with it
<roasbeef> yeh low cost, tangible benefit 
<ariard> I def can see errorr codes about funding rejecct being consumed to do a step-by-step negotation
<roasbeef> ppl are free to structure their errors or not lol 
<ariard> where only one side adjust its opening offer
<BlueMatt> if 'what you do in response' is loose you just end up with a bunch of random return values which are buggy and dont line up with exactly what a counterparty should do.
<ariard> I mean something cross-implem
<BlueMatt> I'm not saying it has to be super well-defined, but lets not add some loose categorization of errors which dont mean anything cause we have humans processing it anyway
<roasbeef> I think we're talking past each other here, there're diff classes of errors and hard to argue that we couldn't benefit from them being more structured, up to the impl what they do w/ that structure, certain errors can be programatically handled w/ more context 
<BlueMatt> that just adds complexity and a ton of "wait, which category is this?" for nothing.
<cdecker> No, I think we shouldn't impose structure on the error message, since by definition they cannot be enumerated, and therefore we'll always have some that are undefined
<roasbeef> if you think it's complex, ok don't impl it 
<BlueMatt> what cdecker said
<roasbeef>  cdecker: a pre-req is adding _error codes_ 
<cdecker> We can talk about adding metadata, for specific ones, but not mandate a fix structure
<roasbeef> which allows them to be enumerated 
<BlueMatt> unless there's a structure for exactly how to handle the error code, dont send it
<roasbeef> by structure I mean tlv values here, which are malleable 
<cdecker> Yeah, and 99% will be `ERROR_OTHER` with a string repr :-)
<roasbeef> things aren't that black and white....
<ariard> class of errors are easy, in case of doubt send $UNDEFINED
<roasbeef> idk we've gone thru lnd and have about 15 or so errors we'll start making more defined w/ error codes 
<BlueMatt> cdecker: right, at which point we might as well not add the error flags anyway
<BlueMatt> if there's something we can do with them, then, yea, lets define errors, I'm just saying lets say what you can/may want to do with them when we define the error flags
<bitconner> what's wrong with error code + description?
<t-bast> but shouldn't those well-defined errors instead inspire new dedicated protocol messages instead?
<BlueMatt> bitconner: nothing, as long as what action should be taken on each error code is well-defined
<BlueMatt> we already have a mess of error codes with htlc forwarding
<t-bast> in your example of funding being rejected, it looks to me we should just better define a renegotiation of parameters in that case
<BlueMatt> parsing those things to figure out who is to blame and who blames who and what to do in response is a mess.
<roasbeef> t-bast: lnd in the past just used the first byte for the error code, very easy to re-add that and standardize it, then the payload after that can be a TLV, really easy to roll out 
<bitconner> gotcha, iiuc the goal would be to enumerate as many of them as possible, and really minimally on the actual description
<bitconner> that makes sense
<BlueMatt> I'm just saying dont over-impose structure unless we have something specific we want to do with it, otherwise we end up back where we are with htlc error codes.
<BlueMatt> so, like, impose structure on the basis of what we expect a counterparty to (maybe) do in response, not on the basis of what category of error it is
<ariard> maybe we should come first with a list of cases where more defined errors would provide values for a error-parsing implem
<cdecker> Isn't using the first byte as type a violation of bolt2, or at least preventing it from being printed?
<roasbeef> ariard: yeh we have those to publish soon 
<BlueMatt> I guess I'm just suggesting a different way to categorize errors which I think is more useful in general.
<cdecker> Rather add a TLV with the type byte at the end, that's backward compatible
<t-bast> ariard: ACK
<roasbeef> cdecker: is it? ppl have handled it fine for years, this was our error before we did the bolt stuff 
<ariard> roasbeef: okay let's hold on the conv until then
<roasbeef> adding a tlv to a variable length field can't really work, I've pointed it out a few times in the past...
<roasbeef> to the end 
<roasbeef> ariard: def, idk what's even being talked about rn lol 
<cdecker> roasbeef: the spec says not to print it verbatim if there are non-ASCII chars, which couldmean escape them, but that'll mangle the printed message
<ariard> roasbeef: something something error codes :p 
<roasbeef> kek
<roasbeef> g2g, peace y'all 
<cdecker> roasbeef: the message is length-prefixed, so it is clear where the TLV starts
<roasbeef> cdecker: but what if the length exceeds the full msg size? 
<roasbeef> just like w/ the init message, in theory there could be so many feature bits it takes up the entire message 
<BlueMatt> then its invalid, throw it away?
<cdecker> Exactly
<cdecker> Then fill it up to the end and don't add a TLV, or don't be so verbose and have a TLV
<roasbeef> that req isn't spelled out anywhere afaik, just pointing out the inconsistency 
<cdecker> I don't see the issue
<jkczyz> ok, for sake of time I'd say we should move on. is there an action item here?
<BlueMatt> it sounds like roasbeef to propose more concrete error codes and structure so we can have a more concrete discussion
<t-bast> Carla KC said she'd post something to the ML soon
<cdecker> Yep, but let's look at that as a later extension to the existing proposal
<t-bast> let's wait for that to discuss further
<jkczyz> #action follow up discussion after more concrete proposal is available
<jkczyz> #topic Funding expiry #839
<cdecker> Ok, got to run off. Nice seeing you all again (can't wait for a real-life meeting again...)
<t-bast> jkczyz: if people are dropping off, we can keep this one for next time, there's no rush
<t-bast> Or people can have a look at it directly on Github
<ariard> yeah we can start by this one next time
<jkczyz> t-bast: sure, looks like you were looking for more feedback
<t-bast> Damn right I missed real-life events
<jkczyz> #action follow up in GH
<ariard> t-bast: overall i'ts just a gentlemann agreement on the fundee-side
<ariard> no way to make it chain enforceable ?
<t-bast> ariard: yes exactly, but at least it makes it known to the funder what deadline he should enforce
<t-bast> so the funder can do its CPFP accordingly
<ariard> right, if you're a nice fundee yoou might even give a grace period of few blokcs past the deadline
<ariard> fee-bumping is a best effort :D
<bitconner> it's not clear to me what this adds exactly?
<t-bast> exactly, I planned for eclair to be a nice fundee and give a few blocks of leeway
<t-bast> bitconner: it helps avoid paying for two on-chain transactions for nothing in case funding times out...
* sword_sm1th (sword_smit@bitcoinfundamentals.org) has joined
<t-bast> I laid out that scenario in the PR description, we have a few users that try to optimize funding fees and ran into it a few times
<ariard> bitconner: AFAICT, double-spend the funding after the deadline
<bitconner> why not just double spend the coins?
<t-bast> because they can't know the deadline is met right now
<t-bast> they only discover it after funding tx confirms and the other party doesn't react
<ariard> it's a way for both counterparties to clean their states nicely
<bitconner> i see, so after funding_expiry the funder could make the descision to double spend if CPFP fails?
<ariard> yeah and if your counterparty did have a push_msat, don't count on it anymore
<t-bast> yes exactly, it lets both sides know what the timeout should be instead of a value hidden in the implementation
<bitconner> what should the sender do if the fundee says 10 years?
<bitconner> funder*
<t-bast> the fundee doesn't say anything
<bitconner> i.e. does the funder need to also propose
<bitconner> ?
<t-bast> that's covered in the PR, isn't it?
<t-bast> only the funder proposes
<ariard> jkczyz: don't forget to call endmeeting, otherwise you will have bad kharma until next meeting ;)
<bitconner> ah okay, i had them mixed up
<t-bast> if the fundee is not ok with the timeout proposed by the funder, he just rejects
<bitconner> gotcha
<jkczyz> ariard: finger is on the button
<t-bast> I might need to dive into the details of what "reject" means at that point in the channel's lifetime
<bitconner> sounds useful to me :)
<t-bast> we have at least one user who'd really love to have it :D
<ariard> it might be used as signaling with towers to cancel a watching period
<t-bast> I gotta go, but thanks everyone for the meeting and thanks jkczyz for chairing!
* sword_smith has quit (*.net *.split)
<jkczyz> #endmeeting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@t-bast and others