💬 maflcko commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3559732640)
concept ACK, fwiw
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3559732640)
concept ACK, fwiw
💬 maflcko commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3559735371)
Looks like there are a bunch of code review comments. Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3559735371)
Looks like there are a bunch of code review comments. Are you still working on this?
👍 hodlinator approved a pull request: "refactor: Header sync optimisations & simplifications"
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3489612258)
re-ACK de4242f47476769d0a7f3e79e8297ed2dd60d9a4
Only removed `const` from `HeadersGeneratorSetup::chain_start` to work around GCC 13 issue since my previous ACK (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3410905600).
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3489612258)
re-ACK de4242f47476769d0a7f3e79e8297ed2dd60d9a4
Only removed `const` from `HeadersGeneratorSetup::chain_start` to work around GCC 13 issue since my previous ACK (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3410905600).
💬 hodlinator commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2547438257)
Aha, that explains why I was getting other results when not rebasing onto latest master (https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3559658957).
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2547438257)
Aha, that explains why I was getting other results when not rebasing onto latest master (https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3559658957).
💬 brunoerg commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3559744710)
> Sounds good. So I guess this is up-for-grabs, and a dev with the latest macOS can just update the docs (https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md#macos-hints-for-libfuzzer) with the steps that work with that version?
ACK on doing it.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3559744710)
> Sounds good. So I guess this is up-for-grabs, and a dev with the latest macOS can just update the docs (https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md#macos-hints-for-libfuzzer) with the steps that work with that version?
ACK on doing it.
💬 maflcko commented on pull request "ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG":
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3559762513)
Looks like it worked, fwiw: https://github.com/bitcoin/bitcoin/actions/runs/19545916567/job/55966862251#step:6:151:
```
+ '[' 1 = 1 ']'
+ git log HEAD~10 -1 --format=%H
+ git log HEAD~10 -1 --format=%H
+ mapfile -t KEYS
+ git config user.email ci@ci.ci
+ git config user.name ci
+ gpg --keyserver hkps://keys.openpgp.org --recv-keys E777299FC265DD04793070EB944D35F9AC3DB76A D1DBF2C4B96F2DEBF4C16654410108112E7EA81F 152812300785C96444D3334D17565732E08E5E41 6B002C6EA3F91B1B0DF0C9BC8F617F1200
...
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3559762513)
Looks like it worked, fwiw: https://github.com/bitcoin/bitcoin/actions/runs/19545916567/job/55966862251#step:6:151:
```
+ '[' 1 = 1 ']'
+ git log HEAD~10 -1 --format=%H
+ git log HEAD~10 -1 --format=%H
+ mapfile -t KEYS
+ git config user.email ci@ci.ci
+ git config user.name ci
+ gpg --keyserver hkps://keys.openpgp.org --recv-keys E777299FC265DD04793070EB944D35F9AC3DB76A D1DBF2C4B96F2DEBF4C16654410108112E7EA81F 152812300785C96444D3334D17565732E08E5E41 6B002C6EA3F91B1B0DF0C9BC8F617F1200
...
✅ fanquake closed an issue: "cmake: (release) version handling is broken"
(https://github.com/bitcoin/bitcoin/issues/31898)
(https://github.com/bitcoin/bitcoin/issues/31898)
💬 fanquake commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-3559765520)
Closing for now, in favour of #33911, as we might reafactor this in some way. Will follow up there.
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-3559765520)
Closing for now, in favour of #33911, as we might reafactor this in some way. Will follow up there.
💬 l0rinc commented on pull request "clang-format: Set Bitcoin Core IncludeCategories":
(https://github.com/bitcoin/bitcoin/pull/33917#issuecomment-3559773920)
ACK fa7e222a23266e258d82e085f62cbf89d20dc8f3
Thanks for adjusting these.
After this PR we can remove the deprecated ones and add the new options in 17 by regenerating the config.
(https://github.com/bitcoin/bitcoin/pull/33917#issuecomment-3559773920)
ACK fa7e222a23266e258d82e085f62cbf89d20dc8f3
Thanks for adjusting these.
After this PR we can remove the deprecated ones and add the new options in 17 by regenerating the config.
💬 l0rinc commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3559791440)
> a dev with the latest macOS can just update the docs
Should I revive https://github.com/bitcoin/bitcoin/pull/32084?
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3559791440)
> a dev with the latest macOS can just update the docs
Should I revive https://github.com/bitcoin/bitcoin/pull/32084?
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3559900892)
Rebased ef1f96134098e73b47bfc988d878422917ceac1d -> a2d5f75a26976ec3292606bec149ef1d325383a2 ([blocktreestore_8](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_8) -> [blocktreestore_9](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_8..blocktreestore_9))
* Fixed conflict with #33724
* Updated coinstatsindex compatibility test to only check upgrading the version, not downgrading.
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3559900892)
Rebased ef1f96134098e73b47bfc988d878422917ceac1d -> a2d5f75a26976ec3292606bec149ef1d325383a2 ([blocktreestore_8](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_8) -> [blocktreestore_9](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_8..blocktreestore_9))
* Fixed conflict with #33724
* Updated coinstatsindex compatibility test to only check upgrading the version, not downgrading.
💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547563662)
> a call to GetShortID() without a preceding call to FillShortTxIDSelector() is well defined and will use shorttxidk... as 0
As far as I can see, that is not guaranteed in C++. `CBlockHeaderAndShortTxIDs` has a user-provided default constructor and `shorttxidk0`/`shorttxidk1` are without in-class initialization, so unless `FillShortTxIDSelector()` has run, they hold indeterminate values, not guaranteed zeros.
Either way, based on the serialization code and the constructors, the object is n
...
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547563662)
> a call to GetShortID() without a preceding call to FillShortTxIDSelector() is well defined and will use shorttxidk... as 0
As far as I can see, that is not guaranteed in C++. `CBlockHeaderAndShortTxIDs` has a user-provided default constructor and `shorttxidk0`/`shorttxidk1` are without in-class initialization, so unless `FillShortTxIDSelector()` has run, they hold indeterminate values, not guaranteed zeros.
Either way, based on the serialization code and the constructors, the object is n
...
💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547568059)
> The tests SaltedOutpointHasherBench* do not exist?
Please see the attached benchmarks in the PR description. They were part of the PR originally, but aren't really useful after this change, doesn't make sense to commit them.
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547568059)
> The tests SaltedOutpointHasherBench* do not exist?
Please see the attached benchmarks in the PR description. They were part of the PR originally, but aren't really useful after this change, doesn't make sense to commit them.
💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547575869)
I'm not really a fan of that, I explicitly try to make long and expressive commits - it can be soft-wrapped on the client-side by the vertically-challended IDEs :D
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2547575869)
I'm not really a fan of that, I explicitly try to make long and expressive commits - it can be soft-wrapped on the client-side by the vertically-challended IDEs :D
📝 willcl-ark reopened a pull request: "Clear out space on CentOS, depends, gui GHA job"
(https://github.com/bitcoin/bitcoin/pull/33514)
Fixes #33293
Clear out space on jobs running on GHA by deleteing unnecessary files.
Raised in #33293 which pointed to a solution like https://github.com/google/oss-fuzz/commit/b7f04d782277638a67bc44865de445977eed4708 which is adapted slightly here.
Only runs when cache provider (runner) is `gha`.
~~A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/18130629028/job/51596196163#step:6:2~~
A run applying to all jobs when using GHA can be seen here:
...
(https://github.com/bitcoin/bitcoin/pull/33514)
Fixes #33293
Clear out space on jobs running on GHA by deleteing unnecessary files.
Raised in #33293 which pointed to a solution like https://github.com/google/oss-fuzz/commit/b7f04d782277638a67bc44865de445977eed4708 which is adapted slightly here.
Only runs when cache provider (runner) is `gha`.
~~A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/18130629028/job/51596196163#step:6:2~~
A run applying to all jobs when using GHA can be seen here:
...
💬 willcl-ark commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3559986128)
> Hitting this again on my and @TheCharlatan's fork: [m3dwards/bitcoin/actions/runs/19539048875/job/55940226257](https://github.com/m3dwards/bitcoin/actions/runs/19539048875/job/55940226257)
Re-opening based on above report, can close again if someone else has another approach they would prefer.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3559986128)
> Hitting this again on my and @TheCharlatan's fork: [m3dwards/bitcoin/actions/runs/19539048875/job/55940226257](https://github.com/m3dwards/bitcoin/actions/runs/19539048875/job/55940226257)
Re-opening based on above report, can close again if someone else has another approach they would prefer.
💬 Christewart commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3560024890)
> It would be good to add a test case that demonstrates scriptPubKeys of this form (baremultisig and possible multisig in p2sh, with valid key push prior to invalid key) are spendable.
Hi AJ, are these the test vectors you had in mind?
```
[
"0 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501",
"1 0 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 2 CHEC
...
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3560024890)
> It would be good to add a test case that demonstrates scriptPubKeys of this form (baremultisig and possible multisig in p2sh, with valid key push prior to invalid key) are spendable.
Hi AJ, are these the test vectors you had in mind?
```
[
"0 0x47 0x3044022044dc17b0887c161bb67ba9635bf758735bdde503e4b0a0987f587f14a4e1143d022009a215772d49a85dae40d8ca03955af26ad3978a0ff965faa12915e9586249a501",
"1 0 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 2 CHEC
...
👍 TheCharlatan approved a pull request: "test: Retry download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3489961023)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3489961023)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899
👍 TheCharlatan approved a pull request: "clang-format: Set PackConstructorInitializers: CurrentLine"
(https://github.com/bitcoin/bitcoin/pull/33912#pullrequestreview-3489975713)
ACK fad0c76d0a109ab7b063f0d405588cf1e6c15e4d
(https://github.com/bitcoin/bitcoin/pull/33912#pullrequestreview-3489975713)
ACK fad0c76d0a109ab7b063f0d405588cf1e6c15e4d
💬 maflcko commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3560092109)
> > a dev with the latest macOS can just update the docs
>
> Should I revive [#32084](https://github.com/bitcoin/bitcoin/pull/32084)?
I think you found that it doesn't work reliably. If there truly is no single way to get fuzzing to work fully reliably on macOS, it seems best to remove the docs temporarily.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3560092109)
> > a dev with the latest macOS can just update the docs
>
> Should I revive [#32084](https://github.com/bitcoin/bitcoin/pull/32084)?
I think you found that it doesn't work reliably. If there truly is no single way to get fuzzing to work fully reliably on macOS, it seems best to remove the docs temporarily.