< fanquake> Yea it's rare that we'd backport something that isn't actually bugfix, especially to such an old/stable branch.
< bitcoin-git> [bitcoin] fanquake closed pull request #19009: Print "verifychain" progress every 1 step if user verifies whole blockchain. (master...patch-3) https://github.com/bitcoin/bitcoin/pull/19009
< * kallewoof> wakes up
< kallewoof> so, the mining stuff is a bit of a work in progress, but AJ is making a follow-up pull request on top of the current one, which has mining in it. the current miner I use on my end is this: https://github.com/kallewoof/bitcoin/tree/202008-signet-ajkey-scripts
< kallewoof> I'll look into making simple tests that check the current signet consensus stuff with what we have, if people are uncomfortable with the PR not having enoguh tests, but i personally think "worst thing that happens is that signet does not work" (everything else has tests, after all, and the signet code *is* touched then too) fwiw.
< kallewoof> wumpus sipa aj ^
< sipa> kallewoof: what PoW difficulty does (the default) signet have?
< kallewoof> sipa: very low, lower than main/testnet
< sipa> or maybe better, how low can you set it for a custom one
< kallewoof> it takes a few seconds on a cpu initially
< aj> kallewoof: poking at making it accept OP_TRUE as the block challenge with no witness commitment, so plain -generate works
< sipa> alternatively, if it can be made low enough, a block could be constructed entirely on the python side (the test framework probably has nearly everything already)
< kallewoof> we could also just pre-mine the block(s) in that case. if the coinbase etc. are all predefined we can set the nonce and such. shouldn't be needed tho as the difficulty is pretty low. (the original mining tests used the c++ miner though, so it may be slow actually..)
< sipa> yeah, that's also a possibility
< sipa> mining from the python side means you could easily extend it with some less trivial challenge script too (though that would mean adding the sighash logic there as well, which may make sense anyway)
< kallewoof> yeah, i mean the signing code is not big; we could literally drop the generate script into the test framework and use it
< kallewoof> aj: that would mean the generate script can import those things without doing import path hacks too.. maybe the contrib/signet stuff should just call ../../test/functional/test_framework/(signet_)generate.py or something
< sipa> kallewoof: fwiw, the powLimit in the signet PR doesn't exactly match the difficulty as specified in BIP325 (the digits after 00002adc28 should be zeroes)
< kallewoof> *blinks*
< aj> kallewoof: https://github.com/ajtowns/bitcoin/commit/33b01c42be9e4635c9f590c65d6c2873b0b0f8be allows you to set `-signet_blockscript=51` and then just pretend you're on regtest (except for difficulty)
< kallewoof> aj: nice, thanks
< aj> kallewoof: another option (short of including generate.py) might be have the blockscript be "OP_SHA256 x OP_EQUAL" which would mean just adding a constant suffix to the witness commitment for each block
< sipa> exercising the sighash logic would be even nicer :)
< * sipa> hides
< kallewoof> sipa: are you saying that the compact 0x1e2adc28 expands to 00002adc28000[...] and not the one that is there now? I derived it the other way around, and just kept it as is. Having zeroes makes sense but would mean another reset.
< sipa> kallewoof: yes
< sipa> well it wouldn't invalidate any existing blocks (with high probability), but it is technically a signet softfork i guess
< kallewoof> ohh... actually, you're right it wouldn't. it would just make the limit drop a small amount
< sipa> i mean: the implementation doesn't match the spec right now (and though the probability is small, it's not unobservable; one in 299928 attempts)
< sipa> one in 299928 *succesful* attempts
< aj> sipa: yeah, that needs generate.py or equivalent
< kallewoof> aj: generate.py without the copy-pasted stuff is not large, though. maybe it should (permanently) reside in the test framework path
< kallewoof> but that means more for reviewers to review, so probably just try to hack something easy for now..
< aj> kallewoof: yeah, maybe; don't really want to add it in before cleaning it up more though
< kallewoof> aj: all right
< aj> sipa, kallewoof: changing the powLimit to 2adc280000000 doesn't seem to invalidate any existing blocks
< kallewoof> i just confirmed that it doesn't, too
< kallewoof> pushed squashie
< kallewoof> is there a point not squashing if you end up having to rebase on master due to a conflict? I can't see one personally, so gonna squash.
< gwillen> kallewoof: if you squash + rebase in one operation it becomes slightly harder to tell whether the squash was just a squash or changed anything (when re-reviewing), but I don't know that it's a big deal
< gwillen> unless it's a large complex PR with a bunch of existing review
< gwillen> (and you can always use commandline git to check)
< gwillen> (but it's a bit of a PITA)
< kallewoof> gwillen: i made squash commits, then ended up with a conflict against master and rebased. the entire PR is as such "detached" from whatever commit reviewers saw the last time, so i could've added stuff to the other commits without anyone noticing at that point.
< sipa> kallewoof: unless PRs are huge (i haven't reviewed your signet one on detail, but it doesn't look very large), i find that most of the work in a review is understanding the flow of changes and how things fot together
< kallewoof> yeah, i thought so, but i caused poor jonatack to have to restart his review several times cause i kept squashing changes.
< sipa> once that's done, i don't mind reviewing things from scratch again... it'll go a lot faster
< sipa> and for the fact that you coukd introduce small bur dangerous changes that could be missed on re-review... well ideally that's what you have tests for
< kallewoof> *nod*
< sipa> they don't just help guarantee that functionality won't break, they also give reviewers confidence
< kallewoof> yeah, tests are important. i felt like i had them in the signet PR because i USED to have them, before I split it up to make review easier.
< sipa> kallewoof: i think i've moved in that direction as well; for segwit, everything was tested by adding segwit support into the wallet and using that to test the consensus logic... which i think made the change much bigger than it needed to be
< gwillen> I don't know if this is something we could add to one of the workflow docs under doc/, but it's not too hard to ask git to check for you whether squashing changed anything
< sipa> for taproot there is zero wallet support, and signing isn't even implemented in the consensus pr... but there are tests in python that just reimplement it
< gwillen> it's a bit fiddly, but now that github allows checking out arbitrary commits, including ones that were rebased-away, it's more straightforward
< kallewoof> sipa: yeah, i noticed! it makes sense, but i was surprised when i was trying to use it in btcdeb/tap :)
< sipa> kallewoof: right, i see how that complicates things :)
< kallewoof> gwillen: i am actually confused why git(hub) doesn't have a way to just compare changes since last time. like, give me a diff of what i reviewed before and now
< kallewoof> gwillen: i hit that "submit review" button so github clearly knows exactly where i was the last time..
< gwillen> I know that gitlab makes this possible, I thought github had some way to do this
< gwillen> but at least in gitlab, the only thing you can compare is "changes since last push", so if they rebase against changes to upstream, it's useless because the diff includes all the upstream changes too
< kallewoof> yeah... having a snapshot of the state when you last reviewed and comparing that to current form would be ideal
< sipa> i think there is just less effort done on making rebase-heavy workflows easy
< gwillen> at least when it says "so-and-so force-pushed from <id> to <id>", getting a direct diff on the command line is easy
< gwillen> just 'git fetch <id>' each of them, then 'git diff <id> <id>'
< aj> i think lots of rebases is something a few open source projects do, but most/paying customers tend not to?
< gwillen> (however if that includes a rebase against new upstream changes, that command won't be useful because it will include those, the right command for that is a little more subtle)
< luke-jr> sipa: when you're splitting up the Taproot PR, I do think it would make sense to do consensus and policy separately
< gwillen> ahhh, I always forget this -- in the github UI when you see "so-and-so force-pushed", the words "force-pushed" are a link to the diff
< gwillen> they kind of hide it, but if you click there you can see the changes in the force push
< luke-jr> gwillen: the notification emails have links that I suspect are supposed to be that, but they never seem to work for me
< aj> kallewoof, sipa: so currently signet/generate.py supports signing the blocktx via either constructing a psbt, or by calling signrawtransactionwithwallet directly. any reason to keep the latter or can/should i go psbt only, do you think?
< kallewoof> aj: i think psbt only is ok
< sipa> aj: i'd do psbt only
< * aj> deletes some more code
< kallewoof> https://github.com/bitcoin/bitcoin/pull/18267/commits/cbb6f9439df8593c3aa52b56698788cc73108831 now tests OP_TRUE mining, and also submitblock checks the first 10 blocks in the current signet chain and also tries the first of those on a non-compatible (different challenge) signet chain.
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to 0.19: https://github.com/bitcoin/bitcoin/compare/28a9df7d76a6...89a6bb924571
< bitcoin-git> bitcoin/0.19 ed0223e Luke Dashjr: scheduler: Workaround negative nsecs bug in boost's wait_until
< bitcoin-git> bitcoin/0.19 89a6bb9 MarcoFalke: Merge #18284: [0.19] scheduler: Workaround negative nsecs bug in boost's w...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18284: [0.19] scheduler: Workaround negative nsecs bug in boost's wait_until (0.19...wrkarnd_boost_wait_until) https://github.com/bitcoin/bitcoin/pull/18284
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/15886b08aa5f...862fde88be70
< bitcoin-git> bitcoin/master cc26fab practicalswift: tests: Add fuzzing harness for CNode
< bitcoin-git> bitcoin/master 862fde8 MarcoFalke: Merge #19067: tests: Add fuzzing harness for CNode
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19067: tests: Add fuzzing harness for CNode (master...fuzzers-2020-05-25) https://github.com/bitcoin/bitcoin/pull/19067
< bitcoin-git> [bitcoin] jonasschnelli pushed 3 commits to 0.19: https://github.com/bitcoin/bitcoin/compare/89a6bb924571...aee9d2306ad1
< bitcoin-git> bitcoin/0.19 2ea826c Suhas Daftuar: Add txids with non-standard inputs to reject filter
< bitcoin-git> bitcoin/0.19 52c3bec Gregory Sanders: test addition of unknown segwit spends to txid reject filter
< bitcoin-git> bitcoin/0.19 aee9d23 Jonas Schnelli: Merge #19681: 0.19: Add txids with non-standard inputs to reject filter
< bitcoin-git> [bitcoin] jonasschnelli merged pull request #19681: 0.19: Add txids with non-standard inputs to reject filter (0.19...2020-08-reject-unknown-wit-0.19) https://github.com/bitcoin/bitcoin/pull/19681
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #19629: refactor: Keep mempool interface in validation (master...2007-nomem) https://github.com/bitcoin/bitcoin/pull/19629
< MarcoFalke> gwillen: Are you aware of git range-diff ? This helps re-reviewing a fixed up version of the changes, and it "hides" any rebases that happened in between.
< sipa> MarcoFalke: TIL
< MarcoFalke> Well, it can also be used to review rebases and see what manual changes were needed to rebase.
< aj> oops, looking at rust-bitcoin and misread "macro_rules!" as "marco_rules!"
< MarcoFalke> Though, it doesn't work for large rewrites... In which case you are better of reviewing from scratch anyway
< MarcoFalke> macro fake
< sipa> in c++ most uses of marcos have been replaced with templates, thankfully
< vasild> Has it been considered before importing an external library into bitcoin core as a git submodule?
< vasild> I am trying to add a stripped version of crypto++ - just the bits that do sha3-256, but it is all tied together by internal dependencies, so the process may end up importing e.g. 80% of the library source
< MarcoFalke> Which dependency is tor using?
< vasild> let me see...
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19826: Pass mempool reference to chainstate constructor (master...2008-valMemRef) https://github.com/bitcoin/bitcoin/pull/19826
< sipsorcery> vasild: it may not be of any use but this is the repo I used as a basis for a keccak hash https://github.com/ethereum/ethash/tree/master/src/libethash
< sipsorcery> I only ended up needing two classes and 4 headers and no external dependencies
< vasild> MarcoFalke: tor could use openssl as an external dependency or src/ext/keccak-tiny
< MarcoFalke> well, then copy src/ext/keccak-tiny for now ;)
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19827: init: Disallow negation of blockversion (master...2008-parseBoolBlockVersion) https://github.com/bitcoin/bitcoin/pull/19827
< wumpus> vasild: I woudl suggest finding a stand-alone implementation of SHA3, a reminder: it does not need to be efficient
< wumpus> we're not going to take up a subtree of submodule just for this
< wumpus> MarcoFalke: exactly, a tiny implementation (as far as possible for something like Keccak) is preferred here
< vasild> I gave up with crypto++ after it turned out that it is not possible to take just sha3.h and sha3.cpp from its implementation (I don't want to do any modificatoins of those imported files)
< vasild> looking at https://github.com/coruus/keccak-tiny now
< wumpus> sure, I understand you don't want to end up maintaining this, but I wouldn't say small modifications (e.g. type names) are a problem to fit it better in our repository
< wumpus> nice
< vasild> I also added a wrapper class SHA3_256 that has the same interface as our existing hashers: Write(), Finalize() and Reset() methods.
< wumpus> awesome, it's good to be consistent about that
< vasild> yeah, would also help if we decide to ditch whatever we have imported and import something else
< fanquake> That certainly looks better than random Tor code, git submodules, OpenSSL or other shared libs
< wumpus> exactly
< vasild> :-)
< wumpus> we were getting scared for a bit
< vasild> fear not! :-D
< kallewoof> Weirdly, one of my linux machines complains that ./optional.h:24 ('static auto& nullopt = boost::none;') is unused in like 40+ places; it looks correct, but none of my other machines give me that warning no matter what flags I try (the specified -Wunused-variable is already present, I think)...
< wumpus> isn't *const* missing?
< wumpus> or constexpr
< wumpus> it doesn't look right if you can assign something else to nullopt, and also, this is likely the reason you get a warning when including the file and not using nullopt
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/862fde88be70...4326515f01d7
< bitcoin-git> bitcoin/master df53688 Carl Dong: chain: Remove UB CChain comparison
< bitcoin-git> bitcoin/master 4326515 fanquake: Merge #19822: chain: Fix CChain comparison UB by removing it (it was unuse...
< bitcoin-git> [bitcoin] fanquake merged pull request #19822: chain: Fix CChain comparison UB by removing it (it was unused) (master...2020-08-chain-comparison-UB) https://github.com/bitcoin/bitcoin/pull/19822
< bitcoin-git> [bitcoin] promag opened pull request #19828: wallet, refactor: Remove duplicate map lookups in GetAddressBalances (master...2020-08-getaddressbalances) https://github.com/bitcoin/bitcoin/pull/19828
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/4326515f01d7...1dac4dcf088f
< bitcoin-git> bitcoin/master 9034f6e Hennadii Stepanov: Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR annotations
< bitcoin-git> bitcoin/master 1dac4dc fanquake: Merge #19758: Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR an...
< bitcoin-git> [bitcoin] fanquake merged pull request #19758: Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR annotations (master...200818-tsa) https://github.com/bitcoin/bitcoin/pull/19758
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/1dac4dcf088f...22acd36d5365
< bitcoin-git> bitcoin/master c91b241 Jake Leventhal: Updated outdated help command for getblocktemplate (fixes #19625)
< bitcoin-git> bitcoin/master 22acd36 Wladimir J. van der Laan: Merge #19646: doc: Updated outdated help command for getblocktemplate
< bitcoin-git> [bitcoin] laanwj merged pull request #19646: doc: Updated outdated help command for getblocktemplate (master...fix-outdated-getblocktemplate-help) https://github.com/bitcoin/bitcoin/pull/19646
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/22acd36d5365...9632b7edc748
< bitcoin-git> bitcoin/master d3e8adf Sebastian Falbesoner: util: remove c-string interfaces for DecodeBase58{Check}
< bitcoin-git> bitcoin/master 9632b7e Wladimir J. van der Laan: Merge #19739: refactor: remove c-string interfaces for DecodeBase58{Check}...
< bitcoin-git> [bitcoin] laanwj merged pull request #19739: refactor: remove c-string interfaces for DecodeBase58{Check} (master...20200811-util-remove-cstring-decodebase58) https://github.com/bitcoin/bitcoin/pull/19739
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #19827: init: Disallow negation of blockversion (master...2008-parseBoolBlockVersion) https://github.com/bitcoin/bitcoin/pull/19827
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9632b7edc748...5edef20a65f0
< bitcoin-git> bitcoin/master 7b6d0f1 Raúl Martínez (RME): Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messa...
< bitcoin-git> bitcoin/master 5edef20 MarcoFalke: Merge #19797: net: Remove old check for 3-byte shifted IP addresses from p...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19797: net: Remove old check for 3-byte shifted IP addresses from pre-0.2.9 nodes (master...patch-1) https://github.com/bitcoin/bitcoin/pull/19797
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/5edef20a65f0...ca30d34cf94b
< bitcoin-git> bitcoin/master 4227a8e Hennadii Stepanov: qt: Make "Create new receiving address" default unconditionally
< bitcoin-git> bitcoin/master 4ec49f8 Hennadii Stepanov: qt: Leverage the default "Create new receiving address" button
< bitcoin-git> bitcoin/master ca30d34 MarcoFalke: Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new receiv...
< jnewbery> wumpus: #19607 may be ready for merge if you're happy with it
< gribble> https://github.com/bitcoin/bitcoin/issues/19607 | [p2p] Add Peer struct for per-peer data in net processing by jnewbery · Pull Request #19607 · bitcoin/bitcoin · GitHub
< wumpus> jnewbery: let's see!
< jonatack> maybe #19405 too
< gribble> https://github.com/bitcoin/bitcoin/issues/19405 | rpc, cli: add network in/out connections to `getnetworkinfo` and `-getinfo` by jonatack · Pull Request #19405 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/ca30d34cf94b...1cf73fb8eba3
< bitcoin-git> bitcoin/master aba0335 John Newbery: [net processing] Remove CNodeState.name
< bitcoin-git> bitcoin/master 7cd4159 John Newbery: [net processing] Add Peer
< bitcoin-git> bitcoin/master 1f96d2e John Newbery: [net processing] Move misbehavior tracking state to Peer
< bitcoin-git> [bitcoin] laanwj merged pull request #19607: [p2p] Add Peer struct for per-peer data in net processing (master...2020-07-peer) https://github.com/bitcoin/bitcoin/pull/19607
< jonatack> Looks like merging doesn't remove a PR from Chasing Concept ACK in https://github.com/bitcoin/bitcoin/projects/8
< jonatack> :)
< jonatack> at least, not automatically
< sipa> better Concept ACK is quickly
< luke-jr> lol
< meshcollider> jonatack: indeed, same with high priority and everything, still need to manually remove after merge
< meshcollider> #startmeeting
< lightningbot> Meeting started Fri Aug 28 19:00:12 2020 UTC. The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< meshcollider> #bitcoin-core-dev Wallet 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 ariard digi_james amiti fjahr
< meshcollider> jeremyrubin emilengler jonatack hebasto jb55
< provoostenator> hi
< achow101> hi
< meshcollider> Topics this week?
< provoostenator> No topics from me. I rebased a bunch of things.
< meshcollider> There are a few PRs that need review (from myself included) at the moment
< kanzure> hi
< achow101> just waiting on some reviews
< provoostenator> And just got back from vacation, so will do more review soon(tm)
< luke-jr> just a few? :P
< meshcollider> Always a few :)
< achow101> soon(tm)
< meshcollider> So hopefully we can tick off some bigger things this upcoming week
< provoostenator> Yeah, SQLite, Taproot wallet, OP_SECURE_THE_BAG support, should be doable.
< meshcollider> :D
< achow101> don't forget hardware wallets
< provoostenator> Can we get fast rescan while we're at it?
< sipa> paypal integration?
< provoostenator> I keep forgetting to disable debug mode when I open ancient wallets
< jonatack> hi
< provoostenator> Or maybe just a way to turn off debug mode without recompiling :-)
< achow101> I'd like to shill for #19619
< gribble> https://github.com/bitcoin/bitcoin/issues/19619 | Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp by ryanofsky · Pull Request #19619 · bitcoin/bitcoin · GitHub
< meshcollider> achow101: I am mid-way through reviewing that
< achow101> as that's a requirement for sqlite wallets now
< meshcollider> But Marco's comment needs addressing anyway
< sipa> provoostenator: what is debug mode?
< provoostenator> --enable-debug
< provoostenator> Which slows rescan down enourmously
< achow101> I think phantomcircuit had something for rescanning on the block filters
< achow101> or was working on something for that
< provoostenator> I think MarcoFalke had a PR, but closed it.
< provoostenator> Indeed using filters
< provoostenator> There were some hairy issues around refilling the keypool and updating the filters.
< jonatack> I plan to get to the #11413 follow-ups soon, e.g. tests, cleanups, and universal explicit feerate argument for RPCs. Needed before feature freeze.
< gribble> https://github.com/bitcoin/bitcoin/issues/11413 | [wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option by kallewoof · Pull Request #11413 · bitcoin/bitcoin · GitHub
< jonatack> RN we're overriding conf_target, which is a little confusing/scary
< provoostenator> jonatack: nice, feel free to tag me on it
< jonatack> will do
< meshcollider> Yeah, #15845
< gribble> https://github.com/bitcoin/bitcoin/issues/15845 | wallet: Fast rescan with BIP157 block filters by MarcoFalke · Pull Request #15845 · bitcoin/bitcoin · GitHub
< provoostenator> Also let me know if there's anything in #16378 that needs changing for your vision of universal fee rates.
< jonatack> provoostenator: enable-debug slows down everything by ~half for me anyway
< gribble> https://github.com/bitcoin/bitcoin/issues/16378 | The ultimate send RPC by Sjors · Pull Request #16378 · bitcoin/bitcoin · GitHub
< provoostenator> More like 10x for rescan
< provoostenator> But I can never be bothered to stop.
< jonatack> provoostenator: mentioned that PR to moneyball recently, will review/test it
< meshcollider> Ok so basically #action review lots!
< achow101> just review every PR tagged wallet
< jonatack> (ultimate send rpc)
< meshcollider> #endmeeting
< lightningbot> Meeting ended Fri Aug 28 19:14:22 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< provoostenator> One reason I made that PR is that people didn't like overloading arguments like conf_target even more
< jonatack> I should review 16378 asap actually
< jnewbery> thanks wumpus!
< achow101> wen miniscript?
< sipa> i'll get back to it some time; too many other things to work on right now
< phantomcircuit> achow101, i have something but kept getting frustrated because the organization of the gcs block filter stuff just seems wrong, it's really hard to extend without h4x
< ariard> sdaftuar: we may have another slight improvement to eviction logic after #19670, if our slots are full we actually reject inbound connection from past IsDiscouraged peers
< gribble> https://github.com/bitcoin/bitcoin/issues/19670 | Protect localhost and block-relay-only peers from eviction by sdaftuar · Pull Request #19670 · bitcoin/bitcoin · GitHub
< ariard> but we don't pick them up as prioritized candidates for eviction if they're already connected to us and we get a new inbound
< ariard> as a first heuristic we could iterate on all IsDiscouraged and randomly pick out for eviction, thus biasing towards not-known-misbehaving peers
< sipa> ariard: discouraged peers have m_prefer_evict set, which goes into the eviction logic