💬 maflcko commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1819037403)
Whatever is done here, at some point something needs to be done on the 26.x branch to avoid a failing CI.

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399186219)
Yes, I was only suggesting to remove the names, e.g. change `uint64_t mempool_sequence` to `uint64_t /*unused*/`.

💬 TheCharlatan commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399189391)
Keeping around a dead branch for the purpose of fuzzing seems weird to me. Besides the `inBlock` case is still exercised from `processBlock`.

💬 maflcko commented on issue "fuzz, parse_iso8601: attempt to dereference an end-of-stream istreambuf_iterator":
(https://github.com/bitcoin/bitcoin/issues/28917#issuecomment-1819043199)
See https://github.com/boostorg/date_time/issues/190

💬 maflcko commented on issue "fuzz, parse_iso8601: attempt to dereference an end-of-stream istreambuf_iterator":
(https://github.com/bitcoin/bitcoin/issues/28917#issuecomment-1819049606)
> not sure why oss-fuzz doesn't find it.

oss-fuzz doesn't use gcc, but clang and libc++

💬 m3dwards commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1819053244)
@kashifs thank you for your review, fixed all the found typos and removed the `$ ` signs.

Yes, for the mainnet test of import mempool you would need to be fully synced. I've added a note about that and an extra call to `-getinfo` to confirm it.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399223399)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: can you order chunks by mining score?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399231122)
It would be good to explain the rationale for the miner score somewhere. Why a * b and b * a?

📝 hebasto opened a pull request: "build: Fix regression in "ARMv8 CRC32 intrinsics" test"
(https://github.com/bitcoin/bitcoin/pull/28919)
In the master branch, the `aarch64` binaries lack support for CRC32 intrinsics.

The `vmull_p64` is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a `+crypto` for architecture flags.

The regression was introduced in https://github.com/bitcoin/bitcoin/pull/26183 (v25.0).

The `./configure` script log excerpts:
- the master branch @ d752349029ec7a76f1fd440db2ec2e458d0f3c99:
```
checking whether C++ compiler accepts -march=armv8-a+crc
...

💬 sipa commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819105431)
@Sjors I believe most if not all of this PR will be rewritten, and split up into several components. The goal here is just to give an idea of the high-level interactions with other changes (wallet behavior, package validation/relay/RBF, ...). I don't think a detailed line-by-line code review at this stage is a good use of your time.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399244164)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: this should also list the cluster id, and maybe also the chunk number.

Could also add an argument limit the total number of vbytes.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399246551)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1 : Could this use `CompareMiningScore` or does it have a different goal?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399253500)
bf467f8286425b692a1736ab6d417d0ba6074658 Maybe make this rule 7. That seems a bit more clear than saying "previously this rule referred to fee rate"

💬 hebasto commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1819129629)
A failure in the "Win64 native, VS 2022" CI job is unrelated. See: https://github.com/bitcoin/bitcoin/pull/28905.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399257380)
"a higher feerate or (using clusters) mining score"

👍 BrandonOdiwuor approved a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1739931365)
tested ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457


💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399277318)
bf467f8286425b692a1736ab6d417d0ba6074658: history can be expanded:

```md
* Cluster mempool introduced, dropping rule 2 and introducing rule 7. As of **v27.0** ([PR 28676](https://github.com/bitcoin/bitcoin/pull/28676)).
```

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1399277913)
Ok removed the 0 padding in 1b0fa12fa5

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1819164214)
Updated to address luke's comment and added a test to ensure rpccookieperms are being applied

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399281690)
bf467f8286425b692a1736ab6d417d0ba6074658: `; new chunk feerate %s <= old chunk feerate`

💬 hebasto commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399309912)
> Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.

Mind sharing your reasoning behind this decision? Asking because the alternative looks more generic and will work if any other dependency package will switch to CMake.

---

A side note, not directly related to this PR: the CMake-based build system has no such an issue at all.