💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399123100)
In commit "depends: build libmultiprocess with position independant code" (c3a962be10a98192a93518b0222c21fc7f115404)

I don't think I really understand what's going on here, but this seems like a reasonable change given that `--with-pic` seems to be used on so many other depends builds [^1]. I guess it is a little unclear why in the other depends builds, the `--with-pic` option seems to be selectively applied for individual platforms like linux, freebsd, netbsd, openbsd (and never darwin), whi
...

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399137900)
0b284b5fd29c06d46c1ec60ea7e1bcd07f36feb1: it would be more practical to (also) have a lookup by transaction hash

⚠️ dergoegge opened an issue: "fuzz, parse_iso8601: attempt to dereference an end-of-stream istreambuf_iterator"
(https://github.com/bitcoin/bitcoin/issues/28917)
Ran into this crash on my own infra, not sure why oss-fuzz doesn't find it.

```
$ echo "MjIyMw0NDQ0NDQ0NDQ0NDQ0NDcIn" | base64 --decode > parse_iso8601-46463936b8a32173e167a89aad1ddc9a81f24bef.crash
$ FUZZ=parse_iso8601 ./src/test/fuzz/fuzz parse_iso8601-46463936b8a32173e167a89aad1ddc9a81f24bef.crash
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3937133750
INFO: Loaded 1 modules (568922 inline 8-bit counters): 568922 [0x5624a4a983f0, 0x5624a4b2324a),
INFO: Loade
...

💬 sipa commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399160874)
To check `a/b > c/d` you can instead check `a*d > c*b`, which avoids divisions (which are an order of magnitude slower than multiplication).

💬 sipa commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399161977)
I believe that's an idea we've toyed with (calling it "sibling eviction"), but so far it isn't clear how to align that with DoS prevention.

💬 hebasto commented on pull request "[26.x] Final Changes":
(https://github.com/bitcoin/bitcoin/pull/28872#discussion_r1399162674)
I'm sorry for that late comment. Mind adding the following stuff:
```
- The transaction list in the GUI to no longer provide a special category for “payment to yourself”. Now transactions that have both inputs and outputs that affect the wallet are displayed on separate lines for spending and receiving. (gui#119)

- A new menu option allows migrating a legacy wallet based on keys and implied output script types stored in BerkeleyDB (BDB) to a modern wallet that uses descriptors stored in SQL
...

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399162733)
Fixed

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399163164)
I updated the commit message 👍🏾

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399163551)
Updated with your suggestion

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399165583)
This will change the input of the `CBlockPolicyEstimator` fuzzing test, I added a docstring `/*inBlock*/`.

⚠️ dergoegge opened an issue: "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed"
(https://github.com/bitcoin/bitcoin/issues/28918)
```
$ echo "PACVlVuVlZWVlZUE3pUAANNRAFEA09NRUb9RUVFR/wD/AP//AP8AWwD//wcErq6urv///////wD/4wAAAAD4a9cA////ra2tra2tra2tra2VlZWVMGA5OTk5OZWVlZWVlZWVlZWVlZWVlYUVJwyq6wEAlZWblZWVlZWVlZWVlZWVlZWblZWVlZWVlZWVlZWV//+V/5WV/////z4AAAAALAtfAgAAX/8=" | base64 --decode > coinselection-d97eed2ff63da56af72c8c858c560a7c6f2aef45.crash
$ FUZZ=coinselection ./src/test/fuzz/fuzz coinselection-d97eed2ff63da56af72c8c858c560a7c6f2aef45.crash
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 189972
...

💬 dergoegge 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-1819018704)
@murchandamus @furszy @brunoerg what's the status on this?

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399168585)
To override virtual functions from the `CValidationInterface` class, they must have same number of parameters and types to the corresponding virtual function in `CValidationInterface.

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1399181655)
> MakeAndPushMessage
> https://github.com/ajtowns/bitcoin/commit/0fe129b1b437d39221cef843f969f1adc31b543e

Thanks, done both. Looks like you forgot `std::forward`? Added it, and added you as co-author

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