💬 dergoegge commented on issue "RFC: Add multiprocess fuzz target":
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3510614348)
I'm working on making it easier to deploy custom testing workloads with fuzzamoto, so that something as simple as a different binary name isn't a struggle (i.e. doesn't require patches). See https://github.com/dergoegge/fuzzamoto/issues/48, which will hopefully also enable the use of python with our testing framework. I'll circle back here once that is done.
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3510614348)
I'm working on making it easier to deploy custom testing workloads with fuzzamoto, so that something as simple as a different binary name isn't a struggle (i.e. doesn't require patches). See https://github.com/dergoegge/fuzzamoto/issues/48, which will hopefully also enable the use of python with our testing framework. I'll circle back here once that is done.
🚀 fanquake merged a pull request: "ci: Use cmake --preset=dev-mode in test-each-commit task"
(https://github.com/bitcoin/bitcoin/pull/33823)
(https://github.com/bitcoin/bitcoin/pull/33823)
👍 dergoegge approved a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442187499)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a
(https://github.com/bitcoin/bitcoin/pull/33536#pullrequestreview-3442187499)
ACK dee7eec64389aa48daff6f7f3ecbd931af72050a
🚀 fanquake merged a pull request: "doc: reference fuzz coverage steps in quick-start"
(https://github.com/bitcoin/bitcoin/pull/33536)
(https://github.com/bitcoin/bitcoin/pull/33536)
💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2509736434)
I don't like this, why not just remove the const?
```patch
-const CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
+CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
```
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2509736434)
I don't like this, why not just remove the const?
```patch
-const CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
+CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash())))};
```
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3510658120)
> The test case producing the ubsan error should just be deleted in my opinion. It does not add anything in terms of coverage. How about:
I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
I understand the nonnull check can in theory be useful in low-level environments where a nullptr-deref would not be detected as access violation, but there are probably more users using modern systems that may wa
...
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3510658120)
> The test case producing the ubsan error should just be deleted in my opinion. It does not add anything in terms of coverage. How about:
I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
I understand the nonnull check can in theory be useful in low-level environments where a nullptr-deref would not be detected as access violation, but there are probably more users using modern systems that may wa
...
💬 l0rinc commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509751663)
Bumped into this again - if we remove the `const` from https://godbolt.org/z/fjc6be65M (`int& ref{Assert(a)};`) that seems to also fix the problem - why is that worse than introducing global suppression?
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509751663)
Bumped into this again - if we remove the `const` from https://godbolt.org/z/fjc6be65M (`int& ref{Assert(a)};`) that seems to also fix the problem - why is that worse than introducing global suppression?
💬 l0rinc commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510675433)
I also prefer extracting these smaller changes from big PR to smaller, dedicated ones to have *some* progress and to have focused discussions
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3510675433)
I also prefer extracting these smaller changes from big PR to smaller, dedicated ones to have *some* progress and to have focused discussions
💬 maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509780870)
Removing `const` over a bunch of places, including consensus code, seems like the wrong fix for a short-term temporary workaround for a long-fixed single compiler bug that affects a single version.
The suppression is not global, all other CI or local runs will still have the warning enabled.
Also, the temporary workaround can trivially and quickly removed in https://github.com/bitcoin/bitcoin/pull/33775, which is needed anyway for other reasons.
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509780870)
Removing `const` over a bunch of places, including consensus code, seems like the wrong fix for a short-term temporary workaround for a long-fixed single compiler bug that affects a single version.
The suppression is not global, all other CI or local runs will still have the warning enabled.
Also, the temporary workaround can trivially and quickly removed in https://github.com/bitcoin/bitcoin/pull/33775, which is needed anyway for other reasons.
💬 fanquake commented on pull request "doc: Correct `pkgin` command usage on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510723650)
ACK 0698c6b494de0e28c9b909585905aab5b187286e
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510723650)
ACK 0698c6b494de0e28c9b909585905aab5b187286e
🚀 fanquake merged a pull request: "doc: Correct `pkgin` command usage on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/33827)
(https://github.com/bitcoin/bitcoin/pull/33827)
💬 stickies-v commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509810054)
I hadn't thought of the reduced-height reorg scenario, thanks for that. Reorg-safety in general is still an unsolved problem for the public interface, and this is another case of that.
Note: the problem you report already exists for objects based on the `Range` class whenever `size()` depends on chain length, such as for `Chain::Entries` with `Chain::Entries::back()` being able to throw a `std::runtime_error`.
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509810054)
I hadn't thought of the reduced-height reorg scenario, thanks for that. Reorg-safety in general is still an unsolved problem for the public interface, and this is another case of that.
Note: the problem you report already exists for objects based on the `Range` class whenever `size()` depends on chain length, such as for `Chain::Entries` with `Chain::Entries::back()` being able to throw a `std::runtime_error`.
💬 l0rinc commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509811677)
> Removing const over a bunch of places
k, I had the impression this was the only one 👍
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2509811677)
> Removing const over a bunch of places
k, I had the impression this was the only one 👍
✅ fanquake closed a pull request: "guix: build glibc with `--enable-static-pie`"
(https://github.com/bitcoin/bitcoin/pull/33821)
(https://github.com/bitcoin/bitcoin/pull/33821)
💬 fanquake commented on pull request "guix: build glibc with `--enable-static-pie`":
(https://github.com/bitcoin/bitcoin/pull/33821#issuecomment-3510739654)
Given more patching needed, wont bother with this.
(https://github.com/bitcoin/bitcoin/pull/33821#issuecomment-3510739654)
Given more patching needed, wont bother with this.
💬 fanquake commented on pull request "doc: Correct `pkgin` command usage on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510748600)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33827#issuecomment-3510748600)
Backported to `30.x` in #33609.
✅ fanquake closed an issue: "fuzz: AddressSanitizer: odr-violation typeinfo name for CCoinsViewBacked"
(https://github.com/bitcoin/bitcoin/issues/32995)
(https://github.com/bitcoin/bitcoin/issues/32995)
💬 fanquake commented on issue "fuzz: AddressSanitizer: odr-violation typeinfo name for CCoinsViewBacked":
(https://github.com/bitcoin/bitcoin/issues/32995#issuecomment-3510749704)
This was either solved @ 21.1.x or user error.
(https://github.com/bitcoin/bitcoin/issues/32995#issuecomment-3510749704)
This was either solved @ 21.1.x or user error.
💬 TheCharlatan commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509842466)
Yes, I was merely mentioning it, because the `Tip` and `Genesis` methods in current master at least provided a bit better integrity. I don't think this is important at all though. External callers should provide synchronization for this. So the two calls should just be removed imo.
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2509842466)
Yes, I was merely mentioning it, because the `Tip` and `Genesis` methods in current master at least provided a bit better integrity. I don't think this is important at all though. External callers should provide synchronization for this. So the two calls should just be removed imo.
🚀 fanquake merged a pull request: "scripted-diff: Remove obsolete comment"
(https://github.com/bitcoin/bitcoin/pull/33826)
(https://github.com/bitcoin/bitcoin/pull/33826)