💬 fanquake commented on issue "feature_pruning.py failed on macOS":
(https://github.com/bitcoin/bitcoin/issues/27092#issuecomment-1427935704)
Thanks. However these are more than likely due to resource contention, or issues on your local machine. I assume the same failure doesn't happen if you run the test directly?

💬 MarcoFalke commented on issue "feature_index_prune.py failed on macOS":
(https://github.com/bitcoin/bitcoin/issues/27091#issuecomment-1427951662)
Maybe the default timeout can be increased for this one?

💬 MarcoFalke commented on issue "feature_pruning.py failed on macOS":
(https://github.com/bitcoin/bitcoin/issues/27092#issuecomment-1427952667)
Let's discuss in #27092

✅ MarcoFalke closed an issue: "feature_pruning.py failed on macOS"
(https://github.com/bitcoin/bitcoin/issues/27092)

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1427954058)
Concept ACK.

💬 fanquake commented on pull request "refactor, wallet: Avoid variable shadowing":
(https://github.com/bitcoin/bitcoin/pull/27087#issuecomment-1427955686)
There are currently multiple instances of `-Wshadow` warnings emmited when compiling the codebase. Is there some reason this one specifically needs fixing? Otherwise this just introduces a pointless merge conflict to a number of very recently-rebased PRs.

💬 fanquake commented on issue "Check usages of `#if defined(...)`":
(https://github.com/bitcoin/bitcoin/issues/16419#issuecomment-1427966591)
> It is. See https://github.com/bitcoin/bitcoin/pull/16547.

It isn't. See #25302.

💬 fanquake commented on pull request "build: Check usages of #if defined(...)":
(https://github.com/bitcoin/bitcoin/pull/25302#issuecomment-1427967375)
@brokenprogrammer sorry for not following up. Changes should always be squashed. I will get to reviewing this shortly.

💬 petertodd commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1427970070)
@dergoegge

> I would expect anyone to audit their own choice of that hash as well

Notice how in the assume-UTXO work, we've chosen to not even give users the ability to choose their own assumed-valid UTXO set. Instead, the hashes are fixed by Bitcoin Core because we don't trust users to audit their own choice of UTXO set.

@naumenkogs assumeutxo is much more likely to "help node running culture" in terms of getting more pruned nodes up and running. And it has the advantage of having muc
...

💬 fanquake commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1104505309)
Done

💬 fanquake commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1104505445)
Done

💬 furszy commented on pull request "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound":
(https://github.com/bitcoin/bitcoin/pull/26441#discussion_r1104502606)
I don't think that making the members public is good, it breaks the class encapsulation and allows multiple threads to access/modify the fields without any contention (read/write operations should be guarded).

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104509569)
1. Yes, part of me wanted to add some exception handling here, however the [original code](https://github.com/bitcoin/bitcoin/blob/master/src/util/system.cpp#L435-L444) this was factored out of did not have any, and so my thinking was that this would be OK to leave as-is, happy to be persuaded otherwise though...
2. When run with `net_specific = false`, this will return the general `DataDir`, e.g. `$HOME/.bitcoin` and with `true` will return the network-specific DataDir e.g. `$HOME/.bitcoin/reg
...

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104510651)
Totally agree there could be two wrapper functions here if desired, `EnsureDataDir` and `EnsureNetDataDir` perhaps? It might just means more duplicated code in the end though?

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104511719)
Now addresses in 7b69b99f9

💬 rex4539 commented on issue "feature_index_prune.py failed on macOS":
(https://github.com/bitcoin/bitcoin/issues/27091#issuecomment-1427993898)
```
o% PYTHON_DEBUG=1 ./test/functional/test_runner.py feature_index_prune.py
Temporary test directory at /var/folders/pn/rswd5k6175d02hbt25tvl47r0000gn/T/test_runner_₿_🏃_20230213_155540
Running Unit Tests for Test Framework Modules
..........
----------------------------------------------------------------------
Ran 10 tests in 0.746s

OK
Remaining jobs: [feature_index_prune.py]
..........................................................................................................
...

✅ rex4539 closed an issue: "feature_index_prune.py failed on macOS"
(https://github.com/bitcoin/bitcoin/issues/27091)

💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104520762)
> 2\. When run with `net_specific = false`, this will return the general `DataDir`, e.g. `$HOME/.bitcoin` and with `true` will return the network-specific DataDir e.g. `$HOME/.bitcoin/regtest`

> 2\. When run with `net_specific = false`, this will return the general `DataDir`, e.g. `$HOME/.bitcoin` and with `true` will return the network-specific DataDir e.g. `$HOME/.bitcoin/regtest`

Sorry, if my question was unclear. My point is whether we are interested in the `wallet` directory when `net
...

💬 fanquake commented on pull request "guix: consolidate to glibc 2.27 for Linux builds":
(https://github.com/bitcoin/bitcoin/pull/27029#discussion_r1104538636)
The patch is included in the branch. I've also bumped the branch to be the head of the current 2.27 release branch, which contains some additional backported fixes.

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104533746)
e0cbd87583cae174d5331ef6fb994f46013f2988, style nit:
```suggestion
uint64_t m_keypool_size GUARDED_BY(cs_desc_man){DEFAULT_KEYPOOL_SIZE};
```

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104533154)
e0cbd87583cae174d5331ef6fb994f46013f2988, style nit:
```suggestion
uint64_t m_keypool_size GUARDED_BY(cs_KeyStore){DEFAULT_KEYPOOL_SIZE};
```