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

👍 pinheadmz approved a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487#pullrequestreview-2871920816)
ACK 09ee8b7f278627b917f0784adf23cbc76cae5fa0

Built and tested on macos/arm64 and reviewed code. This is a simple optimization that deduplicates a call to `Block.GetHash()` when reading a block from disk by `BlockManager`. The hash is checked against the expected result from a `CBlockIndex`. Covered by a new unit test.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 09ee8b7f278627b917f0784adf23cbc76cae5fa0
-----BEGIN PGP SIGNATURE-
...

🤔 theStack reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2871820534)
Reviewed thoroughly up to ff7476ec32f4dbee07bf04169e741e2aea5a9ab7, looks good to me (with one refactoring suggestion below). Still need to go again through the parsing code as there is a lot going on with all the conditions on what is allowed and what not w.r.t. derivations.

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2109683327)
in commit ff7476ec32f4dbee07bf04169e741e2aea5a9ab7: can't this be simplified to
```suggestion
out += pubkey->ToString(type);
```
? It's a bit confusing that there are two `StringType` enum classes within different classes, (`PubkeyProvider::StringType` and `DescriptorImpl::StringType`), but the code here seems to deal only with the former. I guess whether the diff makes sense or not depends on if it's possible in the future that the default arguments for pubkey providers `ToStrin
...

💬 luke-jr commented on pull request "gui: Add a menu action to restore then migrate a legacy wallet":
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2913426259)
>by adding a menu item in the "Migrate Wallet" menu

IMO this is bad UX. Users will be looking to restore the wallet, and the details of how it does that shouldn't require unintuitive steps.