💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290145678)
syle nit: Could be ubuntu-latest, to avoid having to touch this again in the future?
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290145678)
syle nit: Could be ubuntu-latest, to avoid having to touch this again in the future?
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290159660)
nit: IIUC the checkout merge_base issue mentioned in https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3133536724 still remains and would be nice to fix. But this can be done in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290159660)
nit: IIUC the checkout merge_base issue mentioned in https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3133536724 still remains and would be nice to fix. But this can be done in a follow-up.
💬 maflcko commented on issue "RFC: Adding bitcoin-{node,gui} binaries for IPC in 30.0 release":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-3209542920)
Anything left to be done here?
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-3209542920)
Anything left to be done here?
💬 maflcko commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-3209553290)
> The wrapper binary has been merged with [#31375](https://github.com/bitcoin/bitcoin/pull/31375). Actually shipping the binaries would happen if [#31802](https://github.com/bitcoin/bitcoin/pull/31802) lands.
This was merged yesterday.
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-3209553290)
> The wrapper binary has been merged with [#31375](https://github.com/bitcoin/bitcoin/pull/31375). Actually shipping the binaries would happen if [#31802](https://github.com/bitcoin/bitcoin/pull/31802) lands.
This was merged yesterday.
💬 maflcko commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3209577547)
OSS-Fuzz is now failing, as explained above in https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3198034390 (with possible fixes).
https://oss-fuzz-build-logs.storage.googleapis.com/log-66a59cf3-d2f6-4bf1-b4e1-0a5ef44fbaaf.txt
```
...
Step #3 - "compile-afl-address-x86_64": /src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
Step #3 - "compile-afl-address-x86_64": Extracting native_capnp...
Step #3 - "compile-afl-address-x86_64": /src
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3209577547)
OSS-Fuzz is now failing, as explained above in https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3198034390 (with possible fixes).
https://oss-fuzz-build-logs.storage.googleapis.com/log-66a59cf3-d2f6-4bf1-b4e1-0a5ef44fbaaf.txt
```
...
Step #3 - "compile-afl-address-x86_64": /src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
Step #3 - "compile-afl-address-x86_64": Extracting native_capnp...
Step #3 - "compile-afl-address-x86_64": /src
...
💬 maflcko commented on issue "fuzz: `txgraph`: Assertion `cmp == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/33097#issuecomment-3209585918)
Do you recall the fuzzing engine used to find this? I tried libFuzzer on 8 threads for two weeks, but haven't re-found it so far.
(https://github.com/bitcoin/bitcoin/issues/33097#issuecomment-3209585918)
Do you recall the fuzzing engine used to find this? I tried libFuzzer on 8 threads for two weeks, but haven't re-found it so far.
🤔 stickies-v reviewed a pull request: "index: Don't commit state in BaseIndex::Rewind"
(https://github.com/bitcoin/bitcoin/pull/33212#pullrequestreview-3139699536)
Concept ACK
With this change, we only call `Commit()` during `Sync()` and `ChainStateFlushed()`. The latter makes sense, but I'd like to look into the `Sync()` usage a bit more to understand if this fix is complete.
(https://github.com/bitcoin/bitcoin/pull/33212#pullrequestreview-3139699536)
Concept ACK
With this change, we only call `Commit()` during `Sync()` and `ChainStateFlushed()`. The latter makes sense, but I'd like to look into the `Sync()` usage a bit more to understand if this fix is complete.
🤔 janb84 reviewed a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3139747275)
re ACK 03c66ee229393335fabae1192b871f9ec68adf74
changes since last ACK:
- rebase
- addressed some NIT's
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3139747275)
re ACK 03c66ee229393335fabae1192b871f9ec68adf74
changes since last ACK:
- rebase
- addressed some NIT's
💬 maflcko commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2290465048)
should this really be hidden by default? It seems like it should only happen unexpectedly, in which case a LogWarning may be better, or at least LogInfo?
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2290465048)
should this really be hidden by default? It seems like it should only happen unexpectedly, in which case a LogWarning may be better, or at least LogInfo?
💬 maflcko commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290393840)
my understanding is that this should refer to the pull introducing the min required, which would be `[0.7.1](https://github.com/bitcoin/bitcoin/pull/28907)` via https://github.com/bitcoin-core/libmultiprocess/commit/414542f81e0997354b45b8ade13ca144a3e35ff1
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290393840)
my understanding is that this should refer to the pull introducing the min required, which would be `[0.7.1](https://github.com/bitcoin/bitcoin/pull/28907)` via https://github.com/bitcoin-core/libmultiprocess/commit/414542f81e0997354b45b8ade13ca144a3e35ff1
💬 dergoegge commented on issue "fuzz: `txgraph`: Assertion `cmp == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/33097#issuecomment-3209820443)
The crashing input was found by afl++ but libFuzzer and honggfuzz were also part of the campaign (probably took ~64 CPU hours).
(https://github.com/bitcoin/bitcoin/issues/33097#issuecomment-3209820443)
The crashing input was found by afl++ but libFuzzer and honggfuzz were also part of the campaign (probably took ~64 CPU hours).
⚠️ fanquake opened an issue: "oss-fuzz: build is broken"
(https://github.com/bitcoin/bitcoin/issues/33232)
Now that #31802 is merged, `oss-fuzz` needs fixing, to either opt-out of multiprocess, or configure depends to use the pre-built Clang for native package compilation (as Ubuntu 20.04 ships with GCC 9):
```bash
/src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
Extracting native_capnp...
/src/bitcoin-core/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK
Preprocessing native_capnp...
Configuring native_capnp...
-- The CXX compiler identification is GNU 9.4
...
(https://github.com/bitcoin/bitcoin/issues/33232)
Now that #31802 is merged, `oss-fuzz` needs fixing, to either opt-out of multiprocess, or configure depends to use the pre-built Clang for native package compilation (as Ubuntu 20.04 ships with GCC 9):
```bash
/src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
Extracting native_capnp...
/src/bitcoin-core/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK
Preprocessing native_capnp...
Configuring native_capnp...
-- The CXX compiler identification is GNU 9.4
...
📝 Sjors opened a pull request: "IPC followups for PR 31802"
(https://github.com/bitcoin/bitcoin/pull/33233)
- have `dependencies.md` link to the PR that updated the capnp version: https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290393840
(https://github.com/bitcoin/bitcoin/pull/33233)
- have `dependencies.md` link to the PR that updated the capnp version: https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290393840
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290572656)
Done in #33233
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2290572656)
Done in #33233
🤔 janb84 reviewed a pull request: "IPC followups for PR 31802"
(https://github.com/bitcoin/bitcoin/pull/33233#pullrequestreview-3140010248)
ACK 2a815d126bc90f787f7adbe8cade45cb7307429b
PR updates dependencies document to point to version bump PR of capnproto.
- "code" review ✅
(https://github.com/bitcoin/bitcoin/pull/33233#pullrequestreview-3140010248)
ACK 2a815d126bc90f787f7adbe8cade45cb7307429b
PR updates dependencies document to point to version bump PR of capnproto.
- "code" review ✅
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3209949040)
@fanquake @maflcko I opened a PR to temporarily disable IPC build for oss-fuzz, please take a look: https://github.com/google/oss-fuzz/pull/13854
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3209949040)
@fanquake @maflcko I opened a PR to temporarily disable IPC build for oss-fuzz, please take a look: https://github.com/google/oss-fuzz/pull/13854
💬 Sjors commented on issue "oss-fuzz: build is broken":
(https://github.com/bitcoin/bitcoin/issues/33232#issuecomment-3209951042)
Since I'm not at all familiar with oss-fuzz, I've gone for the simplest solution for now: https://github.com/google/oss-fuzz/pull/13854
(https://github.com/bitcoin/bitcoin/issues/33232#issuecomment-3209951042)
Since I'm not at all familiar with oss-fuzz, I've gone for the simplest solution for now: https://github.com/google/oss-fuzz/pull/13854
💬 maflcko commented on pull request "IPC followups for PR 31802":
(https://github.com/bitcoin/bitcoin/pull/33233#issuecomment-3209973594)
lgtm ACK 2a815d126bc90f787f7adbe8cade45cb7307429b
(https://github.com/bitcoin/bitcoin/pull/33233#issuecomment-3209973594)
lgtm ACK 2a815d126bc90f787f7adbe8cade45cb7307429b
💬 fanquake commented on pull request "doc: add Linux GUI runtime instructions to doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/33197#discussion_r2290638615)
Opened #33217 to fix the issue.
(https://github.com/bitcoin/bitcoin/pull/33197#discussion_r2290638615)
Opened #33217 to fix the issue.
🤔 BrandonOdiwuor reviewed a pull request: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367#pullrequestreview-3140088293)
ACK eb2a9435044ac91442fafc606c5af2f473bff3c5
Confirmed that the new `bitcoincore_enable_language(...)` macro properly validates user-defined `APPEND_*FLAGS` early via `check_source_compiles(...)`, fixing #32323 by catching bad flags during enable_language(...), and removing the need a clean build directory.
(system: macOS 15.6)
(branch: master)
<img width="1512" height="917" alt="Screenshot 2025-08-21 at 13 21 10" src="https://github.com/user-attachments/assets/be10daae-31c4-4eb7-8504-
...
(https://github.com/bitcoin/bitcoin/pull/32367#pullrequestreview-3140088293)
ACK eb2a9435044ac91442fafc606c5af2f473bff3c5
Confirmed that the new `bitcoincore_enable_language(...)` macro properly validates user-defined `APPEND_*FLAGS` early via `check_source_compiles(...)`, fixing #32323 by catching bad flags during enable_language(...), and removing the need a clean build directory.
(system: macOS 15.6)
(branch: master)
<img width="1512" height="917" alt="Screenshot 2025-08-21 at 13 21 10" src="https://github.com/user-attachments/assets/be10daae-31c4-4eb7-8504-
...