💬 Crypt-iQ commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461199887)
I have a mac and prefer option 2 also. The fuzz tests usually break each macOS update and I don't think it makes sense to maintain documentation for it. I occasionally find it useful just to see that a harness is working, but I can fix issues on my own as they come up.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461199887)
I have a mac and prefer option 2 also. The fuzz tests usually break each macOS update and I don't think it makes sense to maintain documentation for it. I occasionally find it useful just to see that a harness is working, but I can fix issues on my own as they come up.
💬 marcofleon commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461226122)
Concept ACK on option 2. I remember struggling on my mac when I first started fuzzing and the section we have in the docs didn't really help. Ended up just figuring it out on my own. These days pretty much all of my fuzzing happens on Linux.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461226122)
Concept ACK on option 2. I remember struggling on my mac when I first started fuzzing and the section we have in the docs didn't really help. Ended up just figuring it out on my own. These days pretty much all of my fuzzing happens on Linux.
🤔 stickies-v reviewed a pull request: "contrib: turn off compression of macOS SDK to fix determinism (across distros)"
(https://github.com/bitcoin/bitcoin/pull/32009#pullrequestreview-3393052598)
utACK 71b0f474eee3aceb8c17b9ffa7fa866ecb3a93af, Concept ACK on disabling compression to increase determinism.
71b0f474eee3aceb8c17b9ffa7fa866ecb3a93af seems like a strict win, no need to include what we don't use. I think conceptually 0a9b63b1f447bc31e6b56410f7c9c25abc7f5047 also makes sense: the increased size of the tarball should not be a blocked for increased determinism. However, on my machines (arm64 macOS 15.6 and x86_64 Debian Bookworm ) determinism seems to be _reduced_ without compr
...
(https://github.com/bitcoin/bitcoin/pull/32009#pullrequestreview-3393052598)
utACK 71b0f474eee3aceb8c17b9ffa7fa866ecb3a93af, Concept ACK on disabling compression to increase determinism.
71b0f474eee3aceb8c17b9ffa7fa866ecb3a93af seems like a strict win, no need to include what we don't use. I think conceptually 0a9b63b1f447bc31e6b56410f7c9c25abc7f5047 also makes sense: the increased size of the tarball should not be a blocked for increased determinism. However, on my machines (arm64 macOS 15.6 and x86_64 Debian Bookworm ) determinism seems to be _reduced_ without compr
...
💬 maflcko commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461244223)
No strong opinion, but I don't mind having docs that just say they only works with a specific brew llvm version and macOS version, and have them updated when they break.
Though, saying that libFuzzer is not supported on macOS seems also fine.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461244223)
No strong opinion, but I don't mind having docs that just say they only works with a specific brew llvm version and macOS version, and have them updated when they break.
Though, saying that libFuzzer is not supported on macOS seems also fine.
💬 maflcko commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3461248758)
Done in https://github.com/bitcoin/bitcoin/pull/33732
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3461248758)
Done in https://github.com/bitcoin/bitcoin/pull/33732
🤔 naiyoma reviewed a pull request: "wallet: Improve wallet creation error message with usage hint"
(https://github.com/bitcoin/bitcoin/pull/33124#pullrequestreview-3393091125)
I'm not sure this is necessary since the correct syntax can be found using `-help `
(https://github.com/bitcoin/bitcoin/pull/33124#pullrequestreview-3393091125)
I'm not sure this is necessary since the correct syntax can be found using `-help `
💬 Crypt-iQ commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461280539)
crACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461280539)
crACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e
🤔 TheCharlatan reviewed a pull request: "test: Add bitcoin-chainstate test for assumeutxo functionality"
(https://github.com/bitcoin/bitcoin/pull/33728#pullrequestreview-3392815899)
Concept ACK
Consolidating the tests into the regtest case seems fine to me, especially since this is just a utility to showcase and doogfood some of the kernel work.
(https://github.com/bitcoin/bitcoin/pull/33728#pullrequestreview-3392815899)
Concept ACK
Consolidating the tests into the regtest case seems fine to me, especially since this is just a utility to showcase and doogfood some of the kernel work.
💬 TheCharlatan commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472836534)
It is weaker, yes, but we similarly guard in other situations. We also still have the assertion in ActivateSnapshot to guard this condition at a more critical point in time.
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472836534)
It is weaker, yes, but we similarly guard in other situations. We also still have the assertion in ActivateSnapshot to guard this condition at a more critical point in time.
💬 TheCharlatan commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472614198)
Since this is an optional positional argument, I think I would drop the `-` prefix.
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472614198)
Since this is an optional positional argument, I think I would drop the `-` prefix.
✅ maflcko closed a pull request: "wallet: Improve wallet creation error message with usage hint"
(https://github.com/bitcoin/bitcoin/pull/33124)
(https://github.com/bitcoin/bitcoin/pull/33124)
💬 maflcko commented on pull request "wallet: Improve wallet creation error message with usage hint":
(https://github.com/bitcoin/bitcoin/pull/33124#issuecomment-3461323619)
Yeah, tend to agree. One will have to read the full help anyway to understand it.
(https://github.com/bitcoin/bitcoin/pull/33124#issuecomment-3461323619)
Yeah, tend to agree. One will have to read the full help anyway to understand it.
✅ maflcko closed a pull request: "[wip] [nomerge] [draft] 2510 msan zero"
(https://github.com/bitcoin/bitcoin/pull/33686)
(https://github.com/bitcoin/bitcoin/pull/33686)
💬 maflcko commented on pull request "[wip] [nomerge] [draft] 2510 msan zero":
(https://github.com/bitcoin/bitcoin/pull/33686#issuecomment-3461341894)
yeah, closing for now. It is a bit faster with the fast machines from the CI here, but it isn't going to pass in 6 hours, even with them, so moving to dedicated hardware without timeouts seems the only option.
(https://github.com/bitcoin/bitcoin/pull/33686#issuecomment-3461341894)
yeah, closing for now. It is a bit faster with the fast machines from the CI here, but it isn't going to pass in 6 hours, even with them, so moving to dedicated hardware without timeouts seems the only option.
💬 glozow commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461383230)
> I'd highly suggest waiting until @luke-jr can comment and weigh in on this directly before even considering hastily merging this.
We have previously given operators time to respond when we noticed problems with their DNS seed. I'm not sure what is giving people the impression that we're not doing the same here; Luke has been pinged both on this PR and directly through other platforms.
@luke-jr please update us here instead of on twitter.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461383230)
> I'd highly suggest waiting until @luke-jr can comment and weigh in on this directly before even considering hastily merging this.
We have previously given operators time to respond when we noticed problems with their DNS seed. I'm not sure what is giving people the impression that we're not doing the same here; Luke has been pinged both on this PR and directly through other platforms.
@luke-jr please update us here instead of on twitter.
💬 maflcko commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2432198820)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
I wonder if md gives a speedup, similar to the valgrind task? https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2451180232
Also on GHA runners, the timeout is short a few minutes of being hit?
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2432198820)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
I wonder if md gives a speedup, similar to the valgrind task? https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2451180232
Also on GHA runners, the timeout is short a few minutes of being hit?
💬 maflcko commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3461409851)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3461409851)
Are you still working on this?
💬 brunoerg commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461417256)
Concept ACK on option 2. I use macOS and I agree this is a pain -- The effort of updating the documentation is not worth it. The only downside of it would be not attracting macOS people to fuzzing development (?). But anyway, if someone really wants to get in fuzzing dev would have to have a linux at some point.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461417256)
Concept ACK on option 2. I use macOS and I agree this is a pain -- The effort of updating the documentation is not worth it. The only downside of it would be not attracting macOS people to fuzzing development (?). But anyway, if someone really wants to get in fuzzing dev would have to have a linux at some point.
💬 fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2472963951)
Pushed up `-md` for a look.
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2472963951)
Pushed up `-md` for a look.
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473009165)
We cannot phase out the `with-winpthreads?` argument, as during the recursive invocation it is unset:https://github.com/bitcoin/bitcoin/blob/f380e6251021cc7aa0c47f564147abea922dc050/contrib/guix/manifest.scm#L154-L157
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473009165)
We cannot phase out the `with-winpthreads?` argument, as during the recursive invocation it is unset:https://github.com/bitcoin/bitcoin/blob/f380e6251021cc7aa0c47f564147abea922dc050/contrib/guix/manifest.scm#L154-L157