💬 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
...

💬 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.

🚀 fanquake merged a pull request: "rpc: fix getrawtransaction segfault"
(https://github.com/bitcoin/bitcoin/pull/29003)

✅ fanquake closed an issue: "getrawtransaction xxxxxx.... 2 causes a segfault"
(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.

💬 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.

🤔 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)

📝 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.

💬 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.

💬 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

💬 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.

🤔 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:
``
...

💬 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.

✅ hebasto closed a pull request: "build: Move `message.{h,cpp}` from `libbitcoin_util` to `libbitcoin_common`"
(https://github.com/bitcoin/bitcoin/pull/28549)

💬 sipa commented on pull request "fuzz: Avoid timeout in bitdeque":
(https://github.com/bitcoin/bitcoin/pull/29012#issuecomment-1843087428)
utACK fad1903b8a85506378101c1f857ba47b4a058fb4

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1417498148)
> So, with the current code, having 4 peers would never trigger it, right? Because median will always be 0.

Yes and this sort of (on accident) mimics how/when we trigger the warning on master at 5 or more samples: https://github.com/bitcoin/bitcoin/blob/dde7ac5c704688c8a9af29bd07e5ae8114824ce7/src/timedata.cpp#L77

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1417498497)
Done

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1417504050)
I see your point but I think I would like to keep discussion about improving the warning condition to a future PR. The current adjusted time mechanism has the same flaw and is even worse as it stops sampling entirely after 200 samples.

Maybe we can collect some stats on how often outbound connections rotate and how many samples from which connections are actually useful to inform such a change.

---

I will add the release note.

💬 achow101 commented on pull request "test: Fix test by checking the actual exception instance":
(https://github.com/bitcoin/bitcoin/pull/28989#issuecomment-1843111898)
ACK 55e3dc3e03510e97caba1547a82e3e022b0bbd42

📝 ismaelsadeeq opened a pull request: "test: doc: follow-up #28368"
(https://github.com/bitcoin/bitcoin/pull/29013)
This is a simple PR that does two things
1. Fixes #29000 by waiting for the fee estimator to catch up after `removeForBlock` calls before calling `estimateFee` in the `BlockPolicyEstimates` unit test.

2. Addressed some outstanding review comments from #28368
- Updated `NewMempoolTransactionInfo::m_from_disconnected_block` to `NewMempoolTransactionInfo::m_mempool_limit_bypassed` which now correctly indicates what the boolean does.
- Changed input of `processTransaction`'s tx_info `m_su
...