💬 frankomosh commented on pull request "doc: reference fuzz coverage steps in quick-start":
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2508642020)
done. Thanks
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2508642020)
done. Thanks
💬 frankomosh commented on pull request "doc: reference fuzz coverage steps in quick-start":
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2508643943)
I don't think they are super helpful. I just removed them in the new commit.
(https://github.com/bitcoin/bitcoin/pull/33536#discussion_r2508643943)
I don't think they are super helpful. I just removed them in the new commit.
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3509389963)
Thank you @hodlinator!
In my last push (184c54c):
- Push hodlinator's fix as a co-authored commit on top (I slightly changed the diff to align with the style of `HeadersGeneratorSetup`). I can squash the last two commits if that's preferred.
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3509389963)
Thank you @hodlinator!
In my last push (184c54c):
- Push hodlinator's fix as a co-authored commit on top (I slightly changed the diff to align with the style of `HeadersGeneratorSetup`). I can squash the last two commits if that's preferred.
💬 hodlinator commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3509899129)
> * Push hodlinator's fix as a co-authored commit on top (I slightly changed the diff to align with the style of `HeadersGeneratorSetup`). I can squash the last two commits if that's preferred.
Thanks for taking my change! Technically I think the repo policy is that every commit should build on all CI, see for example https://github.com/bitcoin/bitcoin/pull/33785/commits/fae1d99651e29341e486a10e6340335c71a2144e which adds `const&` in the same change as disabling the errors, instead of doi
...
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3509899129)
> * Push hodlinator's fix as a co-authored commit on top (I slightly changed the diff to align with the style of `HeadersGeneratorSetup`). I can squash the last two commits if that's preferred.
Thanks for taking my change! Technically I think the repo policy is that every commit should build on all CI, see for example https://github.com/bitcoin/bitcoin/pull/33785/commits/fae1d99651e29341e486a10e6340335c71a2144e which adds `const&` in the same change as disabling the errors, instead of doi
...
💬 maflcko commented on pull request "guix: build glibc with `--enable-static-pie`":
(https://github.com/bitcoin/bitcoin/pull/33821#issuecomment-3510076340)
Looks like the error was:
```
...
The following derivations will be built:
/gnu/store/mv6c9hq3xpl208kwdj1akzy3md11fn8d-glibc-cross-riscv64-linux-gnu-2.31.drv
/gnu/store/q96z7xnxs4py883rasgyy6hxkcp11cdi-gcc-cross-riscv64-linux-gnu-13.3.0.drv
/gnu/store/ia37k6ramx87qg6vi2gffap5ikkzjhaj-riscv64-linux-gnu-toolchain-13.3.0.drv
building /gnu/store/mv6c9hq3xpl208kwdj1akzy3md11fn8d-glibc-cross-riscv64-linux-gnu-2.31.drv...
note: keeping build directory `/guix_temp_dir/guix-build-glibc-
...
(https://github.com/bitcoin/bitcoin/pull/33821#issuecomment-3510076340)
Looks like the error was:
```
...
The following derivations will be built:
/gnu/store/mv6c9hq3xpl208kwdj1akzy3md11fn8d-glibc-cross-riscv64-linux-gnu-2.31.drv
/gnu/store/q96z7xnxs4py883rasgyy6hxkcp11cdi-gcc-cross-riscv64-linux-gnu-13.3.0.drv
/gnu/store/ia37k6ramx87qg6vi2gffap5ikkzjhaj-riscv64-linux-gnu-toolchain-13.3.0.drv
building /gnu/store/mv6c9hq3xpl208kwdj1akzy3md11fn8d-glibc-cross-riscv64-linux-gnu-2.31.drv...
note: keeping build directory `/guix_temp_dir/guix-build-glibc-
...
💬 maflcko commented on issue "malloc: Failed to allocate segment from range group - out of space":
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3510202526)
> Can somebody else reproduce this?
Does it only happen on macOS, or also on Linux?
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3510202526)
> Can somebody else reproduce this?
Does it only happen on macOS, or also on Linux?
💬 maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#issuecomment-3510216185)
Is this rfm with 3 acks?
(https://github.com/bitcoin/bitcoin/pull/33785#issuecomment-3510216185)
Is this rfm with 3 acks?
💬 maflcko commented on pull request "scripted-diff: Remove obsolete comment":
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3510244379)
lgtm ACK 36724205fc1f226d7b5493ed0212c336e7f2ae84
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3510244379)
lgtm ACK 36724205fc1f226d7b5493ed0212c336e7f2ae84
👍 hebasto approved a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3441742819)
ACK f06c6e18983139dd63873b3537d2f87b8c6ec752.
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3441742819)
ACK f06c6e18983139dd63873b3537d2f87b8c6ec752.
💬 maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509568255)
Just for ref, one could write this to please clang-format, but not sure if it is worth it:
```cpp
#define NONFATAL_UNREACHABLE() \
do { \
throw NonFatalCheckError{ \
"Unreachable code reached (non-fatal)", \
std::source_location::current(), \
}; \
} while (0)
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509568255)
Just for ref, one could write this to please clang-format, but not sure if it is worth it:
```cpp
#define NONFATAL_UNREACHABLE() \
do { \
throw NonFatalCheckError{ \
"Unreachable code reached (non-fatal)", \
std::source_location::current(), \
}; \
} while (0)
💬 stickies-v commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509575390)
> The previous calls to tip and genesis were at least thread safe
With previous, do you mean master? I'm not sure how the current calls are not thread safe? These are read-only calls without side effects? And both in master and this PR, the caller has no guarantees that the returned tip is indeed the tip because cs_main isn't exposed (regardless of threads)?
> I wonder if we should even keep these methods now.
I agree, but I want to generalize that even more. I think `Range` objects sho
...
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509575390)
> The previous calls to tip and genesis were at least thread safe
With previous, do you mean master? I'm not sure how the current calls are not thread safe? These are read-only calls without side effects? And both in master and this PR, the caller has no guarantees that the returned tip is indeed the tip because cs_main isn't exposed (regardless of threads)?
> I wonder if we should even keep these methods now.
I agree, but I want to generalize that even more. I think `Range` objects sho
...
💬 maflcko commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509606630)
> With previous, do you mean master? I'm not sure how the current calls are not thread safe? These are read-only calls without side effects? And both in master and this PR, the caller has no guarantees that the returned tip is indeed the tip because cs_main isn't exposed (regardless of threads)?
I think there could be a rare and theoretical race where the function does not return any tip, but rather throws `std::runtime_error("No entry in the chain at the provided height");` when the height i
...
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509606630)
> With previous, do you mean master? I'm not sure how the current calls are not thread safe? These are read-only calls without side effects? And both in master and this PR, the caller has no guarantees that the returned tip is indeed the tip because cs_main isn't exposed (regardless of threads)?
I think there could be a rare and theoretical race where the function does not return any tip, but rather throws `std::runtime_error("No entry in the chain at the provided height");` when the height i
...
💬 stickies-v commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3510516230)
Concept ACK for standardizing CI workflows around `dev-mode` and for increasing kernel coverage. CI runtimes seem acceptable.
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3510516230)
Concept ACK for standardizing CI workflows around `dev-mode` and for increasing kernel coverage. CI runtimes seem acceptable.
🤔 janb84 reviewed a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442038438)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a
The author removed the duplicate and redundant code coverage instructions which made me disagree with this PR. The author now only references that there are instructions on generating code coverage for fuzzing elsewhere in the documentation, which seems to me as a reasonable thing and good addition.
Thank you for changing this.
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442038438)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a
The author removed the duplicate and redundant code coverage instructions which made me disagree with this PR. The author now only references that there are instructions on generating code coverage for fuzzing elsewhere in the documentation, which seems to me as a reasonable thing and good addition.
Thank you for changing this.
✅ fanquake closed an issue: "RFC: Cancelling waitNext calls in the IPC mining interface"
(https://github.com/bitcoin/bitcoin/issues/33575)
(https://github.com/bitcoin/bitcoin/issues/33575)
🚀 fanquake merged a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676)
(https://github.com/bitcoin/bitcoin/pull/33676)
💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510573430)
`84b2ad0334...9989853447`: rebase due to conflicts
> I don't think it makes much sense to split this out; it doesn't reduce the size of the parent PR meaningfully,
I agree it does not help much. It helps _a little_. And that is the point - to get some smaller changes out of the main PR.
> and renaming a function doesn't really seem like it justifies a PR on its own.
Having this in its own PR helped have a [fresh discussion](https://github.com/bitcoin/bitcoin/pull/33565#discussion_r24
...
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510573430)
`84b2ad0334...9989853447`: rebase due to conflicts
> I don't think it makes much sense to split this out; it doesn't reduce the size of the parent PR meaningfully,
I agree it does not help much. It helps _a little_. And that is the point - to get some smaller changes out of the main PR.
> and renaming a function doesn't really seem like it justifies a PR on its own.
Having this in its own PR helped have a [fresh discussion](https://github.com/bitcoin/bitcoin/pull/33565#discussion_r24
...
👍 rkrux approved a pull request: "scripted-diff: Remove obsolete comment"
(https://github.com/bitcoin/bitcoin/pull/33826#pullrequestreview-3442161682)
crACK 36724205fc1f226d7b5493ed0212c336e7f2ae84
(https://github.com/bitcoin/bitcoin/pull/33826#pullrequestreview-3442161682)
crACK 36724205fc1f226d7b5493ed0212c336e7f2ae84
✅ fanquake closed a pull request: "Check required interfaces before generating manpages"
(https://github.com/bitcoin/bitcoin/pull/33828)
(https://github.com/bitcoin/bitcoin/pull/33828)
💬 fanquake commented on pull request "Check required interfaces before generating manpages":
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3510609561)
This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3510609561)
This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.