< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/b54a10e777f9...56f06a986385
< bitcoin-git> bitcoin/master d98f459 Carl Dong: guix: Explicitly set umask in build container
< bitcoin-git> bitcoin/master 56f06a9 fanquake: Merge #21271: guix: Explicitly set umask in build container
< bitcoin-git> [bitcoin] fanquake merged pull request #21271: guix: Explicitly set umask in build container (master...2021-02-guix-umask) https://github.com/bitcoin/bitcoin/pull/21271
< bitcoin-git> [bitcoin] dongcarl opened pull request #21298: guix: Bump time-machine, glibc, and linux-headers (master...2021-02-bump-guix-and-glibc) https://github.com/bitcoin/bitcoin/pull/21298
< bitcoin-git> [gui] MarcoFalke merged pull request #219: qt: Prevent the main window popup menu (master...210223-toolbar) https://github.com/bitcoin-core/gui/pull/219
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/56f06a986385...434065a48344
< bitcoin-git> bitcoin/master ca5bd1c Hennadii Stepanov: qt: Prevent the main window popup menu
< bitcoin-git> bitcoin/master 434065a MarcoFalke: Merge bitcoin-core/gui#219: qt: Prevent the main window popup menu
< bitcoin-git> [gui] MarcoFalke merged pull request #214: qt: Disable requests context menu actions when appropriate (master...disable-contextactions-novalue) https://github.com/bitcoin-core/gui/pull/214
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/434065a48344...09bc7bfed1e6
< bitcoin-git> bitcoin/master bb3da8f Jarol Rodriguez: qt: Disable requests context menu actions when appropriate
< bitcoin-git> bitcoin/master 09bc7bf MarcoFalke: Merge bitcoin-core/gui#214: qt: Disable requests context menu actions when...
< bitcoin-git> [gui] MarcoFalke merged pull request #223: qt: Re-add and rename transaction "Edit Label" action (master...2021-02-readd-edit-label) https://github.com/bitcoin-core/gui/pull/223
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/09bc7bfed1e6...8ca6bd0dac8f
< bitcoin-git> bitcoin/master 22664d6 Wladimir J. van der Laan: Revert "qt: Remove Transactionview Edit Label Action"
< bitcoin-git> bitcoin/master 5440c07 Wladimir J. van der Laan: qt: Rename "Edit label" to "Edit address label"
< bitcoin-git> bitcoin/master 8ca6bd0 MarcoFalke: Merge bitcoin-core/gui#223: qt: Re-add and rename transaction "Edit Label"...
< bitcoin-git> [bitcoin] laanwj pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/8ca6bd0dac8f...2059d32edbd0
< bitcoin-git> bitcoin/master faabce7 MarcoFalke: test: Start only the number of nodes that are needed
< bitcoin-git> bitcoin/master fad2515 MarcoFalke: test: Remove unused bug workaround
< bitcoin-git> bitcoin/master fa4d8f3 MarcoFalke: test: Cache 25 mature coins for ADDRESS_BCRT1_P2WSH_OP_TRUE
< bitcoin-git> [bitcoin] laanwj merged pull request #21200: test: Speed up rpc_blockchain.py by removing miniwallet.generate() (master...2102-testSpeedUp) https://github.com/bitcoin/bitcoin/pull/21200
< bitcoin-git> [bitcoin] jonatack closed pull request #21297: test: `feature_blockfilterindex_prune.py` improvements (master...feature_blockfilterindex_prune-improvements) https://github.com/bitcoin/bitcoin/pull/21297
< jonatack> vasild: working on finishing review of #20685 (would be great to get it in relatively early in this release cycle)
< gribble> https://github.com/bitcoin/bitcoin/issues/20685 | Add I2P support using I2P SAM by vasild · Pull Request #20685 · bitcoin/bitcoin · GitHub
< vasild> thanks! unrelated to that, I got to #20197 in my to-(re)review list :)
< gribble> https://github.com/bitcoin/bitcoin/issues/20197 | p2p: protect onions in AttemptToEvictConnection(), add eviction protection test coverage by jonatack · Pull Request #20197 · bitcoin/bitcoin · GitHub
< vasild> hm, since you mentioned the I2P PR, I have been thinking about adding fuzz tests to it and I think it will be necessary to change the data members from Sock to Sock* - will be purely mechanical if needed
< vasild> Sock* or std::unique_ptr<Sock>
< vasild> this is because right now a test cannot "replace" the Sock member with FuzzedSock (which inherits Sock)
< jonatack> vasild: thanks! above all, looking for feedback on the protection allocation ratios between regular, onion, localhost (and soon i2p, and maybe cjdns) peers, i went for the simplest thing but it maybe protects too many localhost and onion peers
< vasild> but if the main code uses pointers and a global function "create a Sock for me and give me a pointer to it", then a test could replace that global function with another one which returns a pointer to FuzzedSock
< vasild> (I did not test the above but am 99% sure that this will be the case)
< vasild> wumpus: ^
< jonatack> vasild: no worries. do you think it should be done in 20685?
< vasild> wumpus: since you ACK'ed it already, just a headsup that maybe a Sock -> Sock* mechanical change is coming in order to accomodate fuzzing :)
< vasild> not strictly necessary, but would be good to avoid a followup PR, will see once I have the code
< jonatack> diff shouldn't be too bad if mechanical
< vasild> yes, I imagine something like s/Sock/std::unique_ptr<Sock>/ and s/sock./sock->/ and a difference when constructing Sock objects
< hebasto> ^ join to improve it :)
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/2059d32edbd0...1b1d8bde1c94
< bitcoin-git> bitcoin/master fa560cc MarcoFalke: test: Intermittent issue in feature_blockfilterindex_prune
< bitcoin-git> bitcoin/master 1b1d8bd MarcoFalke: Merge #21252: test: Add missing wait for sync to feature_blockfilterindex_...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #21252: test: Add missing wait for sync to feature_blockfilterindex_prune (master...2102-testFix) https://github.com/bitcoin/bitcoin/pull/21252
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/1b1d8bde1c94...cac10e66d230
< bitcoin-git> bitcoin/master fa4fbec MarcoFalke: scripted-diff: Rename PROVIDE_MAIN_FUNCTION -> PROVIDE_FUZZ_MAIN_FUNCTION
< bitcoin-git> bitcoin/master fae216a MarcoFalke: scripted-diff: Rename MakeFuzzingContext to MakeNoLogFileContext
< bitcoin-git> bitcoin/master cac10e6 MarcoFalke: Merge #21264: fuzz: Two scripted diff renames
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #21264: fuzz: Two scripted diff renames (master...2102-fuzzingRenames) https://github.com/bitcoin/bitcoin/pull/21264
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/cac10e66d230...c0e44ee8e4ba
< bitcoin-git> bitcoin/master fa1b713 MarcoFalke: test: Assume node is running in subtests
< bitcoin-git> bitcoin/master fa730e9 MarcoFalke: test: Avoid connecting to real network when running tests
< bitcoin-git> bitcoin/master c0e44ee MarcoFalke: Merge #21254: test: Avoid connecting to real network when running tests
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #21254: test: Avoid connecting to real network when running tests (master...2102-testNoTelemetry) https://github.com/bitcoin/bitcoin/pull/21254
< bitcoin-git> [gui] MarcoFalke merged pull request #226: Add "Last Block" and "Last Tx" rows to peer details area (master...add-last-block-and-last-transaction-to-peer-details) https://github.com/bitcoin-core/gui/pull/226
< bitcoin-git> [bitcoin] MarcoFalke pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/c0e44ee8e4ba...e49117470b77
< bitcoin-git> bitcoin/master 4dc2fd6 Jon Atack: qt: add RPCConsole::TimeDurationField helper, call systime only once
< bitcoin-git> bitcoin/master a21be7c Jon Atack: gui: add "Last Tx" (CNodeStats::nLastTXTime) to peer details
< bitcoin-git> bitcoin/master 70d3c5d Jon Atack: gui: add "Last Block" (CNodeStats::nLastBlockTime) to peer details
< bitcoin-git> [bitcoin] jonatack reopened pull request #21297: test: `feature_blockfilterindex_prune.py` improvements (master...feature_blockfilterindex_prune-improvements) https://github.com/bitcoin/bitcoin/pull/21297
< wumpus> #startmeeting
< core-meetingbot> Meeting started Thu Feb 25 19:00:09 2021 UTC. The chair is wumpus. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
< core-meetingbot> Available commands: action commands idea info link nick
< achow101> hi
< hebasto> hi
< amiti> hi
< wumpus> #bitcoin-core-dev Meeting: achow101 aj amiti ariard bluematt cfields Chris_Stewart_5 digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik
< wumpus> petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild wumpus
< meshcollider> hi
< wumpus> it doesn't look like any topics have been proposed this week, any last minute ones ?
< sipa> hi
< sdaftuar> hi
< fjahr> hi
< wumpus> (fwiw, if you want to propose a meeting topic during the week use #proposedmeetingtopic <topic> and it will appear in http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt)
< wumpus> #topic High priority for review
< core-meetingbot> topic: High priority for review
< wumpus> https://github.com/bitcoin/bitcoin/projects/8 7 blockers, 2 chasing concept ACK
< wumpus> congrats to provoostenator for getting the external wallet signer merged
< amiti> can I add #21061 to the list?
< wumpus> anything to add/remove or ready for merge?
< gribble> https://github.com/bitcoin/bitcoin/issues/21061 | [p2p] Introduce node rebroadcast module by amitiuttarwar · Pull Request #21061 · bitcoin/bitcoin · GitHub
< fjahr> I would like to add #19521
< gribble> https://github.com/bitcoin/bitcoin/issues/19521 | Coinstats Index by fjahr · Pull Request #19521 · bitcoin/bitcoin · GitHub
< wumpus> amiti: added!
< amiti> wumpus: thank you :)
< achow101> I'd like to replace #17331 with #21083
< gribble> https://github.com/bitcoin/bitcoin/issues/17331 | Use effective values throughout coin selection by achow101 · Pull Request #17331 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/21083 | wallet: Avoid requesting fee rates multiple times during coin selection by achow101 · Pull Request #21083 · bitcoin/bitcoin · GitHub
< jonatack> hi
< wumpus> fjahr: also added
< fjahr> wumpus: thanks!
< wumpus> achow101: done
< jnewbery> hi
< jamesob> hi
< wumpus> anything else for this topic? any other topics to discuss?
< achow101> topic suggestion: testing strategy for descriptor and legacy wallets
< emzy> hi
< achow101> (perhaps this is more suited for the wallet meeting next week)
< wumpus> achow101: okay, up to you
< achow101> if there's nothing else to discuss
< wumpus> i'm fine with either, we can discuss it now otherwise it's the end of the meeting
< wumpus> ok
< wumpus> #topic Testing strategy for descriptor and legacy wallets (achow101)
< core-meetingbot> topic: Testing strategy for descriptor and legacy wallets (achow101)
< achow101> Currently I find the way that we do tests for both descriptor and legacy wallets a bit halfassed and not that great
< achow101> so I had opened #20892 which runs both descriptor and legacy wallet variants of a test at a time
< gribble> https://github.com/bitcoin/bitcoin/issues/20892 | tests: Run both descriptor and legacy tests within a single test invocation by achow101 · Pull Request #20892 · bitcoin/bitcoin · GitHub
< achow101> however ryanofsky disagrees with this approach
< achow101> what other options or suggestions do people have for running tests that should be run on both descriptor and legacy wallets?
< wumpus> if the parts are completely independent, why not split it up
< achow101> the particular issue is with tests that should be run on both
< wumpus> e.g. have the test run with --legacy and --descriptor-wallet based on what is enabled
< wumpus> as separate invocations
< luke-jr> isn't that how it works now?
< achow101> that's how it works how
< wumpus> what's wrong with that then ?
< achow101> but if I am running a test without the test runner, that's 2 commands and not everyone who is working on wallet stuff may realize that
< wumpus> well add a --both for that maybe?
< wumpus> for manual use
< luke-jr> default being both seems fine for manual use
< wumpus> yes
< luke-jr> test runner can still specify them separately
< achow101> that seems reasonable
< wumpus> i mean if it saves you hassle during development (don't forget to document it) then why not
< achow101> there are also a number of test cases within some of the tests that are wallet type specific that seem like there should be an indicator of being skipped
< achow101> but I think it would be better to split those into separate tests, although that is a non trivial task
< jnewbery> I think ideally we only want this behavoiur for wallet_ tests (tests that are explicitly testing the wallets), and not the tests that incidentally use the wallet for doing things like constructing transactions
< luke-jr> achow101: eh, we don't "skip" tests that don't make sense :P
< wumpus> that's why i prefer the split approach for the test runner, it's easier to indicate what is skipped
< luke-jr> feature_namecoin_integration [SKIP]
< luke-jr> :P
< achow101> jnewbery: yes. we should encourage MiniWallet use
< wumpus> jnewbery: agree, we don't want to end up running everything twice if the point is not the wallet
< achow101> the problem with some tests incidentally using the wallet is that determining the type of wallet to create in that case is pretty annoying since we have to take into account what wallet support is compiled
< luke-jr> achow101: the framework can deal with that easily enough
< jnewbery> yes, we should, but there are still tests that use the wallet incidentally and it'd be a shame to make them run for twice as long in the mean time
< achow101> luke-jr: it's a bit messy
< luke-jr> why?
< wumpus> maybe add a preferred_oneoff_wallet_type() or so
< achow101> as in, the logic currently for dealing with that is a bit messy imo
< luke-jr> incidental use shouldn't care which kind it gets
< wumpus> (until it's all switched to MiniWallet ofc)
< sipa> are there any blockers for miniwallet to be used more? or is it just the effort of transitioning the tests to use it?
< wumpus> I think the bottleneck is review
< sipa> of course.
< wumpus> there's a lot of PRs open that convert tests to miniwallet IIRC
< achow101> there are a surprising number of tests that use the wallet incidentally
< wumpus> all the "run XXX even with wallet disabled" PRs
< sipa> i was thinking about moving the generic signing framework in feature_taproot to be in the test_framework and making it more generally usable (as it has rough code for signing basically anything)
< luke-jr> what is wrong with using the wallet incidentally? just that it means we don't test it on non-wallet builds?
< wumpus> the issue for this is #20078
< gribble> https://github.com/bitcoin/bitcoin/issues/20078 | test: Convert non-wallet tests to use our python MiniWallet · Issue #20078 · bitcoin/bitcoin · GitHub
< jnewbery> At some point in the future (perhaps when multiprocess is merged), would it make sense for the wallet_ tests to not use the full test framework, but instead to have the wallet interact with a mock node?
< wumpus> luke-jr:that's another advantage yes
< achow101> luke-jr: depending on the test, sometimes there is no problem, and other times the test relies on some specific behavior of the wallet that is not guaranteed across types
< achow101> e.g. there's a test for doing something with conflicts which relies on sethdseed which is not available for descriptor wallets
< achow101> that particular test isn't doing anything with the wallet except needing the same privkeys on 2 nodes
< luke-jr> XD
< achow101> in any case, it seems like i should keep the test runner as it is and just make the run twice behavior for manual test invocation
< jnewbery> only for wallet_ tests
< wumpus> right the two wallets are not entirely interchangable and that can result in ugliness
< achow101> yes, only for wallet tests
< jnewbery> SGTM
< wumpus> SGTM too
< * luke-jr> wonders how difficult it would be at this point to add a new wallet type that is just a wrapper around an external Lightning wallet
< meshcollider> 👍
< wumpus> any other topics?
< wumpus> #endmeeting
< core-meetingbot> topic: Bitcoin Core development discussion and commit log | Feel free to watch, but please take commentary and usage questions to #bitcoin | Channel logs: http://www.erisian.com.au/bitcoin-core-dev/, http://gnusha.org/bitcoin-core-dev/ | Meeting topics http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt / http://gnusha.org/bitcoin-core-dev/proposedwalletmeetingtopics.txt
< core-meetingbot> Meeting ended Thu Feb 25 19:26:50 2021 UTC.