< bitcoin-git> [bitcoin] ariard closed pull request #19871: doc: Clarify scope of eviction protection of outbound block-relay peers (master...2020-09-clarify-eviction-block-relay) https://github.com/bitcoin/bitcoin/pull/19871
< bitcoin-git> [bitcoin] ariard reopened pull request #19871: doc: Clarify scope of eviction protection of outbound block-relay peers (master...2020-09-clarify-eviction-block-relay) https://github.com/bitcoin/bitcoin/pull/19871
< aj> signet merged, taproot in high-priority for review
< sipa> #addtoajsqueue #19988
< gribble> https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub
< aj> sipa: yeahyeah, it's on the top of a list
< kallewoof> i'm surprised taproot PR is not in conflict with signet merge. I recall it not being trivial to merge the two when I did it myself.
< aj> kallewoof: some of it might have been the CheckSig changes that we got rid of?
< kallewoof> that's probably it yeah!
< * sipa> tries
< sipa> works fine
< aj> aggh, gmaxwell is commenting in the factorio subreddit
< kallewoof> sipa: yeah i tried it too and it worked without conflicts.
< bitcoin-git> [bitcoin] hebasto opened pull request #19991: net: Use alternative port for incoming Tor connections (master...200922-tor) https://github.com/bitcoin/bitcoin/pull/19991
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19993: refactor: Signet fixups (master...2009-signetFixups) https://github.com/bitcoin/bitcoin/pull/19993
< jnewbery> Hi folks. There's a P2P meeting today in an hour. We have one proposed topic: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-IRC-meetings#22-sept-2020. Feel free to propose more in the next hour.
< gribble> https://github.com/bitcoin/bitcoin/issues/22 | Update the list of hard-coded node IP addresses · Issue #22 · bitcoin/bitcoin · GitHub
< jnewbery> bad bot
< jnewbery> sipa: perhaps a brief overview of #19988 would be of interest to the meeting attendees (motivation/design philosphy rather than technical details that can be found in the PR)
< gribble> https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub
< gleb> jnewbery: I have this PR #19958. I would just make a little announcement during the meeting, not planning to actually discuss it, because I think the trade-off is simple and already known.
< gribble> https://github.com/bitcoin/bitcoin/issues/19958 | Rename feelers to probes by naumenkogs · Pull Request #19958 · bitcoin/bitcoin · GitHub
< sipa> jnewbery: happy to do that
< sdaftuar> hello
< jnewbery> #startmeeting
< lightningbot> Meeting started Tue Sep 22 15:00:35 2020 UTC. The chair is jnewbery. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< hebasto> hi
< gzhao408> hi
< 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
< jnewbery> amiti fjahr jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
< ajonas> hi
< ariard> hi
< aj> holla
< amiti> hi
< gleb> hi
< gribble> https://github.com/bitcoin/bitcoin/issues/22 | Update the list of hard-coded node IP addresses · Issue #22 · bitcoin/bitcoin · GitHub
< fanquake> hi
< jnewbery> - Follow-up on "What would a good transaction propagation framework look like? See a first draw Transactions propagation design goals #19820 (ariard)
< gribble> https://github.com/bitcoin/bitcoin/issues/19820 | Transactions propagation design goals · Issue #19820 · bitcoin/bitcoin · GitHub
< jnewbery> - Overview of https://github.com/bitcoin/bitcoin/pull/19988 (motivation/design philosphy rather than technical details that can be found in the PR)
< jnewbery> Before we get to those, are there any other proposed topics, or does anyone have any short announcements to make?
< gleb> Yeah, I have one.
< vasild> hi
< gleb> I suggested to rename "feeler" connection to "probe" all along the codebase, because feelers currently capture two distinct features: feelers and test-before-evict.
< jnewbery> gleb: feelers -> probes ?
< jnewbery> ok. Let's do that first (and not spend 50 minutes on it :)
< jnewbery> #topic feelers -> probes (gleb)
< gleb> There's some support of renaming, but also couple hesitations and wladimir said we would rather not, because of rebase conflicts etc
< gleb> So I'm planning to drop this idea for now, and just improve the documentation.
< gleb> If someone is strongly in favor of renaming feelers to probes, please comment in the PR sometime soon :)
< sipa> fixing documentation is always good
< gleb> #19958
< gribble> https://github.com/bitcoin/bitcoin/issues/19958 | Rename feelers to probes by naumenkogs · Pull Request #19958 · bitcoin/bitcoin · GitHub
< sipa> or at least, making it less confusing
< gleb> That's it, we now can discuss other topics, unless someone have something to say right now :)
< amiti> thanks for the doc fix :)
< jnewbery> I'm generally in favour of making names more meaningful. If we're going to make this name change, I think it's preferable to do it before connection types are exposed in the RPC
< jnewbery> But I haven't looked at the specifics here, and don't have an opinion on this change
< jnewbery> ok, next topic?
< gleb> Good point wrt RPC
< gleb> yeah, we can move on i guess
< jnewbery> #topic Follow-up on "What would a good transaction propagation framework look like? See a first draw Transactions propagation design goals #19820 (ariard)
< gribble> https://github.com/bitcoin/bitcoin/issues/19820 | Transactions propagation design goals · Issue #19820 · bitcoin/bitcoin · GitHub
< ariard> I think we have a problem ecosystem-wise as we have protocols being designed and deployed
< ariard> which are completely insecure because of implicit assumptions on the tx-relay network and mempools behavior which are false
< sipa> anything that relies on specific properies of tx relay being guaranteed is broken
< gleb> how should we approach this problem? :)
< ariard> yes and I was aiming to synchronize people's model to suggest some kind of join IRC meeting
< ariard> with the LN devs, rusty, cdecker & all were down
< sipa> i think it's a good idea to work towards better analysing and documenting the design goals for transactions, but that's not going to result in any guarantees
< sdaftuar> i think tx relay works pretty well for transactions whose inputs are all confirmed?
< aj> sdaftuar: not if the transaction is RBF'ing something else that was previously relayed?
< ariard> sipa: I agree it's more how do we establish a common mental model ecosystem-wise ?
< sdaftuar> aj: agreed that we can continue to make improvements there!
< sdaftuar> aj: but i think we have something that is pretty reliable right now
< ariard> like either modifiyng second-layer protocols or tx-relay but not staying in-between
< gleb> The problem can be at least split into several
< vasild> Some LN protocol or implementation relies on a transaction propagating to every node?
< sipa> ariard: to me, the only solution if you need things that must confirm within a fixed amount of time is loudly yelling at the user if the timeout runs close
< gleb> For example, "assuming I can pay whatever fee, can I guarantee that my transaction will reach miners"?
< sdaftuar> guarantee is a very hard word to use
< ariard> sipa: I think we should dissociate propagation guarantee from confirmation guarantee, ofc you can't promise confirmation
< sipa> ariard: that's independent of potential improvements to tx relay that can make it more reliable in more varied scenarios - but if your security assumption is that relay (and confirmation!) are guaranteed, nothing can provide that
< ariard> sipa: yes I think we agree on the confirmation part, it's more the relay one which can be exploited by a malicious counterparty
< sipa> ariard: and i feel that framing this as "we need better tx relay because higher level protocols rely on some of its assumptions" is kind of missing the point
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/77376034d4ab...d692d192cda3
< bitcoin-git> bitcoin/master fa80c81 MarcoFalke: Assert that RPCArg names are equal to CRPCCommand ones (blockchain)
< bitcoin-git> bitcoin/master fa6bb0c MarcoFalke: Assert that RPCArg names are equal to CRPCCommand ones (rawtransaction)
< bitcoin-git> bitcoin/master d692d19 MarcoFalke: Merge #19849: Assert that RPCArg names are equal to CRPCCommand ones (bloc...
< gleb> sipa: so you're suggestion it should be users' responsibility?
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19849: Assert that RPCArg names are equal to CRPCCommand ones (blockchain,rawtransaction) (master...2008-rpcAssertNames) https://github.com/bitcoin/bitcoin/pull/19849
< gleb> To act when the timeout is too close. Go to a miner, or use other software, or whatelse
< sipa> i don't see what else you can do
< ariard> sipa: if I understand your point correctly we can deterministically guarantee propagation it's more a probabilistic issue
< ariard> *we can't
< gleb> I'm not even sure about probabilistic, facing an incentivised attacker.
< instagibbs> of course you can't, otherwise we wouldn't need PoW
< sipa> indeed
< sipa> so we should look at what features are useful for common cases in higher-level protocols, and to what extent those can be improved upon
< gleb> Random idea: a node could probe random nodes in the network to see how many of them knows about a tx.
< sipa> but if you frame this as "otherwise higher-level protocols are insecure", that's besides the point - if they were before, they'll still be insecure after
< vasild> if improved from 90% to 95% that is better but still not guaranteed (100%)
< ariard> I think the changes I'm proposing are more feature-wise than "making better tx relay"
< aj> sipa: i think it's more "spam prevention should be hard to use as an attack vector to prevent relay" when currently it's fairly easy to use it as an attack vector? (i consider rbf rules as spam prevention, adjust the wording if you disagree i guess)
< sdaftuar> is there a concrete set of proposed changes to consider?
< ariard> sipa: what do you understood by "we need better tx relay" ?
< gleb> I would propose to be more clear that "tx relay is not reliable no matter how much fees you pay"?
< ariard> sdaftuar: a) better documentation of policy to avoid someone hitting them for lack of testing b) something like package relay or consorts
< aj> gleb: there's reliable as in works 99% of the time, and reliable as in it's secure even in the face of a state-level attacker
< sdaftuar> ariard: i don't know what consorts means, but i don't think "package relay" is sufficiently fleshed out yet, unless there's a proposal i've missed?
< gleb> aj: "not guaranteed" perhaps. I'm not pushing the exact wording right now :)_
< ariard> sdaftuar: but there is a more philosophical question, "what if we tighten a policy rule and someone has built on it being liberal"
< ariard> lik increasing the mininal transaction size
< sipa> ariard: basically, my problem is with the word "need" - i don't know that we need anything, but that doesn't mean there can't be improvements
< sdaftuar> ariard: i agree that is a good question
< ariard> sipa: right, my wording is bad, it's more how we define clear rules a la BIP 125, and that's it don't make assumptions outside
< ariard> AFAIK, BIP 125 is the only standard on a mempool policy aiming to offer an interface usable by wallets/applications?
< sdaftuar> i think it's reasonable to ask whether policy changes to Bitcoin Core should always be documented in a BIP so that wallet authors can take those changes into account
< ariard> sdaftuar: voila, that's what I've in mind :)
< ariard> or protocol authors
< sdaftuar> that was exactly why i had asked for BIP 125 to be drafted in the first place, fwiw
< sipa> ariard: documenting the relay policy more accurate definitely sounds like a good idea
< sdaftuar> unfortunately we're starting from a place where none of it (except for bip 125) is documented
< ariard> sdaftuar: yes like we didn't have a bip for carve-out and some folks are trying to reuse it beyond LN
< ariard> sadly insecurely
< jnewbery> I don't think p2p policy belongs in BIPs. https://github.com/bitcoin-core/bitcoin-devwiki/wiki seems like a more appropriate place for it
< sdaftuar> jnewbery: thanks i was just going to say that it's not clear to me that BIPs are the right place either
< sdaftuar> on one hand, policy changes may have effects on other software, so a BIP might seem the right place
< ariard> I agree not necessarily a BIP as far it's documented somewhere
< luke-jr> strict policy, no, but things that coordinate yes
< ariard> though we have the example of BIP 125
< luke-jr> BIP 125 gives meaning to particular sequence values
< sdaftuar> however there is also a sense that we make implementation-specific changes frequently enough that it woulnd't make sense to always publish things in the BIP repo to reflect them
< ariard> sdaftuar: I agree that's too heavy to update anytime we change the rejection filter
< luke-jr> ariard: it's a bug for software to rely on node policies
< ariard> luke-jr: how do you frame BIP 125, it's a node policy ?
< luke-jr> ariard: it's not a node policy, it's a definition of sequence values; implementations can honour or ignore the request
< luke-jr> the latter decision is the policy
< jnewbery> I feel like we're getting into the semantic weeds here. The important thing is that policy is documented somewhere, not what you call it.
< luke-jr> (text aside)
< sdaftuar> jnewbery: i don't think that's exactly true. i think the point luke is making (or at least making me consider harder) is whether there are some aspects of node policy that are different from others
< jnewbery> and to me, the wiki feels like the obvious place. This is information about Bitcoin Core's policy that is being communicated to external developers
< luke-jr> stuff external developers should design around, makes sense to have in a BIP; but generally, that should not include most node policy
< luke-jr> (we can still document policy of course)
< ariard> luke-jr: okay if I follow your semantic BIP 125 define a request mechanism ; choosing to implement this mechanism is a policy decision
< luke-jr> ariard: yes, basically
< ariard> luke-jr: and I agree with you we can't enforce that node operators are effectively deploying this policy
< sdaftuar> luke-jr: in general, though, external developers are desiging around the policy already
< luke-jr> sdaftuar: that's a bug in their design IMO
< ariard> luke-jr: no more we can guarantee that everyone isn't running with blocksonly
< sdaftuar> and in fact that is the source of this whole discussion
< luke-jr> and not something we should encourage
< ariard> what was the thinking when bloom filters where turn off by default ?
< ariard> it's this kind of software setting default which encourage/discourage network-wise behaviors?
< ariard> *were
< jnewbery> we have one more topic, so I suggest we move on to that soon
< sipa> ariard: it was turned off because there is an obvious resource usage attack enabled by them (high disk I/O and CPU usage, barely any bandwidth for the attacker), and discussed quite widely before done so
< sdaftuar> luke-jr: perhaps the right way to think about this is that wallet authors should use their best understanding of what policy rules are deployed on the network to generate transactions that will propagate well, but the bug is in relying on that for security?
< ariard> security isn't binary, it's more how do you diminish the risk of transactions not propagating well
< vasild> rely on relay is bad :)
< sdaftuar> security is also not probabilistic
< aj> (20min left)
< jnewbery> thanks aj, let's move on to the next topic
< ariard> sdaftuar: I think we're going to switch off-topic but it sounds like second layers have somehow to have this probabilistic model
< jnewbery> #topic Overview of https://github.com/bitcoin/bitcoin/pull/19988 (motivation/design philosphy rather than technical details that can be found in the PR)
< jnewbery> over to you, sipa
< sipa> hi!
< ariard> jnewbery: wait before to switch do people would like to coordinate an IRC meeting with LN devs to talk about those issues ?
< ariard> or anyone else interested by those propagation issues
< gleb> ariard: The only common understanding we currently have here seem to be "need more docs". I don't see how talking to LN devs helps here?
< luke-jr> sdaftuar: wallets should use standards, and node policies ideally will allow standard transactions
< ariard> gleb: because they have implemented most of the content which could be in those "need more docs" :)
< jnewbery> ariard: can you co-ordinate your next meeting after this meeting? Let's move on to 19988!
< gzhao408> woo 19988!
< ariard> jnewbery: sure let's move on
< jnewbery> thanks!
< sipa> hi!
< sipa> so i recently PR'ed #19988, which is a rebase of 19184
< gribble> https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub
< sipa> the idea is that our tx fetching logic has really grown over time
< sipa> with various data structures needed for coordination, and unclear specification of what they actually implement
< sipa> there are biases in favor/against fetching from certain nodes, but they're all implemented indirectly through random delays and insertions/lookups in maps, that are hard to reason about
< sipa> so instead, my idea was to create a clear specification of what should be fetched from what, or at least something that can be defined in function of a simple data structure
< sipa> and then have one class that encapsulates a very efficient implementation of that
< sipa> 19988 does that
< sipa> to test that, i wrote a fuzz tester which contains a naive reimplementation with the exact same behavior, and which tries to find sequences of operations that makes them diverge
< sipa> (which, it turns out, found a lot of them... but all within ~minutes - it has now run for several weeks altogether...)
< vasild> what does it mean if the naive implementation differs from the real one?
< instagibbs> sorry naive version of what?
< jnewbery> vasild: probably that there's a bug in the real one :)
< instagibbs> like, legacy logic, vs, your encapsulation, vs another implementation?
< vasild> jnewbery: or in the naive one :)
< sipa> instagibbs: difference between the efficient boost::multi_index based implementation, and the naive one in the fuzzer
< instagibbs> ah!
< sdaftuar> concept ACK from me... feature freeze is oct 15, do people have thoughts on getting this in for 0.21?
< instagibbs> naive efficiency-wise
< instagibbs> sdaftuar, don't mean to touch the third rail, but taproot implementation?
< instagibbs> I guess that doesn't intersect aside from PR author
< jnewbery> instagibbs: taproot wouldn't be merged before 0.21, so I think this is the priority
< jnewbery> (in terms of sequencing)
< sdaftuar> yeah i don't know what one has to do with the other, i'd just like to make our review time be efficient
< instagibbs> jnewbery, mmmm ok I guess I hadn't heard that decision
< instagibbs> sdaftuar, same author means limited reaction bandwidth, just noting
< sipa> i was hoping for both :)
< jnewbery> This is +2000/-450 LOC, so reviewing and being comfortable to merge in three weeks seems ... ambitious
< instagibbs> it's not new code.
< sipa> jnewbery: most is in tests
< sipa> (but the non-test code is quite hairy, i admit)
< instagibbs> vast majority of changes were test changes recnetly
< sipa> i'd encourage reviewers to really look at the fuzz test first - even ignoring the fuzzing aspect, the naive reimplementation probably gives a pretty good idea of what the thing *should* do
< instagibbs> (sorry, stopping)
< ajonas> I'd be happy to try some organized nagging if people want to give it a shot to get this in
< instagibbs> concept ACK the actual topic
< jnewbery> it might be better to wait until after 0.21 to merge, so that it has more soak time before being in a release?
< jnewbery> it's not like ADDRv2, where we have some external dependency driving deadlines (torv2 deprecation)
< ajonas> with two min to go can we check in on those 0.21 priorities?
< sipa> well, it depends on reviewer time of coursew
< aj> seems like putting it in early in a cycle would make backporting other p2p things (ie from 0.22pre to 0.21) harder, and there's some reasonable soak time between feature freeze and rc?
< sipa> aj: yeah
< sdaftuar> release date is december 3 right now
< sdaftuar> so that is a fair point
< sdaftuar> (estimated i guess)
< jnewbery> one minute left!
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19994: Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (master...2009-rpcAssertNames) https://github.com/bitcoin/bitcoin/pull/19994
< ajonas> The other two priorities mentioned were:
< ajonas> - ADDRv2 - #19031 (next in sequence is 19845, which is close)
< ajonas> - outbound & block-relay-only connections in functional tests (#19315)
< gribble> https://github.com/bitcoin/bitcoin/issues/19031 | Implement ADDRv2 support (part of BIP155) by vasild · Pull Request #19031 · bitcoin/bitcoin · GitHub
< jnewbery> #endmeeting
< gribble> https://github.com/bitcoin/bitcoin/issues/19315 | [tests] Allow outbound & block-relay-only connections in functional tests. by amitiuttarwar · Pull Request #19315 · bitcoin/bitcoin · GitHub
< lightningbot> Meeting ended Tue Sep 22 16:00:25 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< instagibbs> sdaftuar, which is a good point? there was a flurry of convo there
< sdaftuar> oh i meant that feature freeze -> release was a good bit more time than i had initially realized at least
< instagibbs> ah, +1
< sdaftuar> i guess you'd think i'd remember these things by now, oops
< aj> sdaftuar: why remember anything you can lookup? bandwidth is cheap
< sipa> aj: latency hit, though
< aj> sipa: just do it while configure is running and it's fine
< sipa> bizarre discovery: memcmp in some GCC9 and GCC10 versions are broken, if passed a constant array input that starts with a 0 byte
< sipa> which is fixed in GCC 10.2
< sipa> i suspect some of bitcoin core's unit tests may be affected too
< sdaftuar> whoa
< sipa> i don't immediately see any other uses that would be affected
< roconnor> Not fixed in GCC 10.2. Fixed in master.
< sipa> roconnor: oh yes, i misread the report
< sipa> i confirm the bug exists in the 10.0.1 that's in ubuntu'
< sipa> i can't reproduce it with gcc 9.3
< sipa> i also see the problem with std::lexicographical_compare in C++
< roconnor> weird, I have trouble with gcc 9.3.
< roconnor> Are you using -O2?
< sipa> huh, in C++ i also see it with 9.3
< sipa> but not in C
< sipa> yes, using -O2
< jonatack> Sorry for missing the p2p meeting. The implementations of addrv2, taproot, and PR19988 are probably the top 3 priorities to review for me.
< bitcoin-git> [bitcoin] practicalswift opened pull request #19995: log: Mitigate disk filling attacks by rate limiting LogPrintf(…) (master...mitigate-log-disk-filling-attacks) https://github.com/bitcoin/bitcoin/pull/19995
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/d692d192cda3...c7eb85d00593
< bitcoin-git> bitcoin/master f07fb5a fanquake: build: patch qt libpng to fix powerpc build
< bitcoin-git> bitcoin/master c7eb85d MarcoFalke: Merge #19959: build: patch qt libpng to fix powerpc build
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19959: build: patch qt libpng to fix powerpc build (master...powerpc_libpng_qt) https://github.com/bitcoin/bitcoin/pull/19959
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/c7eb85d00593...b1291b2e8fc3
< bitcoin-git> bitcoin/master e153448 t-bast: Clarify blocksonly whitelistforcerelay test
< bitcoin-git> bitcoin/master b1291b2 MarcoFalke: Merge #19963: Clarify blocksonly whitelistforcerelay test
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19963: Clarify blocksonly whitelistforcerelay test (master...clarify-whitelist-force-relay-test) https://github.com/bitcoin/bitcoin/pull/19963
< bitcoin-git> [bitcoin] sipa opened pull request #19997: History for Taproot PR #19953 (master...taproot-history) https://github.com/bitcoin/bitcoin/pull/19997
< bitcoin-git> [bitcoin] hebasto opened pull request #19998: rpc: Add `via_tor` to `getpeerinfo` output (master...200922-istor) https://github.com/bitcoin/bitcoin/pull/19998
< bitcoin-git> [bitcoin] fanquake closed pull request #19751: depends: Split libpng out of Qt (master...depends_libpng) https://github.com/bitcoin/bitcoin/pull/19751