💬 dergoegge commented on pull request "refactor: Introduce EvictionManager and use it for the inbound eviction logic":
(https://github.com/bitcoin/bitcoin/pull/25572#discussion_r1165282299)
> Repeatedly looking up the eviction manager map and locking and unlocking access to it

Two things:
* There are very few full outbound or block relay only connections, so this happens at most maybe 3 or 4 times for the block relay only loop and 11 or 12 times for the full outbound one.
* The outbound eviction logic is also moved to the eviction manager in a follow up (see #25268), which removes this repeated locking.

> and introducing the potential for maps to be out of sync and having t
...

📝 hebasto opened a pull request: "qt: Register `wallet::AddressPurpose` type"
(https://github.com/bitcoin-core/gui/pull/726)
This PR is a follow up of bitcoin/bitcoin#27217.

Fixes #725.

💬 hebasto commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1165367412)
Fixed in https://github.com/bitcoin-core/gui/pull/726.

💬 fanquake commented on pull request "ci: explicitly install libclang-rt-dev in valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27459#issuecomment-1506784122)
> Maybe https://github.com/bitcoin/bitcoin/pull/27436#issuecomment-1506443972 is related?

I'm pretty certain this is https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, which isn't related to this change, but seems to have been broken in Debian for libc++ & fuzzing for LLVM versions 12 through 16. I'll file a new issue with the libc++ 16 package, and debug futher.

🚀 fanquake merged a pull request: "ci: explicitly install libclang-rt-dev in valgrind jobs"
(https://github.com/bitcoin/bitcoin/pull/27459)

📝 MarcoFalke opened a pull request: "rpc: Add importmempool RPC "
(https://github.com/bitcoin/bitcoin/pull/27460)
Currently it is possible to import a mempool by placing it in the datadir and starting the node. However this has many issues:

* Users aren't expected to fiddle with the datadir, possibly corrupting it
* An existing mempool file in the datadir may be overwritten
* The node needs to be restarted
* Importing an untrusted file this way is dangerous, because it can corrupt the mempool

Fix all issues by adding a new RPC.

💬 glozow commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1506796551)
> > Adding a loadmempool RPC seems to be exactly what we want here?
>
> concept ACK on this, if that's the only use-case. We should probably offer a similar service over RPC first, in addition to information gathering that it's not actually being used in meaningful amounts in production. I've never come across usage myself in my production work.

#27460

💬 Sjors commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1506861773)
TIL you can quite easily verify from the clipboard (at least on macOS, probably on Linux too). No need to put things in a file. `gpg --verify` will show a prompt. Paste everything from `-----BEGIN` to the end, hit enter and then `ctrl + d`.

💬 Sjors commented on issue "Write instructions on offline signing.":
(https://github.com/bitcoin/bitcoin/issues/9492#issuecomment-1506868821)
I think that with PSBT there's no need to go out of our way to document the legacy `signrawtransaction`.

The external signer docs kind of assume a connected device / service, so not really "offline". The PSBT doc could probably be update to use descriptor wallets rather than `createmultisig`, though creating descriptors by hand is a bit tedious at the moment.

The [multisig tutorial](https://github.com/bitcoin/bitcoin/blob/master/doc/multisig-tutorial.md) is more up to date, uses descriptor
...

👍 furszy approved a pull request: "Register `wallet::AddressPurpose` type"
(https://github.com/bitcoin-core/gui/pull/726#pullrequestreview-1383357368)
Tested ACK a45b54406dbce4fbf8a316a0e91615eb480da653

💬 MarcoFalke commented on pull request "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1165491936)
For reference, this fails with:

```
test/sighash_tests.cpp(163): Leaving test case "sighash_from_data"; testing time: 3749686us
test/sighash_tests.cpp(120): Entering test case "sighash_test"
==21825== Source and destination overlap in memcpy(0x8704270, 0x8704270, 36)
==21825== at 0x488CFA0: __GI_memcpy (in /usr/libexec/valgrind/vgpreload_memcheck-arm64-linux.so)
==21825== by 0x895673: CTxIn::operator=(CTxIn const&) (transaction.h:74)
==21825== by 0x8DD22F: SignatureHashOld(CScr
...

💬 pinheadmz commented on issue "Write instructions on offline signing.":
(https://github.com/bitcoin/bitcoin/issues/9492#issuecomment-1506936881)
@Sjors maybe something like this in a new doc with create/sign tx commands added?

https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1487425971

💬 fanquake commented on pull request "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1165506294)
Yea this, and the other failure in the benches you'll see, if you suppress this one (see below), were already known, and the whole reason I wanted to move to 3.20, rather than continuing to fight with broken tools. I'll open the build from source PR.

```bash
Running bench/bench_bitcoin (one iteration sanity check, only high priority)...
bench/bench_bitcoin -sanity-check -priority-level=high
Running with -sanity-check option, output is being suppressed as benchmark results will be useless.
...

💬 achow101 commented on pull request "Register `wallet::AddressPurpose` type":
(https://github.com/bitcoin-core/gui/pull/726#issuecomment-1506961246)
ACK a45b54406dbce4fbf8a316a0e91615eb480da653

💬 MarcoFalke commented on pull request "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1165532774)
Looks like gcc works fine, see also https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1163983711. Though, this won't help with the fuzz task.

✅ hebasto closed an issue: "Crash using getnewaddress in the console"
(https://github.com/bitcoin-core/gui/issues/725)

🚀 hebasto merged a pull request: "Register `wallet::AddressPurpose` type"
(https://github.com/bitcoin-core/gui/pull/726)

💬 ajtowns commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1507067647)
> Right, the changes for package relay only (i.e. no v3, no package RBF) would be this, persist packages across restart, and allow any ancestor package instead of just child-with-parents.

Sorry, I more just mean "what's the minimal set of additional commits that need this that results in something that's useful on its own?"

Maybe the current overarching "useful thing" is just "package mempool acceptance works in regtest via rpc in a way that would be acceptable to expose on mainnet for pac
...

💬 MarcoFalke commented on pull request "ci: set docker run --ulimit to workaround Valgrind assertion":
(https://github.com/bitcoin/bitcoin/pull/27364#issuecomment-1507077524)
I tried to reproduce on fedora 37 on current master, and it passed with podman

💬 MarcoFalke commented on pull request "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1165620563)
The fuzz task seems to be passing, see https://github.com/bitcoin/bitcoin/pull/27364#issuecomment-1507077524, so no need to bump valgrind. You can simply use gcc:

```diff
diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh
index 97b85755e..794a2dcca 100755
--- a/ci/test/00_setup_env_native_valgrind.sh
+++ b/ci/test/00_setup_env_native_valgrind.sh
@@ -8,10 +8,10 @@ export LC_ALL=C.UTF-8

export CI_IMAGE_NAME_TAG="debian:bookworm"
export CON
...