15:00:13 <jnewbery> #startmeeting 
15:00:13 <core-meetingbot> Meeting started Tue Nov 17 15:00:13 2020 UTC.  The chair is jnewbery. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
15:00:13 <core-meetingbot> Available commands: action commands idea info link nick
15:00:17 <amiti> hi
15:00:21 <jnewbery> #bitcoin -core-dev P2P Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james
15:00:22 <ariard> hello
15:00:27 <jnewbery> amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
15:00:34 <jnewbery> fanquake: your clock is 2 minutes fast
15:00:34 <sdaftuar> heya
15:00:36 <troygiorshev> hi!
15:00:55 <jnewbery> Hi folks. Welcome to the first p2p meeting of 22.0!
15:01:00 <ajonas> hi
15:01:11 <sdaftuar> we branched off already?
15:01:13 <jnewbery> The last 0.21 milestone PRs were merged today, so I guess the branch off is imminent.
15:01:40 <jnewbery> We have one proposed topic for today's meeting: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#17-nov-2020.
15:01:44 <gribble> https://github.com/bitcoin/bitcoin/issues/17 | listaccounts method · Issue #17 · bitcoin/bitcoin · GitHub
15:01:59 <ariard> yep
15:02:06 <jnewbery> Before we get to that, I think it'd be useful for us to all share what our goals/priorities are for the 22.0 release cycle.
15:02:12 <jonatack> hi
15:02:28 <jnewbery> does anyone want to go first?
15:02:28 <fanquake> hi
15:02:34 <kanzure> hi
15:03:28 <sdaftuar> i'd say that i think it'd be great to prioritize erlay now
15:03:28 <ariard> Im waiting for https://github.com/bitcoin/bitcoin/pull/19160 landing before to go forward with altnet
15:03:47 <ariard> agree with erlay
15:04:29 <jonatack> BIP 342 implementation. Erlay. (And non-p2p-specific: multiprocess, coinstats, consensus code separation, assumeUTXO...)
15:05:01 <jnewbery> For me, I'd like to make some substantial progress in clarifying the net/net_processing and net_processing/validation interfaces. Those two projects are tracked in #19398 and #20158.
15:05:04 <gribble> https://github.com/bitcoin/bitcoin/issues/19398 | Move remaining application layer data to net processing · Issue #19398 · bitcoin/bitcoin · GitHub
15:05:06 <gribble> https://github.com/bitcoin/bitcoin/issues/20158 | tree-wide: De-globalize ChainstateManager by dongcarl · Pull Request #20158 · bitcoin/bitcoin · GitHub
15:05:19 <sdaftuar> i plan to work on block-relay-only peering too, though i'm stuck on some annoying addr-relay things that might hold me up
15:05:37 <troygiorshev> With the feature freeze now thawed, I wanted to remind everyone about Per-Peer Message logging #19509.  It got a lot of attention a while ago and I think it's a feature a lot of people would appreciate.  Just needs a bit more review!
15:05:40 <gribble> https://github.com/bitcoin/bitcoin/issues/19509 | Per-Peer Message Capture by troygiorshev · Pull Request #19509 · bitcoin/bitcoin · GitHub
15:05:51 <troygiorshev> I'm also paying attention to BIP324 #18242
15:05:57 <gribble> https://github.com/bitcoin/bitcoin/issues/18242 | Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli · Pull Request #18242 · bitcoin/bitcoin · GitHub
15:06:08 <jonatack> jnewbery: sdaftuar: good stuff. i plan to resume review of those.
15:06:31 <jnewbery> thanks jonatack!
15:06:42 <jonatack> troygiorshev: thank you, i meant BIP324 :)
15:06:43 <ariard> I'm planning to work on a better version of #18797 and so understand better tx-standarndes before going back to package relay
15:06:46 <gribble> https://github.com/bitcoin/bitcoin/issues/18797 | Export standard Script flags in bitcoinconsensus by ariard · Pull Request #18797 · bitcoin/bitcoin · GitHub
15:07:34 <amiti> in terms of my work: hoping to make progress on 19315 (adding full-relay and block-relay-only to tests), and I'm working on reviving rebroadcast
15:07:34 <nehan> hi
15:09:00 <jonatack> amiti: will review
15:09:02 <jnewbery> ok, anyone want to add any topics before we get onto "Reducing CVE-2020-26895 class of bugs and Tx-standardness" (ariard)
15:09:14 <gleb> Hi
15:09:16 <aj> touch base on wtxid backport?
15:09:56 <jnewbery> hi aj. Yes, let's do that one first
15:10:09 <jnewbery> #topic touch base on wtic backport (aj)
15:10:09 <core-meetingbot> topic: touch base on wtic backport (aj)
15:10:41 <jnewbery> *wtxid
15:10:43 <sdaftuar> i'm not sure we should have backported wtxid-relay to the 0.20 branch
15:11:09 <aj> #20317 and #20399
15:11:11 <gribble> https://github.com/bitcoin/bitcoin/issues/20317 | Backport wtxid orphan fetch to v0.20 by jnewbery · Pull Request #20317 · bitcoin/bitcoin · GitHub
15:11:12 <gribble> https://github.com/bitcoin/bitcoin/issues/20399 | Revert "Merge #19606: Backport wtxid relay to v0.20" by MarcoFalke · Pull Request #20399 · bitcoin/bitcoin · GitHub
15:12:08 <aj> afaik, #19620 which is in 0.19 and 0.20 already does everything we really need wtxid to do backport-wise for taproot
15:12:12 <gribble> https://github.com/bitcoin/bitcoin/issues/19620 | Add txids with non-standard inputs to reject filter by sdaftuar · Pull Request #19620 · bitcoin/bitcoin · GitHub
15:12:59 <aj> so 20399 is there to revert wtxid relay from 0.20; or 20317 is there to fix up the orphan handling regression if there's a reason to keep it
15:13:02 <sdaftuar> (i missed that github discussion, catching up now)
15:13:11 <jnewbery> I don't have a strong opinion, but I
15:13:51 <jnewbery> 'm curious what has changed since this was discussed in a previous meeting, when people agreed that it should be backported
15:14:30 <aj> 19620 imo
15:15:41 <anekdotin> great job guys
15:15:47 <sdaftuar> (ok caught up)
15:16:16 <sdaftuar> i think that knowing that there was a crashing bug in that line of work sort of heightens the sense that backporting a feature is not really a great idea?
15:16:53 <sdaftuar> i don't think backporting it was a requirement in the first place, but i saw the reasoning that it was a nice-to-have.  but given that it indeed turned out to be risky, i'd say let's drop it, as there's no compelling reason for backport after #19620 either
15:16:57 <gribble> https://github.com/bitcoin/bitcoin/issues/19620 | Add txids with non-standard inputs to reject filter by sdaftuar · Pull Request #19620 · bitcoin/bitcoin · GitHub
15:17:17 <sdaftuar> so falling back on our principle of only backporting bugfixes seems like the prudent thing
15:20:08 <jonatack> A propos, sharing a message from moneyball who mentioned tripling down on testing (bug discovery+fixes) between now and the 0.21 release with all of the new features.
15:20:36 <jnewbery> sdaftuar: do you think it should be reverted?
15:20:39 <sdaftuar> yeah
15:21:15 <jnewbery> because we're not confident in the code?
15:21:19 <sdaftuar> if 0.21 has some other crashign bug related to wtxid-relay, it'd be great to be able to go back to 0.20.2 or whatever and have it not crash!
15:22:19 <aj> jnewbery: so i think your review beg on oct-20th should've resulted in some sort of a "do we really still need this?" response, rather than silence both here and on the PR -- i think that was the only time it was mentioned in these meetings since 19620
15:23:18 <jnewbery> aj: yeah, people agreed in the original meeting that it should have been backported, and then I've raised reviewing the PR in several meetings since then
15:23:52 <jnewbery> I don't think the presence of a bug that was caught in review and fixed in the follow up changes the facts, but I can understand that it makes people nervous
15:24:35 <jnewbery> so if people want it out, then it should be reverted
15:25:20 <jnewbery> #action revert #20317
15:25:21 <core-meetingbot> revert #20317
15:25:23 <gribble> https://github.com/bitcoin/bitcoin/issues/20317 | Backport wtxid orphan fetch to v0.20 by jnewbery · Pull Request #20317 · bitcoin/bitcoin · GitHub
15:25:23 <gribble> https://github.com/bitcoin/bitcoin/issues/20317 | Backport wtxid orphan fetch to v0.20 by jnewbery · Pull Request #20317 · bitcoin/bitcoin · GitHub
15:26:01 <jnewbery> #topic Reducing CVE-2020-26895 class of bugs and Tx-standardness (ariard)
15:26:01 <core-meetingbot> topic: Reducing CVE-2020-26895 class of bugs and Tx-standardness (ariard)
15:26:07 <ariard> hello
15:26:47 <ariard> first and foremost, I apologize for the previous rounds of discussion on this issue, it was a bit noisy to talk grasp problem right this without this vuln being public
15:28:00 <ariard> for the context, lnd didn't check that counterparty commitment signatures were in the curve order /2
15:28:42 <ariard> which means a malicous counterparty was able to feed them with non-relayable witness and thus complete break its security
15:29:14 <ariard> because those time-sensitive transactions won't propagate before timelock expiration
15:29:29 <ariard> of the HTLCs
15:30:21 <ariard> I think the lesson it's not the first time that a LN implementation had issues with tx-standard, CL had some 2y ago IIRC about minimal relay fee
15:31:14 <ariard> so it sounds we have this class of vulns concerning any time-sensitive protocols, and how to mitigate it correctly is a bit unclear
15:31:30 <luke-jr> be strict on transaction form..
15:31:31 <sdaftuar> testmempoolaccept?
15:31:34 <aj> testmempoolaccept RPC isn't useful because you don't want to finish signing the tx, i guess?
15:31:50 <luke-jr> you can never rely on node/relay policies
15:32:00 <luke-jr> just have to do what you can to reasonably pass them
15:32:19 <luke-jr> testmempoolaccept only tells you about your own policy
15:32:24 <ariard> aj: my concern with testmempoolaccept its checks are blurred with the fees ones
15:32:43 <ariard> but your transaction is okay because you plan to broadcast only in the future
15:33:01 <sdaftuar> luke-jr: i agree, but perhaps a belt and suspenders approach where you enforce a strict transaction form and also double-check against testmempoolaccept might be helpful
15:33:17 <luke-jr> it might detect bugs I suppose
15:33:25 <ariard> also you have a really code architecture thing, LN mobiles won't have a mempool but they need to do the check
15:33:25 <sdaftuar> or unexpected changes
15:34:01 <sdaftuar> ariard: i think you'll need to specify the problem a bit better if you are assuming not having access to a full node
15:34:04 <luke-jr> ariard: it can't be secure without your own full node anyway
15:34:06 <sdaftuar> for instance, do you have access to all the inputs?
15:34:33 <ariard> luke-jr: you might receive the headers from a trusted connection to your full node on your LN node
15:34:53 <luke-jr> so you mean you don't have RPC access?
15:35:02 <ariard> sdaftuar: I think we should assume no, it's concering any kind of LN clients
15:35:06 <luke-jr> too bad we removed the reject message..
15:35:16 <sdaftuar> ariard: if no, then isn't this problem hopeless?
15:35:38 <ariard> sdaftuar: you have access to the funding utxo of your channel
15:35:49 <ariard> always, that's a protocol assumption
15:36:04 <sdaftuar> ariard: but if i include an input you don't know about, then you must reject the transaction, because you have no way of knowing if it's valid, let alone standard?
15:36:11 <ariard> what you want to verify is the chain of transaction built from this utxo (commitment+HTLC txn) are tx-relay "valid"
15:36:46 <ariard> sdaftuar: for commitment transaction the utxo is 2-of-2
15:36:50 <luke-jr> ariard: not possible to do reliably..
15:38:02 <ariard> luke-jr: at least you should be able to be sure your transaction is accepted by your full-node
15:38:04 <luke-jr> nodes will relay what they want; there are no consensus rules forcing relay
15:38:26 <luke-jr> ariard: sure
15:38:27 <ariard> luke-jr: I finally agree with you on this, it's more a LN-client to its full-node relation we should care about?
15:39:13 <luke-jr> ariard: so this part of problem is because you don't necessarily have RPC access, and we no longer support the reject msg?
15:40:01 <ariard> luke-jr: I think it's more subtle, you have those LN transactions, that both you and your counterparty must agree on, but ultimately their validity is decided by your full-node
15:40:09 <aj> ariard: what good is a commitment tx with a low fee rate that won't be acceptable possibly for months? don't you want to know that your node will (currently) reject that?
15:40:37 <luke-jr> hmm, I suppose the TXOs spent in it migth also not be something you want broadcast yet
15:40:47 <ariard> aj: in the future we might have fixed-fee commitment transactions and their feerate adjusted by a CPFP+package relay
15:40:58 <ariard> but I may jump one step in the reasoning here, hard to see
15:41:30 <aj> ariard: right, but at that point you'd expect testmempoolaccept to support packages
15:41:45 <sdaftuar> aj: bold :)
15:42:18 <aj> sdaftuar: (easier to implement package testmempoolaccept than package relay anyway?)
15:42:32 <ariard> luke-jr: what's concerning is right now we are leaking those policy check directly in the LN spec, but we may should do the reverse and let those tx-relay be ultimaltey defined by LN clients
15:42:39 <sdaftuar> aj: fewer DoS vectors to worry about for sure!
15:42:53 <glozow> aj: hey guess what
15:43:03 <aj> glozow: you already did it??
15:43:23 <ariard> aj: yes if this the case, it's more a cod architecture reason, to have a strict transaction form lib
15:43:48 <glozow> a little bit, i have a math test this week though, and then i'll send you my draft of package testmempoolaccept
15:43:52 <luke-jr> ariard: the spec just needs to be narrow enough that there's a good hope any tx will be accpeted
15:44:42 <ariard> aj: we shouldn't require LN clients to have access to all the utxo set, you just care about your channel utxo only, testmempoolaccept assume you have access to the full set rn?
15:44:43 <luke-jr> double-checking against your own node makes sense, but ultimately shouldn't be your security check
15:45:13 <luke-jr> ariard: to open a channel, you need access to the entire UTXO set to be sure the inputs are valid..
15:45:49 <sdaftuar> ariard: oh, yes you're right that testmempoolaccept requires all inputs to be available in mempool or utxo set, so if you're testing validity further down a chain that won't work right now
15:45:58 <aj> ariard: i think if you pull some checks out of "here's my full node" into "here's my light library" you'll run the same risk of missing some checks that are actually needed; but ymmv of course
15:46:00 <ariard> luke-jr: I see your point, but you might not have access to the UTXO set for the rest of the discussion
15:46:08 <ariard> *protocol execution, sorry
15:47:05 <ariard> aj: yes that's exactly my worry, mempool checks are a bit blurred, and some we are interested with are also inside the script interpreter
15:47:14 <aj> ariard: maybe a psbt validator could make more sense, alternatively? partial so you don't need all the inputs or even all the signatures, but still get useful results like "fees look low!" "this is a dust output!" "this signature is wrong!"
15:48:06 <aj> ariard: psbts are (probably) common enough that it would justify thorough useful (re)implementation of standardness and consensus and sanity checks
15:48:12 <ariard> luke-jr: without entering in a light-client debtate, for LN if you assume people are using light-clients, it's not reason for them not being at least secure on tx-relay checks
15:48:18 <ariard> lke layering security reasoning
15:49:19 <jnewbery> ariard: testmempoolaccept does require the full UTXO set, but you could imagine a version where you provide the inputs, like signrawtransaction
15:49:21 <sdaftuar> philosophically though there is no such thing as "bitcoin's tx relay rules". i think that's luke's fundamental point (which I agree with)
15:49:22 <ariard> aj: only validating the inputs and not the other transaction check like version, number of max outputs
15:49:25 <ariard> ?
15:49:42 <aj> ariard: validating all the other stuff seems relevant for psbts in general, i think?
15:49:53 <luke-jr> ariard: your non-debatable premise is false
15:50:10 <ariard> sdaftuar: I finally agree with it, what I'm trying to understand is the margin we might have to make thing better
15:50:53 <sdaftuar> i think the best we can do is what luke suggested already: narrow what you accept to a small enough subset that you think will likely to always be relayable now and in the future, and hope for the best.
15:51:28 <sdaftuar> testing validity against a bunch of full-node implementations is a nice backup to make sure your code isn't broken or things haven't changed out from under you
15:53:02 <aj> sdaftuar: not very robust against hostile peers who have access to your code base to find checks you forgot to implement, though; hard to fuzz test for valid-but-non-standard sigs and the like even, afaik
15:53:09 <ariard> I see the point, which means anytime someone is implementing or desinging a L2 protocol you have to open your full-node code and understand those blurred checks
15:53:42 <jnewbery> ariard: do you have what you need from this discussion? Anything else you need or could use help with?
15:53:52 <ariard> aj: we should assume your counterparty shouldn't be able to observe your full-node implementation or policy, but can we really do this?
15:54:44 <ariard> jnewbery: if I'm reworking on a #18797 as a wrapper on top of testmempoolaccept like libconsensus is anyone have strong NACK?
15:54:47 <gribble> https://github.com/bitcoin/bitcoin/issues/18797 | Export standard Script flags in bitcoinconsensus by ariard · Pull Request #18797 · bitcoin/bitcoin · GitHub
15:55:20 <ariard> for code architecture reason, not making an assumption your LN client has access to a RPC interace
15:56:12 <jnewbery> I don't understand what you mean as a 'wrapper on top of testmempoolaccept'
15:56:59 <ariard> jnewbery: on top of ATMP, but it sounds we need to be able to pass a partial utxo set first to it
15:57:07 <jnewbery> but in any case, we should wrap this up. It's almost endmeeting time.
15:57:22 <jnewbery> Any last minute topics before we close?
15:57:27 <gleb> Just wanted to let you guys know that #18261 is ready for review (despite Travis being a bit unhappy, fixing that)
15:57:27 <sdaftuar> if you mean add package support to testmempoolaccept, i think that would be reasonable
15:57:29 <gribble> https://github.com/bitcoin/bitcoin/issues/18261 | Erlay: bandwidth-efficient transaction relay protocol by naumenkogs · Pull Request #18261 · bitcoin/bitcoin · GitHub
15:57:36 <sdaftuar> gleb: nice!
15:57:40 <ariard> yeah people want to run those LN clients on hardware, and you might not want to run a full-node with full resource on this
15:57:47 <ariard> jnewbery: sure, thanks anyone for the discussion
15:57:54 <jnewbery> sdaftuar: glozow has a draft implementation of that
15:57:59 <aj> gleb: haha, was just checking logs to see if it'd be rude to ask about that
15:58:16 <jnewbery> thanks gleb!
15:58:17 <ariard> jnewbery: is it coalesing the feerate check ?
15:58:34 <jnewbery> ariard: yes, returns feerate for each individual tx and the package
15:59:06 <jnewbery> ok, that's time. Thanks all!
15:59:08 <jnewbery> #endmeeting