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

💬 instagibbs commented on pull request "test: Extends MEMPOOL msg functional test":
(https://github.com/bitcoin/bitcoin/pull/28485#discussion_r1417479419)
If I comment out these lines the test still passes. Seems wrong based on text below at https://github.com/bitcoin/bitcoin/pull/28485/files#diff-c5c320cd909288d7cf2d82632c6bafcb9085413bfddf5edf361f288dbd76e0cbR153 ?

Based on my look at the code, in SendMessages we set m_last_inv_sequence = m_mempool.GetSequence() immediately on connection, even if we had no INVs to send. This means new connections can ask for anything historical, regardless of mempool message?

🤔 instagibbs reviewed a pull request: "test: Extends MEMPOOL msg functional test"
(https://github.com/bitcoin/bitcoin/pull/28485#pullrequestreview-1767820295)
I haven't done much review of https://github.com/bitcoin/bitcoin/pull/27675 , but I can't make the test case fail sensibly by commenting out the mempool message, and by the reading of the code.

🚀 achow101 merged a pull request: "test: Fix test by checking the actual exception instance"
(https://github.com/bitcoin/bitcoin/pull/28989)

💬 maflcko commented on pull request "test: doc: follow-up #28368":
(https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1417521141)
nit: use the format that clang-tidy understands and that is used elsewhere in the code?

```suggestion
/*m_mempool_limit_bypassed=*/ false,
```

💬 achow101 commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1843125339)
ACK ca09415e630f0f7de9160cab234bd5ba6968ff2d

🚀 achow101 merged a pull request: "rpc: encryptwallet help, mention HD seed rotation and backup requirement"
(https://github.com/bitcoin/bitcoin/pull/28980)