💬 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};
```

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104511053)
Any specific reason to use unsigned type here (as such types are appropriate for bit-wise operations only)?

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104538320)
9022473ff167f2677ccad27d98705c3887bf9f96, nit: sort?
```suggestion
#include <util/string.h>
#include <util/system.h>
```

💬 hebasto commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104535461)
e0cbd87583cae174d5331ef6fb994f46013f2988, style nit:
```suggestion
walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), (int64_t)1);
```

✳️ MarcoFalke pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/141115a06040...8126551d54ff)
Merge bitcoin/bitcoin#27011: Add simulation-based `CCoinsViewCache` fuzzer

561848aaf2d67791e92754f3d11813bc53959a8f Exercise non-DIRTY spent coins in caches in fuzz test (Pieter Wuille)
59e6828bb5b56a2354a80059d3f660f551f3e207 Add deterministic mode to CCoinsViewCache (Pieter Wuille)
b0ff31084006ac7d4a7afba3190ca75f5f8441af Add CCoinsViewCache::SanityCheck() and use it in fuzz test (Pieter Wuille)
3c9cea1340fd1358d6854209d782922864945eb0 Add simulation-based CCoinsViewCache fuzzer (Pieter Wuille)

Pull request description:

The fuzzer goes through a sequence of operations that get applied to both a real stack of `CCoinsViewCache` objects, and to simulation data, comparing the two at the end.

ACKs for top commit:
jamesob:
re-ACK https://github.com/bitcoin/bitcoin/pull/27011/commits/561848aaf2d67791e92754f3d11813bc53959a8f
dergoegge:
Code review ACK 561848aaf2d67791e92754f3d11813bc53959a8f

Tree-SHA512: 68634f251fdb39436b128ecba093f651bff12ac11508dc9885253e57fd21efd44edf3b22b0f821c228175ec507df7d46c7f9f5404fc1eb8187fdbd136a5d5ee2

🚀 MarcoFalke merged a pull request: "Add simulation-based `CCoinsViewCache` fuzzer"
(https://github.com/bitcoin/bitcoin/pull/27011)

💬 instagibbs commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1104552550)
comment above should be updated I think:

"# Test we can sign most Miniscript (all but ones requiring preimages, for now)"

💬 hebasto commented on pull request "refactor, wallet: Avoid variable shadowing":
(https://github.com/bitcoin/bitcoin/pull/27087#issuecomment-1428050044)
> There are currently multiple instances of `-Wshadow` warnings emmited when compiling the codebase.

It was not about compiler warning, rather about code readability.

> Is there some reason this one specifically needs fixing?

Probably subjective, but there was some extra mental burden while reviewing other PRs which touch this code.

> Otherwise this just introduces a pointless merge conflict to a number of very recently-rebased PRs.

I hoped that just renaming (the first version of
...