📝 ismaelsadeeq opened a pull request: "BlockAssembler: return selected packages vsize and feerate"
(https://github.com/bitcoin/bitcoin/pull/30391)
This PR enables `BlockAssembler` to add all selected packages' feerate and vsize to a vector, and then return the vector as `vFeeratePerSize` in the `CBlockTemplate` struct.

This PR is the first step in the #30157 project.

The packages' vsize and fee rates are used in #30157 to determine the percentile fee rate of the top block in the mempool.

The first commit changes the data type of `CFeeRate` size from `uint32_t` to `uint64_t` because the `BlockAssembler::addPackageTxs` package size
...

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2208702356)
Addressed @dergoegge and @vasild comments. Ready for review again, reACKs would be appreciated!

💬 glozow commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2208707191)
concept ACK

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2208745034)
@instagibbs @sr-gi maybe @mzumsande. This will likely affect you. Any feedback?

⚠️ ismaelsadeeq opened an issue: "Improving Fee estimation tracking issue"
(https://github.com/bitcoin/bitcoin/issues/30392)
This is a tracking issue for an effort to improve fee estimation on Bitcoin Core.

For the conceptual motivation behind this work, see issue #27995 and the previous discussion on Delving Bitcoin: [Mempool-Based Fee Estimation on Bitcoin Core](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/13),

What to Review

- #30391
- #30275

Roadmap

- [ ] #30391
- [ ] Add Fee estimator Module / Mempool forecaster and unit tests
- [ ] Create `estimatefee` rpc an
...

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2208753220)
So the `Sv2Connman` is called, but `start()` isn't, so the mock `BindListenPort` doesn't happen.

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1665606867)
I looked into catching specifically `EAI_ADDRFAMILY` errors but this only applies on GNU Libc and is not POSIX. I checked windows and they have different errors. Rather than try to catalogue every possible system and libc implementation, the code now retries on any error (assuming you first tried with `AI_ADDRCONFIG` in the first place).

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1665607508)
Good suggestion thanks, implemented.

📝 maflcko opened a pull request: " net_processing: use existing RNG object in ProcessGetBlockData "
(https://github.com/bitcoin/bitcoin/pull/30393)
Two small rand fixups. The debug mode one is a follow-up to commit 7af95afa8bc3f36a37d082ef3475cb3e0bd3a0e4 and https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049. The RNG object one to commit 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1665615924)
First half done in https://github.com/bitcoin/bitcoin/pull/30393

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1665616205)
Done in https://github.com/bitcoin/bitcoin/pull/30393

💬 fjahr commented on pull request "validation: Check if mempool exists before size check in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/pull/30388#discussion_r1665621472)
nit: The type doesn't seem so complex that it has to be auto

```suggestions
CTxMemPool* mempool{m_active_chainstate->GetMempool()};
```

🤔 fjahr reviewed a pull request: "validation: Check if mempool exists before size check in ActivateSnapshot"
(https://github.com/bitcoin/bitcoin/pull/30388#pullrequestreview-2158787105)
ACK 33c48c106cf03ff62994ff5777a775982d90eab9

While this isn't something that can be hit by users I agree it makes sense conceptually. I am not sure this will help the performance of the tests much as I have laid out in my comment here: https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2207510865

💬 willcl-ark commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2208839479)
My build on x86_64 (excluding SHA256SUMS.part):

```
d806e1994bc873a975714ce75bca87fd3fcec4055e24d1ed0afe99ebc3503288 bitcoin-f59e9057e2aa-aarch64-linux-gnu-debug.tar.gz
a9844e6a4d3e86df69b98281c942ac01136b4b115fc6a2504740368c5d441fbd bitcoin-f59e9057e2aa-aarch64-linux-gnu.tar.gz
99712894848ef7a0b90b3a8582d617809e986abda202e9d4941ee970ca0d9a3d bitcoin-f59e9057e2aa-arm-linux-gnueabihf-debug.tar.gz
aec34ff47cf315949f2ddbf4eb0ed69dd2ec278b20ab7909a16ce48abd41e7ee bitcoin-f59e9057e2aa-arm-
...

💬 fjahr commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2208845865)
utACK b9ba1a73094f4ad593b527e23d2681f41c225397

👍 willcl-ark approved a pull request: "depends: build libevent with CMake"
(https://github.com/bitcoin/bitcoin/pull/29835#pullrequestreview-2158819949)
ACK f59e9057e2aa596b54cf9e85bab35c3ead137547

Tested locally and with guix build. Changes all look good to me, with equivalent options being set where possible.

There are not direct equivalents of `--disable-dependency-tracking` and `--enable-option-checking` for cmake, but this doesn't seem like a problem for us here.

💬 maflcko commented on pull request "refactor: Make uint256S(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2208857200)
> **transaction_identifier.h** - Fixed dormant bug in `TxidFromString()` where the `string_view` length wasn't respected(!).

This is known, see https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378. Thanks for picking it up!

Maybe submit the fix first?



> Realizing my GH history has minimum proof of work, I might have delayed creating a PR, but it felt timely as Testnet 4 is being worked on.

Not sure what this has to do with testnet 4. May be best to remove unrelate
...

💬 maflcko commented on pull request "validation: Check if mempool exists before size check in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/pull/30388#issuecomment-2208864426)
re-ACK 33c48c106cf03ff62994ff5777a775982d90eab9

🤔 fjahr reviewed a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2158854946)
tACK 29ab88dfe1c43af3620480c99cba844aa414023c

I reviewed the code and confirmed that the test fails as expected when the behavior change is not applied.

💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1665663972)
Huh, either I am confused or something isn't right here: The `parent` above is at height `SNAPSHOT_BASE_HEIGHT - 1` and now the child of this `parent` has a coinbase also at height `SNAPSHOT_BASE_HEIGHT - 1`? It seems this isn't validated so it's not a huge issue though.