💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109304532)
```suggestion
// The memory used by an unordered_set or unordered_map is the sum of the
```

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109380457)
it makes sense to add the `/*max_mempool_size_bytes=*` for primitives - but we don't usually do it for self-explanatory variables:
```suggestion
chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, MAX_MEMPOOL_CACHE_BYTES),
```

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109325757)
If we're already checking equality, what's the point of checking that it's not equal to something else?
This should also enable us inlining `empty_cache`

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109331421)
I really dislike that we're exposing the loop variable just to see if we finished without transitioning.
We seem to exit at worst around `32415` for me locally, maybe we can reduce the attempt count and extract it to a variable:
```C++
constexpr size_t MAX_ATTEMPTS{50'000}; // runaway-loop safety cap
```
And we can add a worst-case condition for the very last iteration which should always be `LARGE`, e.g.:
```C++
// OK → LARGE
for (size_t i{1}; i <= MAX_ATTEMPTS; ++i) {

...

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109336914)
I find the `100/98` a weird way to express a percentage - if we simply want to check that it's roughly 256 KiB, we could check their ratio:
```suggestion
BOOST_CHECK_LT(view.DynamicMemoryUsage() / (256 * 1024.0), 1.1);
```

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109518426)
https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L219 already claims to account for 1-2 pointers, do we need any other change to synchronize it with the current PR?

💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109257463)
Indeed, please resolve the comment

🤔 BrandonOdiwuor reviewed a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2871591979)
ACK fa2bf6bdb7e5f8940fe4bce1aaab29bf2e063b49

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2913060959)
@pinheadmz, that's the first thing I though of, but unfortunately the header is as mutable as it gets (see e.g. `while (!CheckProofOfWork(block.GetHash(), block.nBits, Params().GetConsensus())) ++block.nNonce;`), so it would be dangerous to cache the hash, given that all fields are accessible from the outside and there's no simple way to invalidate the pre-computed hash, unless we expose them all to getter/setters which would invalidate the hash on change (and which would slow down `nNonce` expl
...

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2109545287)
I didn't want to make it that verbose, people can check the source code if they see a failure here :)

💬 l0rinc commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2913073569)
@andrewtoth, if you've closed it on purpose, do you want to explain the reason? Is it because you agree with @maflcko that making it a built-in functionality might be simpler? I'm also leaning in that direction, especially after https://github.com/bitcoin/bitcoin/pull/31144, where obfuscation should become a lot cheaper.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2913209698)
A bigger reason to do this in bitcoind is that it may be able to run in the background and lead to a better user experience.

💬 luke-jr commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#issuecomment-2913210853)
Is this actually intended behaviour?

🤔 hebasto reviewed a pull request: "deps: Bump lief to 0.16.5"
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2871819728)
> > I think it'd be worth investigating any issues with 0.16.5, rather than deferring.
>
> Sounds good, marking the PR as draft again while I investigate.
>
> I've bisected to this commit in `lief`: [lief-project/LIEF@f23ced2](https://github.com/lief-project/LIEF/commit/f23ced2f4ffc170d0a6f40ff4a1bee575e3447cf)
>
> Seems related to scikit-build 0.10 [changing](https://iscinumpy.dev/post/scikit-build-core-0-10/#other-changes) `cmake.targets` to `build.targets`, and defining `build.target
...

💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2109683378)
Is `python-tomli` really necessary?

💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2109682830)
```suggestion
lief==0.16.5 \
```

👍 hebasto approved a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2871843422)
re-ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f, only a [comment](https://github.com/bitcoin/bitcoin/pull/32623#discussion_r2109243775) has been addressed since my recent [review](https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2871162934).

💬 l0rinc commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#issuecomment-2913287986)
ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f

🚀 hebasto merged a pull request: "guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32439)

💬 hebasto commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#discussion_r2109748436)
Guix's `cmake-minimal` remains at version [3.24.2](https://codeberg.org/guix/guix/src/commit/b868ae741e989d56dbaf8dd481c66a1c00e45e25/gnu/packages/cmake.scm#L179).