💬 maflcko commented on pull request "contrib: Make deterministic-coverage error messages more readable":
(https://github.com/bitcoin/bitcoin/pull/32074#issuecomment-2732155793)
@hodlinator I'll wait for you to give the ack/review on this, and once you are happy that your feedback is fully addressed, I'll move it out of draft
(https://github.com/bitcoin/bitcoin/pull/32074#issuecomment-2732155793)
@hodlinator I'll wait for you to give the ack/review on this, and once you are happy that your feedback is fully addressed, I'll move it out of draft
💬 hebasto commented on pull request "depends: Avoid using helper variables in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31360#issuecomment-2732199581)
> CI fails with https://cirrus-ci.com/task/6136289713455104?logs=ci#L531 :
It was an error during rebasing. Fixed.
Thank you!
(https://github.com/bitcoin/bitcoin/pull/31360#issuecomment-2732199581)
> CI fails with https://cirrus-ci.com/task/6136289713455104?logs=ci#L531 :
It was an error during rebasing. Fixed.
Thank you!
⚠️ Prabhat1308 opened an issue: "Failure to run Fuzz tests when running with corpus"
(https://github.com/bitcoin/bitcoin/issues/32089)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When running the fuzz tests with fuzz corpus raises an error
```
FUZZ=process_message build_fuzz/bin/fuzz qa-assets/fuzz_corpora/process_message/ ─╯
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 64371175
INFO: Loaded 1 modules (1252320 inline 8-bit counters): 1252320 [0x1061c8000, 0x1062f9be0),
INFO: Loaded 1 PC tables
...
(https://github.com/bitcoin/bitcoin/issues/32089)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When running the fuzz tests with fuzz corpus raises an error
```
FUZZ=process_message build_fuzz/bin/fuzz qa-assets/fuzz_corpora/process_message/ ─╯
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 64371175
INFO: Loaded 1 modules (1252320 inline 8-bit counters): 1252320 [0x1061c8000, 0x1062f9be0),
INFO: Loaded 1 PC tables
...
👍 fanquake approved a pull request: "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets"
(https://github.com/bitcoin/bitcoin/pull/32019#pullrequestreview-2693581701)
ACK 1f9b2e150ce5aa192226d2daae73f7d678c47d65
(https://github.com/bitcoin/bitcoin/pull/32019#pullrequestreview-2693581701)
ACK 1f9b2e150ce5aa192226d2daae73f7d678c47d65
💬 Prabhat1308 commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2732222228)
I tried running with the
```
ASAN_OPTIONS=detect_container_overflow=0 FUZZ=process_message build_fuzz/bin/fuzz qa-assets/fuzz_corpora/process_message/
```
It crashes with the following log
```
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 917199606
INFO: Loaded 1 modules (1252320 inline 8-bit counters): 1252320 [0x109024000, 0x109155be0),
INFO: Loaded 1 PC tables (1252320 PCs): 1252320 [0x109155be0,0x10a4719e0),
INFO: 4126 files found in qa-assets/fuzz_corpora/pro
...
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-2732222228)
I tried running with the
```
ASAN_OPTIONS=detect_container_overflow=0 FUZZ=process_message build_fuzz/bin/fuzz qa-assets/fuzz_corpora/process_message/
```
It crashes with the following log
```
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 917199606
INFO: Loaded 1 modules (1252320 inline 8-bit counters): 1252320 [0x109024000, 0x109155be0),
INFO: Loaded 1 PC tables (1252320 PCs): 1252320 [0x109155be0,0x10a4719e0),
INFO: 4126 files found in qa-assets/fuzz_corpora/pro
...
✅ fanquake closed an issue: "cmake: `makensis` isn't checked-for before use"
(https://github.com/bitcoin/bitcoin/issues/32018)
(https://github.com/bitcoin/bitcoin/issues/32018)
🚀 fanquake merged a pull request: "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets"
(https://github.com/bitcoin/bitcoin/pull/32019)
(https://github.com/bitcoin/bitcoin/pull/32019)
⚠️ hebasto opened an issue: "qa: `p2p_ibd_stalling.py --v1transport` fails"
(https://github.com/bitcoin/bitcoin/issues/32090)
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/13914507401/job/38935007425:
```
320/321 - p2p_ibd_stalling.py --v1transport failed, Duration: 484 s
stdout:
2025-03-18T04:03:23.417000Z TestFramework (INFO): PRNG seed is: 8897219840825193112
2025-03-18T04:03:23.418000Z TestFramework (INFO): Initializing test directory /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/test_runner_₿_🏃_20250318_034132/p2p_ibd_stalling_112
2025-03-18T04:03:23.719000Z TestFramework (INFO): Pre
...
(https://github.com/bitcoin/bitcoin/issues/32090)
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/13914507401/job/38935007425:
```
320/321 - p2p_ibd_stalling.py --v1transport failed, Duration: 484 s
stdout:
2025-03-18T04:03:23.417000Z TestFramework (INFO): PRNG seed is: 8897219840825193112
2025-03-18T04:03:23.418000Z TestFramework (INFO): Initializing test directory /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/test_runner_₿_🏃_20250318_034132/p2p_ibd_stalling_112
2025-03-18T04:03:23.719000Z TestFramework (INFO): Pre
...
💬 hebasto commented on pull request "build: Drop option to disable hardening.":
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2732279914)
> > > The only case I'm aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.
> >
> >
> > in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely.
>
> Okay, I think this branch will break building on NetBSD, and shouldn't be merged as-is, I'll follow up.
Perhaps I should clarify t
...
(https://github.com/bitcoin/bitcoin/pull/32071#issuecomment-2732279914)
> > > The only case I'm aware of where some hardening flags cause an issue is on NetBSD 10, and this is expected to be fixed in NetBSD 11.
> >
> >
> > in general if some flags (hardening or otherwise) are a problem on some platform this should be taken into account by the build system automatically instead of having the user disable hardening entirely.
>
> Okay, I think this branch will break building on NetBSD, and shouldn't be merged as-is, I'll follow up.
Perhaps I should clarify t
...
💬 laanwj commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732432018)
i'm hesitant about recommending people to use GNU tools on BSD variants, one of the advantages i hoped cmake would have is that we can just use native build tools as-is.
> macOS ships with GNU Make 3.81 from 2006. This has caused difficult to debug issues, e.g. https://github.com/bitcoin/bitcoin/pull/32070 and https://github.com/bitcoin/bitcoin/issues/30978.
That said, i didn't realize that macOS already ships with GNU make, that changes things.
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732432018)
i'm hesitant about recommending people to use GNU tools on BSD variants, one of the advantages i hoped cmake would have is that we can just use native build tools as-is.
> macOS ships with GNU Make 3.81 from 2006. This has caused difficult to debug issues, e.g. https://github.com/bitcoin/bitcoin/pull/32070 and https://github.com/bitcoin/bitcoin/issues/30978.
That said, i didn't realize that macOS already ships with GNU make, that changes things.
💬 Sjors commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732469202)
@laanwj in the longer run I hope we can "just" move depends over to cmake entirely.
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732469202)
@laanwj in the longer run I hope we can "just" move depends over to cmake entirely.
💬 hebasto commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732479269)
> i'm hesitant about recommending people to use GNU tools on BSD variants, one of the advantages i hoped cmake would have is that we can just use native build tools as-is.
That's true.
However, our depends build subsystem is not managed by CMake and requires GNU Make, not BSD Make, to run.
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732479269)
> i'm hesitant about recommending people to use GNU tools on BSD variants, one of the advantages i hoped cmake would have is that we can just use native build tools as-is.
That's true.
However, our depends build subsystem is not managed by CMake and requires GNU Make, not BSD Make, to run.
💬 laanwj commented on pull request "Shuffle depends instructions and recommend modern make for macOS":
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732522102)
> @laanwj in the longer run I hope we can "just" move depends over to cmake entirely.
> However, our depends build subsystem is not managed by CMake and requires GNU Make, not BSD Make, to run.
Ah yes *for the depends* it's unavoidable right now, makes sense, sorry.
(https://github.com/bitcoin/bitcoin/pull/32086#issuecomment-2732522102)
> @laanwj in the longer run I hope we can "just" move depends over to cmake entirely.
> However, our depends build subsystem is not managed by CMake and requires GNU Make, not BSD Make, to run.
Ah yes *for the depends* it's unavoidable right now, makes sense, sorry.
💬 laanwj commented on pull request "http: Make server shutdown more robust":
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2732578296)
From what i remember this was *extrememly* hard to get right with libevent (or at least our use of it), without leaking anything or running into race conditions. There have been a few tries to fix the behavior over the years, but they all were different compromises, adding ever more compexity.
As it seems to be where things are heading anyway (#32061) maybe it's better to focus on replacing libevent and its webserver framework wholesale.
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2732578296)
From what i remember this was *extrememly* hard to get right with libevent (or at least our use of it), without leaking anything or running into race conditions. There have been a few tries to fix the behavior over the years, but they all were different compromises, adding ever more compexity.
As it seems to be where things are heading anyway (#32061) maybe it's better to focus on replacing libevent and its webserver framework wholesale.
🤔 musaHaruna reviewed a pull request: "test: Add test coverage for rpcwhitelistdefault when unset"
(https://github.com/bitcoin/bitcoin/pull/32079#pullrequestreview-2693889131)
Concept ACK
Creating a seperate function`test_rpcwhitelistdefault_unset` is okay, it avoids mixing functionalities into the same function, thereby making `test_rpcwhitelistdefault_permissions` less complex.
Each function is doing exactly what the function name suggests. IMO
(https://github.com/bitcoin/bitcoin/pull/32079#pullrequestreview-2693889131)
Concept ACK
Creating a seperate function`test_rpcwhitelistdefault_unset` is okay, it avoids mixing functionalities into the same function, thereby making `test_rpcwhitelistdefault_permissions` less complex.
Each function is doing exactly what the function name suggests. IMO
💬 musaHaruna commented on pull request "test: Add test coverage for rpcwhitelistdefault when unset":
(https://github.com/bitcoin/bitcoin/pull/32079#discussion_r2000729321)
nit: I think the previous implementation is catching all the edge cases including unintended white spaces.
e.g when I add a space between `"getbestblockhash,getblockcount,"` , it causes the test to fail
` self.users = [
["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash, getblockcount,", "12345"],`
(https://github.com/bitcoin/bitcoin/pull/32079#discussion_r2000729321)
nit: I think the previous implementation is catching all the edge cases including unintended white spaces.
e.g when I add a space between `"getbestblockhash,getblockcount,"` , it causes the test to fail
` self.users = [
["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash, getblockcount,", "12345"],`
💬 musaHaruna commented on pull request "test: Add test coverage for rpcwhitelistdefault when unset":
(https://github.com/bitcoin/bitcoin/pull/32079#discussion_r2000733677)
nit: I think the previous implementation is catching all the edge cases including unintended white spaces.
e.g when I add a space between `"getbestblockhash,getblockcount,"` , it causes the test to fail
` self.users = [
["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash, getblockcount,", "12345"],`
(https://github.com/bitcoin/bitcoin/pull/32079#discussion_r2000733677)
nit: I think the previous implementation is catching all the edge cases including unintended white spaces.
e.g when I add a space between `"getbestblockhash,getblockcount,"` , it causes the test to fail
` self.users = [
["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash, getblockcount,", "12345"],`
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r2000738410)
Thanks!
I've added a commit, which sets Bash as the default shell globally.
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r2000738410)
Thanks!
I've added a commit, which sets Bash as the default shell globally.
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2732656120)
The recent feedback on using Bash by default has been addressed.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2732656120)
The recent feedback on using Bash by default has been addressed.
💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2000762116)
Fixed, thanks
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2000762116)
Fixed, thanks