💬 dergoegge commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803538946)
> I can add steps to reproduce, if you want.
That would be great.
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803538946)
> I can add steps to reproduce, if you want.
That would be great.
💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1803550311)
Another thing to note, is that the code being patched here, has been deleted entirely upstream: https://github.com/qt/qtbase/commit/329db8b64f17c8ef013c586cea1f1c5b49c4a4b7.
> macOS: Remove fallback defines for MAC_OS_X_VERSION_MIN_REQUIRED
> Availability.h from the platform SDK should take care of this these days.
So this points to there still being an underlying issue, and workaround will "break" if/when we start using newer versions of Qt.
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1803550311)
Another thing to note, is that the code being patched here, has been deleted entirely upstream: https://github.com/qt/qtbase/commit/329db8b64f17c8ef013c586cea1f1c5b49c4a4b7.
> macOS: Remove fallback defines for MAC_OS_X_VERSION_MIN_REQUIRED
> Availability.h from the platform SDK should take care of this these days.
So this points to there still being an underlying issue, and workaround will "break" if/when we start using newer versions of Qt.
🚀 fanquake merged a pull request: "test: Add missing wait for version to be sent in add_outbound_p2p_connection"
(https://github.com/bitcoin/bitcoin/pull/28822)
(https://github.com/bitcoin/bitcoin/pull/28822)
💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803592254)
> I did some experiments and I decided to keep the lookup functions. Without it, I still get discrepances even avoiding the comparison when we call Ban with an invalid subnet/netaddr.
Would be good to explain this a bit more, to make sure this is not a bug.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803592254)
> I did some experiments and I decided to keep the lookup functions. Without it, I still get discrepances even avoiding the comparison when we call Ban with an invalid subnet/netaddr.
Would be good to explain this a bit more, to make sure this is not a bug.
💬 fanquake commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1803597897)
> Could this moment be now?
Feel free to open an issue for broader discussion. Given the overhead of supporting it is low, I don't see a pressing reason to do so. There is also a difference between us officially dropping support, and purposefully breaking the ability to compile for certain targets.
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1803597897)
> Could this moment be now?
Feel free to open an issue for broader discussion. Given the overhead of supporting it is low, I don't see a pressing reason to do so. There is also a difference between us officially dropping support, and purposefully breaking the ability to compile for certain targets.
📝 fanquake opened a pull request: "ci: win64 task does use boost:process"
(https://github.com/bitcoin/bitcoin/pull/28829)
It passes `--enable-external-signer`.
(https://github.com/bitcoin/bitcoin/pull/28829)
It passes `--enable-external-signer`.
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803615609)
Steps to reproduce flamegraph:
* Compile fuzz tests normally (libFuzzer recommended)
* `perf record`, for example via `FUZZ=mocked_descriptor_parse perf record -g --call-graph dwarf -F 1001 ./src/test/fuzz/fuzz -runs=2 /tmp/fuzz_input.dat` (`-runs` can be increased to discount an expensive global setup initialization, `--call-graph` and frequency can be modified depending on your system)
* Load the data as a flamegraph: `hotspot ./perf.data`
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803615609)
Steps to reproduce flamegraph:
* Compile fuzz tests normally (libFuzzer recommended)
* `perf record`, for example via `FUZZ=mocked_descriptor_parse perf record -g --call-graph dwarf -F 1001 ./src/test/fuzz/fuzz -runs=2 /tmp/fuzz_input.dat` (`-runs` can be increased to discount an expensive global setup initialization, `--call-graph` and frequency can be modified depending on your system)
* Load the data as a flamegraph: `hotspot ./perf.data`
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803617915)
cc @darosior about `mocked_descriptor_parse`
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803617915)
cc @darosior about `mocked_descriptor_parse`
💬 darosior commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803618906)
Yeah i was looking into it. It's surprising we are spending so long inside the dup key check. Maybe we re-introduced the issue fixed by https://github.com/bitcoin/bitcoin/pull/25540.
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803618906)
Yeah i was looking into it. It's surprising we are spending so long inside the dup key check. Maybe we re-introduced the issue fixed by https://github.com/bitcoin/bitcoin/pull/25540.
💬 maflcko commented on pull request "ci: win64 task does use boost:process":
(https://github.com/bitcoin/bitcoin/pull/28829#issuecomment-1803619984)
lgtm ACK 5f0bf2ef69a607433f58de3364dee10ad347f3e1
(https://github.com/bitcoin/bitcoin/pull/28829#issuecomment-1803619984)
lgtm ACK 5f0bf2ef69a607433f58de3364dee10ad347f3e1
💬 stickies-v commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1387845814)
nit: I think this docstring should now be removed, I don't think the constructor requires a mempool.cs lock anymore?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1387845814)
nit: I think this docstring should now be removed, I don't think the constructor requires a mempool.cs lock anymore?
🚀 fanquake merged a pull request: "ci: win64 task does use boost:process"
(https://github.com/bitcoin/bitcoin/pull/28829)
(https://github.com/bitcoin/bitcoin/pull/28829)
📝 TheCharlatan opened a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830)
The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop.
This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797.
(https://github.com/bitcoin/bitcoin/pull/28830)
The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop.
This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797.
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1803653460)
Rebased 1a589335617944b755235597aafc254e28e4acf9 -> 40e151697d3d1ed594364498d9e6220219d9b545 ([kernelInternalLib_4](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4) -> [kernelInternalLib_5](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_4..kernelInternalLib_5))
* Fixed merge conflict.
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1803653460)
Rebased 1a589335617944b755235597aafc254e28e4acf9 -> 40e151697d3d1ed594364498d9e6220219d9b545 ([kernelInternalLib_4](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4) -> [kernelInternalLib_5](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_4..kernelInternalLib_5))
* Fixed merge conflict.
👍 darosior approved a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1722332060)
utACK fa520848da6d718a07368b42b1a44bd2515e6e5a
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1722332060)
utACK fa520848da6d718a07368b42b1a44bd2515e6e5a
💬 0xB10C commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803679113)
I don't think this adds any value here.
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803679113)
I don't think this adds any value here.
💬 maflcko commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803684814)
Closing for now due to controversy.
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803684814)
Closing for now due to controversy.
✅ maflcko closed a pull request: "doc: uppercase title for "Assumeutxo“"
(https://github.com/bitcoin/bitcoin/pull/28828)
(https://github.com/bitcoin/bitcoin/pull/28828)
💬 maflcko commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803686986)
If you are looking for other good first issues:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests. Once that
...
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803686986)
If you are looking for other good first issues:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests. Once that
...
💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387897830)
```suggestion
${CI_RETRY_EXE} git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_TIDY_LLVM_V /include-what-you-use
```
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387897830)
```suggestion
${CI_RETRY_EXE} git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_TIDY_LLVM_V /include-what-you-use
```