📝 fanquake opened a pull request: "[26.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29011)
Backports for 26.x. Currently:
* https://github.com/bitcoin/bitcoin/pull/28992
(https://github.com/bitcoin/bitcoin/pull/29011)
Backports for 26.x. Currently:
* https://github.com/bitcoin/bitcoin/pull/28992
💬 fanquake commented on pull request "ci: Use Ubuntu 24.04 Noble for asan,tsan,tidy,fuzz":
(https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1842974602)
Will be backported in #29011.
(https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1842974602)
Will be backported in #29011.
💬 brunoerg commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417402704)
In 964e1a8faec984df2799171bc2a9268a3765fbca: Any specific reason to increase the number of added addresses here?
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417402704)
In 964e1a8faec984df2799171bc2a9268a3765fbca: Any specific reason to increase the number of added addresses here?
💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417404342)
Tracepoints are OS level hooks that are not part of the software by default. Logging is about keeping the history of events that occurred along the software lifetime. This does not only involve errors, it also involves information about the soft activity.
Unlike logging, which can be enabled on-the-fly by simply calling an RPC command, users cannot enable tracepoints on their node without recompiling the software with a specific flag. Which requires compiling knowledge, forces the user to shutd
...
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1417404342)
Tracepoints are OS level hooks that are not part of the software by default. Logging is about keeping the history of events that occurred along the software lifetime. This does not only involve errors, it also involves information about the soft activity.
Unlike logging, which can be enabled on-the-fly by simply calling an RPC command, users cannot enable tracepoints on their node without recompiling the software with a specific flag. Which requires compiling knowledge, forces the user to shutd
...
💬 0xB10C commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417424421)
This was lowered to 1 tried and 1 new in https://github.com/bitcoin/bitcoin/pull/23084 as more than one address meant the test could sporadically fail if there's a collision.
@stratospher, I think, if you want to make this clearer, you could have this commit be a revert of https://github.com/bitcoin/bitcoin/commit/5825b34783545f9470d5ab94b87c918980715675 or note it in the commit message.
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417424421)
This was lowered to 1 tried and 1 new in https://github.com/bitcoin/bitcoin/pull/23084 as more than one address meant the test could sporadically fail if there's a collision.
@stratospher, I think, if you want to make this clearer, you could have this commit be a revert of https://github.com/bitcoin/bitcoin/commit/5825b34783545f9470d5ab94b87c918980715675 or note it in the commit message.
💬 brunoerg commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417428585)
Thanks, @0xB10C. An explanation in the commit message would be valuable.
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417428585)
Thanks, @0xB10C. An explanation in the commit message would be valuable.
💬 maflcko commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843018335)
Nice, but I am not sure if we want to go down to route to put everyone back into the makefile hell, because this will just make writing new fuzz tests bloaty and impossible to maintain/review (Who is going to read those repetitive and ugly 1k LOC of build code). (Unrelated: Not even sure how this will interact with cmake)
It seems easier to just put a one-line bash command into the readme to achieve the same, for anyone that needs it (oss-fuzz, fuzz-introspector, afl-lto, etc ...).
Happy
...
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843018335)
Nice, but I am not sure if we want to go down to route to put everyone back into the makefile hell, because this will just make writing new fuzz tests bloaty and impossible to maintain/review (Who is going to read those repetitive and ugly 1k LOC of build code). (Unrelated: Not even sure how this will interact with cmake)
It seems easier to just put a one-line bash command into the readme to achieve the same, for anyone that needs it (oss-fuzz, fuzz-introspector, afl-lto, etc ...).
Happy
...
💬 maflcko commented on pull request "fuzz: p2p: Detect peer deadlocks":
(https://github.com/bitcoin/bitcoin/pull/29009#issuecomment-1843021831)
> Should we do the same for `process_message`? would need to change it to use `ProcessMessagesOnce` as well.
Happy to review a pull request, or happy to include any patch here, that compiles, if someone writes it.
(https://github.com/bitcoin/bitcoin/pull/29009#issuecomment-1843021831)
> Should we do the same for `process_message`? would need to change it to use `ProcessMessagesOnce` as well.
Happy to review a pull request, or happy to include any patch here, that compiles, if someone writes it.
🚀 fanquake merged a pull request: "rpc: fix getrawtransaction segfault"
(https://github.com/bitcoin/bitcoin/pull/29003)
(https://github.com/bitcoin/bitcoin/pull/29003)
✅ fanquake closed an issue: "getrawtransaction xxxxxx.... 2 causes a segfault"
(https://github.com/bitcoin/bitcoin/issues/28986)
(https://github.com/bitcoin/bitcoin/issues/28986)
💬 dergoegge commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843034078)
I split the harnesses into individual files so that we can use the file list in `src/test/fuzz` as the harness list. It should be possible to use that to have a loop (?) in the makefile to build each binary instead of the hard-coded list I have here (I haven't figured out how to do that yet). If that works, adding a new fuzz harness becomes easier as no makefile would need to be edited.
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843034078)
I split the harnesses into individual files so that we can use the file list in `src/test/fuzz` as the harness list. It should be possible to use that to have a loop (?) in the makefile to build each binary instead of the hard-coded list I have here (I haven't figured out how to do that yet). If that works, adding a new fuzz harness becomes easier as no makefile would need to be edited.
💬 fanquake commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1843040475)
Added to #29011 for backporting to 26.x.
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1843040475)
Added to #29011 for backporting to 26.x.
🤔 pablomartin4btc reviewed a pull request: "rpc: fix getrawtransaction segfault"
(https://github.com/bitcoin/bitcoin/pull/29003#pullrequestreview-1767778706)
ACK 9075a446461ccbc446d21af778aac50b604f39b3 on new reg test [added](https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841662429)
(https://github.com/bitcoin/bitcoin/pull/29003#pullrequestreview-1767778706)
ACK 9075a446461ccbc446d21af778aac50b604f39b3 on new reg test [added](https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841662429)
📝 maflcko opened a pull request: "fuzz: Avoid timeout in bitdeque"
(https://github.com/bitcoin/bitcoin/pull/29012)
Avoid timeouts such as https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1842914664
This is done by:
* Limiting the maximum number of iterations if the maximum size of the container is "large" (see the magic numbers in the code).
* Check the equality only once. This should be fine, because if a crash were to happen in the equality check, but the crash doesn't happen if further iterations were run, the fuzz engine should eventually find the crash by truncating the fuzz input.
(https://github.com/bitcoin/bitcoin/pull/29012)
Avoid timeouts such as https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1842914664
This is done by:
* Limiting the maximum number of iterations if the maximum size of the container is "large" (see the magic numbers in the code).
* Check the equality only once. This should be fine, because if a crash were to happen in the equality check, but the crash doesn't happen if further iterations were run, the fuzz engine should eventually find the crash by truncating the fuzz input.
💬 fanquake commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1843059161)
Added to #28768 for backporting to 25.x.
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1843059161)
Added to #28768 for backporting to 25.x.
💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#issuecomment-1843065442)
> Needs rebase (silent merge conflict).
Thanks, rebased
(https://github.com/bitcoin/bitcoin/pull/28043#issuecomment-1843065442)
> Needs rebase (silent merge conflict).
Thanks, rebased
💬 maflcko commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843074084)
> turns out we can actually keep the map if we just don't add all the harness functions into it
Are you sure on this? I don't know how afl or fuzz-introspector detect reachable code paths, but I'd be surprised if the static analysis can follow a function pointer hidden in a map.
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1843074084)
> turns out we can actually keep the map if we just don't add all the harness functions into it
Are you sure on this? I don't know how afl or fuzz-introspector detect reachable code paths, but I'd be surprised if the static analysis can follow a function pointer hidden in a map.
🤔 hebasto reviewed a pull request: "build: Introduce internal kernel library"
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1767824380)
I've reviewed the first commit (7651c93749557839f7db3b73eb6df6357649e2d3).
Not counting the symbols from the `libbitcoin_crypto` library, the `libbitcoin_util` has undefined symbols as follows:
- on the master branch:
```
- `ArgsManager::AddArg`
- `ArgsManager::SelectConfigNetwork`
- `CKey::SignCompact`
- `CPubKey::RecoverCompact`
- `DecodeDestination`
- `gArgs`
- `G_TRANSLATION_FUN`
- `IsValidDestination`
- `PKHash::PKHash`
```
- for 7651c93749557839f7db3b73eb6df6357649e2d3:
``
...
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1767824380)
I've reviewed the first commit (7651c93749557839f7db3b73eb6df6357649e2d3).
Not counting the symbols from the `libbitcoin_crypto` library, the `libbitcoin_util` has undefined symbols as follows:
- on the master branch:
```
- `ArgsManager::AddArg`
- `ArgsManager::SelectConfigNetwork`
- `CKey::SignCompact`
- `CPubKey::RecoverCompact`
- `DecodeDestination`
- `gArgs`
- `G_TRANSLATION_FUN`
- `IsValidDestination`
- `PKHash::PKHash`
```
- for 7651c93749557839f7db3b73eb6df6357649e2d3:
``
...
💬 hebasto commented on pull request "build: Move `message.{h,cpp}` from `libbitcoin_util` to `libbitcoin_common`":
(https://github.com/bitcoin/bitcoin/pull/28549#issuecomment-1843078856)
Closing in favour of #28690.
(https://github.com/bitcoin/bitcoin/pull/28549#issuecomment-1843078856)
Closing in favour of #28690.
✅ hebasto closed a pull request: "build: Move `message.{h,cpp}` from `libbitcoin_util` to `libbitcoin_common`"
(https://github.com/bitcoin/bitcoin/pull/28549)
(https://github.com/bitcoin/bitcoin/pull/28549)