💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1488767884)
@S3RK I agree that Sjors' PR does what we want (and more), up to the specific import format, but

* It has not been updated in 18 months and now has many conflicts
* It depends on two other unmerged PRs, one of which is marked draft
* It is itself marked draft
* It is 30 commits long and requires significant reviewer attention

Meanwhile this PR is narrow in scope, doesn't even touch the actual wallet code and therefore won't conflict with ongoing mega-refactors, and is currently up to da
...

📝 fanquake opened a pull request: "guix: use python-minimal (3.9)"
(https://github.com/bitcoin/bitcoin/pull/27361)
This further minifies the Guix release build environment.

📝 fanquake opened a pull request: "test: remove `GetRNGState` lsan suppression"
(https://github.com/bitcoin/bitcoin/pull/27362)
I am no-longer seeing this, testing with the native_asan job over `x86_64` (Ubuntu 22.04) and `aarch64` (Fedora 37). Can anyone recreate the false-positive?

Based on #27360 (which works locally; unclear what we are doing with Cirrus), which I need for working aarch64 builds.

💬 dergoegge commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1152094594)
```suggestion
TRACE6(net, evicted_inbound_connection,
```

The logic in `net` exclusively evicts inbound peers and there is more eviction logic in `net_processing` for outbound connections. Maybe adding separate trace points for both makes sense?

📝 fanquake opened a pull request: "ci: use LLVM/clang-16 in native_fuzz (ASAN) job"
(https://github.com/bitcoin/bitcoin/pull/27363)
Similar to #27298.

💬 fanquake commented on pull request "test: remove `GetRNGState` lsan suppression":
(https://github.com/bitcoin/bitcoin/pull/27362#issuecomment-1488816059)
Switched to basing on #27363.

💬 fanquake commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488816966)
native_fuzz split into #27363, as that is not blocked up upstream image issues/decisions.

💬 hebasto commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152104910)
Before this change the value of `m_chainman.m_best_header` was synced with the `UpdateBlockAvailability()` call. Now they are not. Is it safe?

💬 hebasto commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152127765)
That's unfortunate that we call a logging function while holding a lock. Maybe
```diff
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2697,10 +2696,15 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c
void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer,
const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers)
{
- if (peer.nUnconnectingHeaders > 0) {
- LogPrint(
...

👍 brunoerg approved a pull request: "addrman: Enable selecting addresses by network"
(https://github.com/bitcoin/bitcoin/pull/27214)
re-ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6

💬 hebasto commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152158505)
nm, it will cause double lock.

💬 TheCharlatan commented on pull request "guix: use python-minimal (3.9)":
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1488878073)
Concept ACK

The `python-minimal` derivation is defined in https://github.com/guix-mirror/guix/blob/v1.4.0/gnu/packages/python.scm. The `python-minimal` package inherits from the `python-3`, which in turn inherits from `python-2.7`. If I am reading this correctly, the baseline `python` package requires the following input dependencies: `bzip2, expat, gdbm, libffi, sqlite, openssl, readline, zlib, tcl, tk`, while the `python-minimal` package requires `expat, libffi, openssl, zlib`. Seems like a
...

💬 dergoegge commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1152170209)
Yes that is fine because `m_best_header` is not used by `UpdateBlockAvailability` afaict.

📝 fanquake opened a pull request: "ci: set docker run --ulimit to workaround Valgrind assertion"
(https://github.com/bitcoin/bitcoin/pull/27364)
Running the `native_fuzz_with_valgrind_job`, on aarch64 (Fedora 37), I've seen the following:
```bash
Run addr_info_deserialize with args ['valgrind', '--quiet', '--error-exitcode=1', '/home/fedora/ci_scratch/ci/scratch/build/bitcoin-aarch64-unknown-linux-gnu/src/test/fuzz/fuzz', '-runs=1', '/home/fedora/ci_scratch/ci/scratch/qa-assets/fuzz_seed_corpus/addr_info_deserialize']
valgrind: m_libcfile.c:66 (vgPlain_safe_fd): Assertion 'newfd >= VG_(fd_hard_limit)' failed.


valgrind: m_libcfile
...

💬 glozow commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1488942459)
> @glozow want to take a look at the mempool tests?

LGTM, makes sense to generate 1 to clear mempool after subtests. changing who calls `generate` shouldn't impact mempool_package_limits.
I haven't taken the time to verify that this fixes the test intermittent failure.

💬 Tracachang commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1488988174)
@pinheadmz

I have followed all the steps using the bash commands from your answer and I am unable to create a receiving taproot address from the GUI (it does not show bech2m but it shows both base58 (legacy & p2sh-segwit and bech32) if I select the watch_wallet, however if I select the "original_wallet" it appears in GUI all 4 including bech32m.

In any case, from console I can `getnewaddress "test" "bech32m"` and it works but for some reason not on GUI.

💬 TheCharlatan commented on pull request "guix: use python-minimal (3.9)":
(https://github.com/bitcoin/bitcoin/pull/27361#issuecomment-1489001205)
ACK d0e571ebb187

```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
53aa12b7fe92d92eaf81a51a8f5d534f92a2f18f0218f29fdf3221d817a53c04 guix-build-d0e571ebb187/output/aarch64-linux-gnu/SHA256SUMS.part
01467530b06b90114561e86968710250e3648fe4aba5c228781ccf9ac72394c2 guix-build-d0e571ebb187/output/aarch64-linux-gnu/bitcoin-d0e571ebb187-aarch64-linux-gnu-debug.tar.gz
fdf6dfc901e371b4e11a7f69eb2ff9b36a3ac393255862bad1dcd45b
...

💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1489034354)
> I found a few other similar mechanisms in other modules, dunno if this change should apply to them as well

Circling back for completeness to cover these two questions.

> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/validation.cpp#L2565-L2569

This code appends warning messages to the `UpdateTipLog()` logging and looks correct.

> https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/warnings.cpp#L55-L57

The `<h
...

👍 Ayush170-Future approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT"
(https://github.com/bitcoin/bitcoin/pull/27333)

💬 furszy commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1489236967)
Before the text; this is not a blocking comment. Purely about adding some ideas.

> I'm still slightly hesitating though. One question that still bothers me is whether we should expose `blank` flag at all. It's an implementation detail and we only need it for the functional tests. It doesn't seem like there is any observable by the user behaviour of the flag, so why test it with functional tests? Unit tests are better fit for testing internal invariants.

Hmm, I also agree that exposing an i
...