💬 TheCharlatan commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851686045)
Nit: Is there one `coinbaseaux` too many here?
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851686045)
Nit: Is there one `coinbaseaux` too many here?
💬 TheCharlatan commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851693999)
In commit b35b23d823ac0fc43e4d137525167a7788ad672a:
Nit: I think we just directly call the new function here?
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851693999)
In commit b35b23d823ac0fc43e4d137525167a7788ad672a:
Nit: I think we just directly call the new function here?
💬 fanquake commented on pull request "build: Use `-ffile-prefix-map` in release builds only":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490602573)
Sure, but it'd be good to get a better explanation as to how Clang builds in oss-fuzz are broken, if Clang builds outside of oss-fuzz are working.
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490602573)
Sure, but it'd be good to get a better explanation as to how Clang builds in oss-fuzz are broken, if Clang builds outside of oss-fuzz are working.
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851712070)
Looks like https://github.com/bitcoin/bitcoin/pull/31337/checks passed CI recently, so maybe this was just a temporary issue?
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851712070)
Looks like https://github.com/bitcoin/bitcoin/pull/31337/checks passed CI recently, so maybe this was just a temporary issue?
💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851712986)
This change was premature since the interface is changed in a later commit. It also doesn't do anything, so I'm dropping it.
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1851712986)
This change was premature since the interface is changed in a later commit. It also doesn't do anything, so I'm dropping it.
💬 dergoegge commented on pull request "build: Use `-ffile-prefix-map` in release builds only":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490620693)
Here is a recent example of clang coverage working for me: http://bitcoind-fuzz.dergoegge.de:8000/bitcoin/harnesses/rpc/coverage_report/coverage/workdir/bitcoin/index.html
I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490620693)
Here is a recent example of clang coverage working for me: http://bitcoind-fuzz.dergoegge.de:8000/bitcoin/harnesses/rpc/coverage_report/coverage/workdir/bitcoin/index.html
I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2490632989)
I re-combined d31e5f9d9986ce1e80b014a1585ab0e944ebcc86 and d1972d751fa882960375050bfa7fc6f4043e0be4 so that the interface change is done along with the change to CreateNewBlock. This seems more clear than having them separate.
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2490632989)
I re-combined d31e5f9d9986ce1e80b014a1585ab0e944ebcc86 and d1972d751fa882960375050bfa7fc6f4043e0be4 so that the interface change is done along with the change to CreateNewBlock. This seems more clear than having them separate.
👍 dergoegge approved a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2450864181)
utACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2450864181)
utACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49
💬 Sjors commented on pull request "kernel: make m_tip_block std::optional":
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851743969)
The reason I did it differently was that `interfaces.cpp` the pattern `blah && blah.value()` makes it clear the first value is a pointer rather than a boolean. That's not clear here. That said, I don't think anyone would be confused either.
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851743969)
The reason I did it differently was that `interfaces.cpp` the pattern `blah && blah.value()` makes it clear the first value is a pointer rather than a boolean. That's not clear here. That said, I don't think anyone would be confused either.
💬 Sjors commented on pull request "kernel: make m_tip_block std::optional":
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851747612)
That's probably worth clarifying in a comment...
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851747612)
That's probably worth clarifying in a comment...
💬 Sjors commented on pull request "kernel: make m_tip_block std::optional":
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851750132)
I didn't think about it that deeply, but yes.
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851750132)
I didn't think about it that deeply, but yes.
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851750952)
Dropped the workaround, and it seems like using `pkgconf` is still broken, so we'd need to retain the workaround. We could also just update only the docs, and change the CI later if it starts breaking again.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851750952)
Dropped the workaround, and it seems like using `pkgconf` is still broken, so we'd need to retain the workaround. We could also just update only the docs, and change the CI later if it starts breaking again.
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851765882)
I'd presume you could only update the CI after GHA has updated their image?
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851765882)
I'd presume you could only update the CI after GHA has updated their image?
💬 hebasto commented on pull request "build: Use `-ffile-prefix-map` in release builds only":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490733585)
> I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
They [use](https://github.com/google/oss-fuzz/blob/51922e43fa80ae858fcd12aa84be3c0c75ed7b0d/infra/base-images/base-runner/coverage#L54-L55) the `-path-equivalence=...` option:
```bash
# Use path mapping, as $SRC directory from the builder is copied into $OUT/$SRC.
PATH_EQUIVALENCE_ARGS="-path-equivalence=/,$OUT"
```
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490733585)
> I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
They [use](https://github.com/google/oss-fuzz/blob/51922e43fa80ae858fcd12aa84be3c0c75ed7b0d/infra/base-images/base-runner/coverage#L54-L55) the `-path-equivalence=...` option:
```bash
# Use path mapping, as $SRC directory from the builder is copied into $OUT/$SRC.
PATH_EQUIVALENCE_ARGS="-path-equivalence=/,$OUT"
```
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270)
Probably. Added a note for that, dropping the package change, but now the build is failing again, with the same as #31334.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270)
Probably. Added a note for that, dropping the package change, but now the build is failing again, with the same as #31334.
💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851796441)
Or we can `brew update`. However, it might be time consuming.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851796441)
Or we can `brew update`. However, it might be time consuming.
💬 hodlinator commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851804865)
Still worried that this will backfire, but will investigate more and maybe spawn a new PR.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851804865)
Still worried that this will backfire, but will investigate more and maybe spawn a new PR.
💬 hodlinator commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851810120)
FYI: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp overloads 21-33
Okay with leaving as-is.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851810120)
FYI: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp overloads 21-33
Okay with leaving as-is.
💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851825568)
It seems that `unlink` is still required because the `pkg-config` package is preinstalled in the GHA image.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851825568)
It seems that `unlink` is still required because the `pkg-config` package is preinstalled in the GHA image.
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490781666)
This is the upstream issue https://github.com/actions/runner-images/issues/10984 and related fix: https://github.com/actions/runner-images/pull/11015.
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490781666)
This is the upstream issue https://github.com/actions/runner-images/issues/10984 and related fix: https://github.com/actions/runner-images/pull/11015.