💬 hebasto commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2485333241)
> @hebasto I don't understand.

`make -C depends HOST=arm-linux-gnueabihf` does not build multiprocess package.

💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485334909)
Forgot to modify `mining.capnp`.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2485392864)
`multiprocess_packages=` seems to be empty for all hosts. It turns out the first commit isn't actually working, as I can confirm locally.

Given that the arm job uses depends, it was incorrect to set `BITCOIN_CONFIG=-DWITH_MULTIPROCESS=ON`. It should simply follow what depends sets. I'll fix that in the next push.

💬 GIgako19929 commented on pull request "build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2485409959)
Marge branch
This pull request includes a change to the `src/test/fuzz/CMakeLists.txt` file to address a compiler bug in Visual Studio 2022 version 17.12. The change conditionally excludes `utxo_snapshot.cpp` from the build if the Visual Studio version is less than 1942.

* [`src/test/fuzz/CMakeLists.txt`](diffhunk://#diff-b937c829d4a0cf9a653f5361656ea8196b6ecef40fd033aa41a6da945e987bcdL126-R129): Added a conditional exclusion for `utxo_snapshot.cpp` to avoid an internal compiler error in Vis
...

💬 fanquake commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2485441127)
> Can someone rerun the windows job?

Kicked it now that the fix is in.

💬 fanquake commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2485445773)
https://github.com/bitcoin/bitcoin/pull/31279/checks#step:6:7681

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1848171448)
This was missing `$(host_os)_`

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2485465760)
I'd like to review this, but first, I'd like to understand why it is still in draft form.

👋 hodlinator's pull request is ready for review: "util: Improve documentation and negation of args"
(https://github.com/bitcoin/bitcoin/pull/31212)

💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2485490965)
> I'd like to review this, but first, I'd like to understand why it is still in draft form.

Had a linefeed issue on Windows in the previous push (and was in a WIP state over the weekend, before that).. but pretty sure it's ready to go.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2485492404)
I wrote:

> I set export `BITCOIND=bitcoin-node` just for the ARM job for now.

I think it should be enough to do this for one depends job and one non-depends job. I'll adjust that later.

👍 TheCharlatan approved a pull request: "multiprocess: Add capnp wrapper for Chain interface"
(https://github.com/bitcoin/bitcoin/pull/29409#pullrequestreview-2445205614)
lgtm ACK 34d3e2a6eaaab9de5328c3e64739f1392696c7db

💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2485516817)
Apparently code coverage on oss-fuzz has been broken since CMake. See https://issues.oss-fuzz.com/issues/379122777. Full logs (https://oss-fuzz-build-logs.storage.googleapis.com/log-6b12c5ff-d6c4-441c-a10f-2dffff96e82c.txt):
```bash
Step #7: [0m [0;31merror: /workspace/out/libfuzzer-coverage-x86_64/src/bitcoin-core/build_fuzz/src/wallet/wallet/walletdb.h: No such file or directory
Step #7: [0m [0;31mwarning: The file '/src/bitcoin-core/build_fuzz/src/wallet/wallet/walletdb.h' isn't covered.
...

💬 fanquake commented on issue "bitcoin-qt failed assertion on startup":
(https://github.com/bitcoin/bitcoin/issues/31289#issuecomment-2485535623)
I guess move to the GUI repo? Unclear if this will be fixed or not. @hebasto.

💬 brunoerg commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#discussion_r1848241863)
In f7dbb300d7a4d18c9d85f80e5fdd7e5bcd21c6f0: Do we really need `TestLoadWallet` here? Couldn't we simply create the wallet directly (e.g. `std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(test_setup->m_node.chain.get()), "", CreateMockableWalletDatabase())};`)?

👍 BrandonOdiwuor approved a pull request: "cmake: Improve robustness and usability"
(https://github.com/bitcoin/bitcoin/pull/31233#pullrequestreview-2445321566)
tACK 4b6a842c28010a00e121fd36cc0b4e1fa658d249 on Ubuntu 24.04

Was able to successfully test that `util_test_runner` and `util_rpcauth_test` are correctly disabled when Python3 interpreter is not found (screenshots below)

Overally this is an improvement, using the `Python3::Interpreter` target defined directly by find_package(...) is cleaner than relying on the `${PYTHON_COMMAND}` variable we define

without Python3
![Screenshot from 2024-11-19 15-24-13](https://github.com/user-attachmen
...

💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2485623740)
(The current Windows CI failure ([internal compiler error](https://github.com/bitcoin/bitcoin/actions/runs/11910989737/job/33191550567?pr=31212#step:10:1278)) is not specific to this PR, see #31303).

📝 defitricks opened a pull request: "Typo Update bitcoin-conf.md"
(https://github.com/bitcoin/bitcoin/pull/31322)
This pull request corrects a minor but important typographical error in the documentation. In the section discussing the placement of comments in the `bitcoin.conf` configuration file, the word **"preferable"** was used incorrectly. The correct word in this context is **"preferred"**.

- **Before:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferable_);"
- **After:** "Comments may appear in two ways: on their own on an otherwise empty line (_preferred_);"

...

💬 TheCharlatan commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2485653947)
I'm not sure why this is really an improvement. Can you elaborate on why no external user would want to pass in a script pubkey directly, instead of manually manipulating the coinbase in the block template?

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2485712161)
I added a commit to make `m_tip_block` and `std::optional`.

Also, looking at `m_tip_block` again, I'm fairly sure it's set to the tip during node initialization. Otherwise we'd never get something wrong:

https://github.com/bitcoin/bitcoin/blob/746f93b4f0f47c67642057944fb79dddf17369f9/src/init.cpp#L1794-L1806

So I was probably doing something wrong earlier.

I dropped the workaround. An early return only happens if:
1. `m_tip_block` is set; AND
2. `m_tip_block.value()` is different f
...