💬 maflcko commented on pull request "build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2484864282)
I'd say this is fine to merge even without an upstream report. Even if there was one, it will take some time (weeks?) to propagate the fix. Blocking the CI for this repo on that seems not ideal.
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2484864282)
I'd say this is fine to merge even without an upstream report. Even if there was one, it will take some time (weeks?) to propagate the fix. Blocking the CI for this repo on that seems not ideal.
💬 maflcko commented on issue "Segmentation fault when ./bitcoind":
(https://github.com/bitcoin/bitcoin/issues/31321#issuecomment-2484868095)
What are the exact steps to reproduce? What is the traceback?
(https://github.com/bitcoin/bitcoin/issues/31321#issuecomment-2484868095)
What are the exact steps to reproduce? What is the traceback?
👍 BrandonOdiwuor approved a pull request: "Decouple WalletModel from RPCExecutor"
(https://github.com/bitcoin-core/gui/pull/841#pullrequestreview-2444475869)
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
Was able to build successfully on Ubuntu 24.04 with Qt version 5.15.13 with `-DENABLE_WALLET=OFF` and `-BUILD_GUI=ON` flags and was able to successfully try a couple of RPCs on the GUI RPC console

(https://github.com/bitcoin-core/gui/pull/841#pullrequestreview-2444475869)
tACK 002b792b9a85100d89e47706c29cf1fd355d9727
Was able to build successfully on Ubuntu 24.04 with Qt version 5.15.13 with `-DENABLE_WALLET=OFF` and `-BUILD_GUI=ON` flags and was able to successfully try a couple of RPCs on the GUI RPC console

💬 maflcko commented on pull request "refactor: Fix remaining clang-tidy performance-inefficient-vector errors":
(https://github.com/bitcoin/bitcoin/pull/31305#discussion_r1847956841)
> Are you saying this will affect fuzzing behavior?
No, I was thinking that this may slow down the fuzz target, but I don't see any difference when running over `./fuzz_corpora/txorphan`, so I guess this doesn't matter.
(Since you are asking, yes, this may influence the fuzz engine behavior. The reason is that the standard library is also annotated with coverage feedback, so a memory reallocation may count as a coverage increase)
(https://github.com/bitcoin/bitcoin/pull/31305#discussion_r1847956841)
> Are you saying this will affect fuzzing behavior?
No, I was thinking that this may slow down the fuzz target, but I don't see any difference when running over `./fuzz_corpora/txorphan`, so I guess this doesn't matter.
(Since you are asking, yes, this may influence the fuzz engine behavior. The reason is that the standard library is also annotated with coverage feedback, so a memory reallocation may count as a coverage increase)
💬 maflcko commented on pull request "refactor: Fix remaining clang-tidy performance-inefficient-vector errors":
(https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2485135779)
lgtm ACK 01e54b7d82a053238b62121eeba2ac2c45b88859
(https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2485135779)
lgtm ACK 01e54b7d82a053238b62121eeba2ac2c45b88859
💬 fanquake commented on pull request "build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2485281314)
Not blocking, but we can leave #31303 open until an upstream report is filed.
(https://github.com/bitcoin/bitcoin/pull/31307#issuecomment-2485281314)
Not blocking, but we can leave #31303 open until an upstream report is filed.
💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2485303923)
@hebasto I don't understand.
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2485303923)
@hebasto I don't understand.
🚀 fanquake merged a pull request: "build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/31307)
(https://github.com/bitcoin/bitcoin/pull/31307)
💬 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.
(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`.
(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.
(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
...
(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.
(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
(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)_`
(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.
(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)
(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.
(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.
(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
(https://github.com/bitcoin/bitcoin/pull/29409#pullrequestreview-2445205614)
lgtm ACK 34d3e2a6eaaab9de5328c3e64739f1392696c7db