💬 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
...

✅ hebasto closed a pull request: "refactor, wallet: Avoid variable shadowing"
(https://github.com/bitcoin/bitcoin/pull/27087)

💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1104560393)
Thanks, updated.

💬 sipa commented on pull request "[WIP] p2p: Add random txn's from mempool to GETBLOCKTXN":
(https://github.com/bitcoin/bitcoin/pull/27086#issuecomment-1428054418)
> I agree with @sipa, with a stronger emphasis that I would probably NACK this change because the cost of this fix is too high, and the privacy gain is too low.

Yeah, Approach NACK. I may be convinced that doing *something* to avoid mempool fingerprinting through GETBLOCKTXN is worth it, but if we want that, there are better ways than this.

> Note that it's very easy for an adversary to change that by simply broadcasting simultaneous double-spends with the same fee. Indeed, n-way double sp
...

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104563193)
Why only for bit-wise operations? It's just a standard data type that represents an unsigned 64-bit integer.
Didn't have any specific reason, just that is more concise and specific. Could change it to a bare unsigned int too.

💬 MarcoFalke commented on pull request "[WIP] p2p: Add random txn's from mempool to GETBLOCKTXN":
(https://github.com/bitcoin/bitcoin/pull/27086#issuecomment-1428059849)
> If we don't add wallet transactions to the mempool but still relay them

Yeah, I didn't mention this, but obviously we wouldn't relay them with the mempool. Doing a one-shot (tor-only) outbound connection to fan-out the tx (one-hop dandelion) without adding it to the mempool shouldn't leave a fingerprint, other than the one left by the tor-only connection, no?

💬 sipa commented on pull request "[WIP] p2p: Add random txn's from mempool to GETBLOCKTXN":
(https://github.com/bitcoin/bitcoin/pull/27086#issuecomment-1428064176)
@MarcoFalke Oh, fair enough, that's a good idea (though it'd probably still need a fallback to normal relay after some delay if we don't observe the transaction being rumoured back to us). I also think it's orthogonal to the idea here, because even absent "first mile" wallet broadcast leakage, we still want the P2P network to obscure transaction relay beyond that.

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104574509)
pushed

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104574758)
pushed

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104575040)
pushed

💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104579180)
I did some digging, and I think (would be nice to get more confirmation on this) we can scrap the `net_specific == false` version entirely, we don't seem to need it. `EnsureDataDir` is called 3 times: 2 times right before reading config files - but it looks like we don't actually need to ensure a datadir there. If datadir doesn't exist, by definition no config files can be read (and that's handled gracefully). The only time we need it is where `net_specific==true`.

Intuitively, this makes sen
...

💬 fanquake commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1428074269)
Does/can this also close #16220?