👍 fanquake approved a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/31186#pullrequestreview-2416130023)
ACK f6577b717416191e65ba8221be57f1a48d74e5d7

🚀 fanquake merged a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/31186)

💬 fanquake commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#discussion_r1829664613)
> Overall, I think this is a reasonable approach because it pretty much just works with no fiddling. The main drawback is that over time the headers list will grow stale, so it'll require some attention now and then.

Not sure if here, but it'd seem useful to document some (minified) version of the PR description in regards to maintaining this going forward. If anything to prevent if from being too arbtrary.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2457652154)
Rebased due to the conflict with the merged bitcoin/bitcoin#31186.

💬 instagibbs commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2457652583)
is this ready for undraft?

👍 hodlinator approved a pull request: "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests"
(https://github.com/bitcoin/bitcoin/pull/30746#pullrequestreview-2416152348)
re-ACK 6fd185c035c1cc4dd961cf14a2087e97fb069440

(Hoping Drahtbot will react properly to this comment.. already ACKed the same PR version above).

💬 fanquake commented on pull request "depends: Specify CMake generator explicitly":
(https://github.com/bitcoin/bitcoin/pull/31171#issuecomment-2457662041)
> in environments where the CMAKE_GENERATOR variable is set.

Which environment/distro did you see this in?

💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2457665391)
Rebased due to the conflict with the merged bitcoin/bitcoin#31186.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1829677516)
Good catch. I'll make that modification if I need to retouch otherwise, but I think it'd be fine as a followup.

💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1829677704)
I think so, it just needs to check if the script was built with a loaded key.

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2457667761)
Updated the benchmark to 860k blocks (September 2024):
<img src="https://github.com/user-attachments/assets/1d70f5d9-68cb-4e5f-85e9-ffb9fd3b3f1c">

This one contains a lot of very big arrays (96'233 separate sizes, biggest was 3'992'470 bytes long) - a big departure from the previous 400k and 700k blocks (having 1500 sizes, biggest was 9319 bytes long).

The performance characteristics are also quite different, now that we have more and bigger byte arrays:

> C++ compiler ...............
...

💬 hebasto commented on pull request "depends: Specify CMake generator explicitly":
(https://github.com/bitcoin/bitcoin/pull/31171#issuecomment-2457674743)
> > in environments where the CMAKE_GENERATOR variable is set.
>
> Which environment/distro did you see this in?

I don’t mean a specific distro, but rather a user-modified environment—for example, setting `CMAKE_GENERATOR` in the `~/.profile` file.

💬 stickies-v commented on pull request "refactor: ensure type safety for txid and wtxid in `RelayTransaction`":
(https://github.com/bitcoin/bitcoin/pull/31001#issuecomment-2457675192)
> I'll look to expand this one a bit then.

Are you planning to rework this PR, or is it ready for review as is? If the former, perhaps best to mark this as draft until that's done?

💬 maflcko commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2457694748)
If the error message is confusing, it would be a reason to improve the error message. However, this applies to all RPCs and maintaining the error message in a single RPC (or each RPC individually) seems inconsistent (or brittle and verbose).

📝 marcofleon converted_to_draft a pull request: "refactor: ensure type safety for txid and wtxid in `RelayTransaction`"
(https://github.com/bitcoin/bitcoin/pull/31001)
This PR updates the `RelayTransaction` function to replace `uint256` with the `Txid` and `Wtxid` types, improving type safety and helping with the gradual transition to using these types throughout the codebase.

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1829706921)
done

💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2457711117)
Rebased and addressed nit from @maflcko

📝 dergoegge converted_to_draft a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.

In both jobs the fuzz binary is built with -DBUILD_FOR_FUZZING to enable Assume assertions as well as FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.

💬 Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829738339)
This seems like a reasonable thing to do. However it seems like the `script.py` implementation should still handle `OP_1NEGATE` imo.

IIUC the `solver.cpp`'s `IsSmallInteger()` and `script.py`'s `is_small_int()` serve 2 different purposes, and renaming the solver.cpp function name would help differentiate them.

💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2457771782)
re-ACK 5e4df9f67179e9cc284cb2ad2264de6b8bb6c606, no changes except for addressing merge conflict from #31139

💬 achow101 commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2457791648)
I don't think this issue has to do with ZFS. I'm debugging the test on the problematic system and it seems that BDB decided to use a page size of 16384 for some reason, rather than our expected and hardcoded 4096. Changing it to 16384 resolves this, but we should instead be using the pagesize given by the wallet file rather than a fixed pagesize. This is a test only issue as the C++ parser does not use a fixed pagesize.