👍 willcl-ark approved a pull request: "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate"
(https://github.com/bitcoin/bitcoin/pull/30039#pullrequestreview-2471071161)
ACK b73d3319377a4c9d7e2dd279c3d106002585bc36

👍 tdb3 approved a pull request: "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate"
(https://github.com/bitcoin/bitcoin/pull/30039#pullrequestreview-2471071557)
ACK b73d3319377a4c9d7e2dd279c3d106002585bc36

📝 glozow opened a pull request: "p2p: track and use all potential peers for orphan resolution"
(https://github.com/bitcoin/bitcoin/pull/31397)
Part of #27463.

(Transaction) **orphan resolution** is a process that kicks off when we are missing UTXOs to validate an unconfirmed transaction. We currently request missing parents by txid; BIP 331 also defines a way to [explicitly request ancestors](https://github.com/bitcoin/bips/blob/master/bip-0331.mediawiki#handle-orphans-better).

Currently, when we find that a transaction is an orphan, we only try to resolve it with the peer who provided the `tx`. If this doesn't work out (e.g. the
...

👍 laanwj approved a pull request: "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate"
(https://github.com/bitcoin/bitcoin/pull/30039#pullrequestreview-2471167870)
ACK b73d3319377a4c9d7e2dd279c3d106002585bc36

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1864584708)
Switched to the approach you suggested.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1864585362)
This is gone now, thanks.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2509452312)
Thanks for your review, @Sjors.

I addressed all of your comments.

> Something like this should do

Done. Instead of creating `DEFAULT_BLOCK_MAX_WEIGHT` that copies `MAX_BLOCK_WEIGHT`, I've used it directly.

> Maybe init.cpp should also enforce that -blockmaxweight <= MAX_BLOCK_WEIGHT

We already clamp the block size in the block assembler, so it will never exceed `MAX_BLOCK_WEIGHT`, but done with a test.

I've added a functional test and release notes.

📝 theStack opened a pull request: "wallet: refactor: various master key encryption cleanups"
(https://github.com/bitcoin/bitcoin/pull/31398)
This PR contains various cleanups around the wallet's master key encryption logic. The default/minimum key derivation rounds magic number of 25000 is hoisted into a constant (member of `CMasterKey`) and two new functions `EncryptMasterKey`/`DecryptMasterKey` are introduced in order to deduplicate code for the derivation round determination and master key en/decryption. Also, mentions of the never-implemented derivation method `scrypt` are removed from the wallet crypter header and both plain and
...

💬 davidgumberg commented on pull request "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2509581003)
ACK https://github.com/bitcoin/bitcoin/commit/b73d3319377a4c9d7e2dd279c3d106002585bc36

💬 purpleKarrot commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509610176)
> If source properties are used to modify the compiler flags for specific source files rather than for a whole target, changing the source’s compiler flags will still result in all of the target’s sources being rebuilt, not just the affected source file.

I don't think that applies here. The source's compile flags are never changed between builds: `sha256_avx2.cpp` is either compiled with `${AVX2_CXXFLAGS}`, or it is not compiled at all. And `AVX2_CXXFLAGS` is only ever set to `-mavx -mavx2`.
...

👍 hebasto approved a pull request: "build: Set shared linker flags in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31395#pullrequestreview-2471346690)
ACK a8e04704f930de168fdddbb1c040dd70fbbe8ff7, tested on Ubuntu 24.04:
```
$ make -C depends -j 16 HOST=arm64-apple-darwin NO_QT=1
$ cmake -B build --toolchain depends/arm64-apple-darwin/toolchain.cmake -DBUILD_SHARED_LIBS=ON -DBUILD_KERNEL_LIB=ON
$ cmake --build build -j 16 -t bitcoinkernel
```

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2509662129)
Rebased https://github.com/bitcoin/bitcoin/commit/19f39271fa9f5b26075ca8f66afe25751813bb75 to https://github.com/bitcoin/bitcoin/commit/ef7b1e8e82819e5104c1f1f49f46ea74b416ced0

>This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.
>
>The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here augments the mempool transaction REPLACED signal with the double-spending transa
...

💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509680979)
> @hebasto, can you elaborate why you think the source-specific compile options are hard to reason about?

Sure. Consider this minimal example:
```
cmake_minimum_required(VERSION 3.22)
project(test CXX)

add_library(interface INTERFACE)
target_compile_options(interface INTERFACE -Wno-mock-interface)

add_executable(test main.cpp)
set_property(SOURCE main.cpp PROPERTY COMPILE_OPTIONS -Wno-mock-source)
target_compile_options(test PRIVATE -Wno-mock-target)
target_link_libraries(test PR
...

📝 hebasto opened a pull request: "ci, macos: Install `pkgconf` Homebrew's package"
(https://github.com/bitcoin/bitcoin/pull/31399)
The updated GHA image [`20241125.556`](https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20241125.556) is now fully [deployed](https://github.com/actions/runner-images/blob/main/README.md#available-images), enabling the installation of `pkgconf` as documented in [`doc/build-osx.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#3-install-required-dependencies).

💬 purpleKarrot commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509724461)
Why is that hard to reason about? If I add

```cmake
add_compile_options(-Wno-mock-global)
```

the resulting order will be `-Wno-mock-global -Wno-mock-target -Wno-mock-interface -Wno-mock-source`, so the flags are ordered from most generic to most specific.

That aside, I don't think the order of arguments is relevant here.

💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509726671)
> Why is that hard to reason about?

People unfamiliar with such peculiarities might not realise that compiler options specified as a target's property take precedence over other compiler options.

💬 hebasto commented on issue "build: Windows debug cross-build fails with `-O0`":
(https://github.com/bitcoin/bitcoin/issues/28109#issuecomment-2509735314)
Now observing a similar error when building on Windows with the default `--config Debug`:
```
...\bitcoin\src\wallet\wallet.cpp(1,1): error C1128: number of sections exceeded object file format limit: compile with /bigobj [...\bitcoin\build-static\src\wallet\bitcoin_wallet.vcxproj]
```

💬 laanwj commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1864872630)
An alternative idea would be to add a `btcStringToSat` function, instead of overloading `btcToSat` on type, as this makes type checking harder.

💬 laanwj commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1864872826)
Yes, passing sats as a decimal should never be necessary.

💬 purpleKarrot commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509746149)
Yeah, they might also not realize that compiler options specified at the target level might take precedence over those specified globally...