💬 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
```
💬 brunoerg commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803693573)
> Would be good to explain this a bit more, to make sure this is not a bug.
I think fuzz can generate subnets with a zone identifier, like: "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121", which is a valid one. However, the lookup call might change the zone identifier. In my machine (macOS), "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121" becomes "676:c962:7962:b787:b392:fed8:7058:c500%31097/121" after lookup and it makes the assertion fails.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803693573)
> Would be good to explain this a bit more, to make sure this is not a bug.
I think fuzz can generate subnets with a zone identifier, like: "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121", which is a valid one. However, the lookup call might change the zone identifier. In my machine (macOS), "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121" becomes "676:c962:7962:b787:b392:fed8:7058:c500%31097/121" after lookup and it makes the assertion fails.
💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-1803694990)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-1803694990)
Concept ACK
💬 hebasto commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387909280)
Thanks! Done.
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387909280)
Thanks! Done.
💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#issuecomment-1803710156)
lgtm ACK 9f208c017174132dbefbc917aa9824c279382597
(https://github.com/bitcoin/bitcoin/pull/28826#issuecomment-1803710156)
lgtm ACK 9f208c017174132dbefbc917aa9824c279382597
💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803717471)
Ah ok. And about the efficiency, is it possible to keep the fuzz input format unchanged instead of using a string representation (Maybe via a string-lookup roundtrip)? I wonder if that'd be more efficient.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803717471)
Ah ok. And about the efficiency, is it possible to keep the fuzz input format unchanged instead of using a string representation (Maybe via a string-lookup roundtrip)? I wonder if that'd be more efficient.
🚀 fanquake merged a pull request: "ci: Switch IWYU to `clang_17` branch"
(https://github.com/bitcoin/bitcoin/pull/28826)
(https://github.com/bitcoin/bitcoin/pull/28826)
👍 ismaelsadeeq approved a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1722502299)
ACK 2be5e799ba623f969f5ffc59667a5bc6799df290 🥘
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1722502299)
ACK 2be5e799ba623f969f5ffc59667a5bc6799df290 🥘