💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556065929)
It's not related to mac, so I put it back in. Although I'm not sure how useful it is to document this here, as it is documented more accurately in the afl++ docs already.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556065929)
It's not related to mac, so I put it back in. Although I'm not sure how useful it is to document this here, as it is documented more accurately in the afl++ docs already.
💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556066319)
Done
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556066319)
Done
💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
This comment led me to remove the steps entirely. They'll just invite bike-shedding over what works for different people, which is partly what we're trying to avoid with this effort in the first place.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
This comment led me to remove the steps entirely. They'll just invite bike-shedding over what works for different people, which is partly what we're trying to avoid with this effort in the first place.
💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556068835)
Kept something similar in the quickstart section at the top.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556068835)
Kept something similar in the quickstart section at the top.
💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3570508575)
* removed the macos steps (https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
* added a reference to the macos notes section in the quickstart section
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3570508575)
* removed the macos steps (https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
* added a reference to the macos notes section in the quickstart section
💬 Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570523079)
@plebhash it looks like you found a real bug. Because `BlockTemplate::createNewBlock` doesn't have a context param, it looks like its destroy method is not invoked until sv2-tp disconnects. So the node keeps holding on to templates even though the Template Provider already pruned them.
I'll open a PR to fix that.
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570523079)
@plebhash it looks like you found a real bug. Because `BlockTemplate::createNewBlock` doesn't have a context param, it looks like its destroy method is not invoked until sv2-tp disconnects. So the node keeps holding on to templates even though the Template Provider already pruned them.
I'll open a PR to fix that.
💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556093636)
> This comment led me to remove the steps entirely
Hmmm, so what happened to "best effort"?
> which is partly what we're trying to avoid with this effort in the first place
I thought we were aiming to provide Mac users a way to have at least basic fuzzing support.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556093636)
> This comment led me to remove the steps entirely
Hmmm, so what happened to "best effort"?
> which is partly what we're trying to avoid with this effort in the first place
I thought we were aiming to provide Mac users a way to have at least basic fuzzing support.
💬 plebhash commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428)
that's good to know!
but do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
from my understanding of capnp, I believe it should be sufficient to drop it from memory, but on the other hand there must be a reason for `destroy` to exist?
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428)
that's good to know!
but do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
from my understanding of capnp, I believe it should be sufficient to drop it from memory, but on the other hand there must be a reason for `destroy` to exist?
💬 maflcko commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556116259)
I think it is fine to keep the libfuzzer-nosan command, but no strong opinion. Seems like it works without the extra flags for most people, and any extra flags can be added in the future, if there is need to.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556116259)
I think it is fine to keep the libfuzzer-nosan command, but no strong opinion. Seems like it works without the extra flags for most people, and any extra flags can be added in the future, if there is need to.
📝 psam21 opened a pull request: "net: Execute Discover() when bind=0.0.0.0 or :: is set"
(https://github.com/bitcoin/bitcoin/pull/33935)
Fixes #31293
When using `-bind=0.0.0.0:port` or `-bind=[::]:port` (or their `-whitebind` equivalents), the `Discover()` function was not being executed because `bind_on_any` was set to false when explicit bind addresses were provided.
This meant that nodes using explicit "any" bind addresses would not discover and advertise their own local addresses to peers, even though they were effectively listening on all interfaces.
This commit checks if any of the explicitly provided `-bind` or `-
...
(https://github.com/bitcoin/bitcoin/pull/33935)
Fixes #31293
When using `-bind=0.0.0.0:port` or `-bind=[::]:port` (or their `-whitebind` equivalents), the `Discover()` function was not being executed because `bind_on_any` was set to false when explicit bind addresses were provided.
This meant that nodes using explicit "any" bind addresses would not discover and advertise their own local addresses to peers, even though they were effectively listening on all interfaces.
This commit checks if any of the explicitly provided `-bind` or `-
...
💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556122623)
Wouldn't the version with extra flags work for even more people?
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556122623)
Wouldn't the version with extra flags work for even more people?
💬 marcofleon commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556129865)
This works on my mac M3:
```bash
cmake -B fuzzbuild \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_BUILD_TYPE=Debug \
-DBUILD_FOR_FUZZING=ON \
-DSANITIZERS=fuzzer
-DAPPEND_LDFLAGS=-Wl,-no_warn_duplicate_libraries
```
I remember copying this from the original documentation when we transitioned to cmake (before the presets). It seems like everyone has their own way to make it work, s
...
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556129865)
This works on my mac M3:
```bash
cmake -B fuzzbuild \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_BUILD_TYPE=Debug \
-DBUILD_FOR_FUZZING=ON \
-DSANITIZERS=fuzzer
-DAPPEND_LDFLAGS=-Wl,-no_warn_duplicate_libraries
```
I remember copying this from the original documentation when we transitioned to cmake (before the presets). It seems like everyone has their own way to make it work, s
...
📝 Sjors opened a pull request: "mining: pass missing context to createNewBlock() and checkBlock()"
(https://github.com/bitcoin/bitcoin/pull/33936)
Adding a `context` parameter ensures that these methods are run in their own thread and don't block other calls.
Additionally this is required for `destroy` handling to work.
This fixes a bug where we would not release any (stale) templates until the client disconnects. This was first observed in https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3568788623.
The issue can be confirmed running [sv2-tp](https://github.com/stratum-mining/sv2-tp) and noticing in the log that destr
...
(https://github.com/bitcoin/bitcoin/pull/33936)
Adding a `context` parameter ensures that these methods are run in their own thread and don't block other calls.
Additionally this is required for `destroy` handling to work.
This fixes a bug where we would not release any (stale) templates until the client disconnects. This was first observed in https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3568788623.
The issue can be confirmed running [sv2-tp](https://github.com/stratum-mining/sv2-tp) and noticing in the log that destr
...
💬 marcofleon commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556140203)
Although, I thought the point was to reduce the number of issues that were constantly being opened wrt to mac fuzzing. In that case, I think it would make the most sense to have no instructions at all, as anything we put there will invite future issues/PRs.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556140203)
Although, I thought the point was to reduce the number of issues that were constantly being opened wrt to mac fuzzing. In that case, I think it would make the most sense to have no instructions at all, as anything we put there will invite future issues/PRs.
👍 TheCharlatan approved a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3500228951)
Re-ACK 78d6402458d7d14533444d893c989f0534a3896f
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3500228951)
Re-ACK 78d6402458d7d14533444d893c989f0534a3896f
💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3570652179)
If the important but breaking change in #33936 lands, then I might as well add the `getCoinbaseTx()` deletion here. It would be still be a separate commit, because it's useful to demonstrate equivalence before deletion.
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3570652179)
If the important but breaking change in #33936 lands, then I might as well add the `getCoinbaseTx()` deletion here. It would be still be a separate commit, because it's useful to demonstrate equivalence before deletion.
💬 maflcko commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556196974)
Yeah, no strong opinion. I just thought that the fuzzer+sanitizer version never worked on recent macOS versions, so removing it makes sense. Though, the libfuzzer-nosan worked for almost everyone, so it seems fine to keep. Having to deal with a pull/issue for that every few months seems fine, but I don't really expect a flood of those.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556196974)
Yeah, no strong opinion. I just thought that the fuzzer+sanitizer version never worked on recent macOS versions, so removing it makes sense. Though, the libfuzzer-nosan worked for almost everyone, so it seems fine to keep. Having to deal with a pull/issue for that every few months seems fine, but I don't really expect a flood of those.
✅ maflcko closed a pull request: "Update GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/33934)
(https://github.com/bitcoin/bitcoin/pull/33934)
💬 maflcko commented on pull request "Update GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/33934#issuecomment-3570673314)
thx, but you'll need to follow the contrib guidelines
(https://github.com/bitcoin/bitcoin/pull/33934#issuecomment-3570673314)
thx, but you'll need to follow the contrib guidelines
✅ maflcko closed a pull request: "net: Execute Discover() when bind=0.0.0.0 or :: is set"
(https://github.com/bitcoin/bitcoin/pull/33935)
(https://github.com/bitcoin/bitcoin/pull/33935)