💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424248273)
typo nit
```suggestion
The choice to use an RPC framework at all instead of a custom protocol was necessitated by the size of Bitcoin Core internal interfaces which consist of around 150 methods that pass complex data structures and are called in complicated ways (in parallel, from callbacks that can be nested and stored). Writing a custom protocol to wrap these complicated interfaces would be too much work, akin to writing a new RPC framework.
```

💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424249827)
nit: rogue backtick
```suggestion
- The call to `getBlockHash` is translated into a Cap’n Proto RPC call. This translation is handled by code automatically generated by the `mpgen` tool, based on the [`chain.capnp`](../../src/ipc/capnp/chain.capnp) file in [`src/ipc/capnp/`](../../src/ipc/capnp/).
```

💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424253642)
Steps 2 and 3 highlight what's handled by generated code, for consistency that might be helpful in steps 4&5 too?

💬 mohamedawnallah commented on issue "Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/issues/27494#issuecomment-1852524854)
Hey, @theartpiece, Are you still working on this issue? If not, I’d like to pick it up. Thanks

📝 dergoegge opened a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)
The `minisketch` harness has low stability due to:
* Rng internal to minisketch
* Benchmarkning for the best minisketch impl

Fix this by seeding the rng and fixing the impl to 0.

Also see #29018.

💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852552451)
> When the mempool is clogged with this kind of TXs, users trying to use Bitcoin as money are priced out in droves, because they can't pay 98% of their UTXOs in fees, so they can only compete by moving orders of magnitude more value than inscriptions. Therefore when inscriptions are setting the floor at $10, they're not just pricing out people trying to transact $10 onchain, but also $20, $50, $100.. maybe up to $500.

I think the linked X thread's analysis and your analysis are both wrong. What
...

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1852558078)
Ran clang-tidy locally to (hopefully) fix its remaining complaints. Included @Fi3's https://github.com/Sjors/bitcoin/pull/25 (plus additional header and test change) which drops the `CIPHER_CONFIRMATION` state, as per https://github.com/stratum-mining/sv2-spec/issues/60.

👍 maflcko approved a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064#pullrequestreview-1778170510)
lgtm

💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406089)
Why hardcode this to `0`? Wouldn't it be better to let the fuzz engine pick one of `MaxImplementation()`?

💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406965)
Is this needed? Both should have been seeded already, no?

💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406512)
When touching this, maybe move the Assert into the helper?

💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852569550)
> > When the mempool is clogged with this kind of TXs, users trying to use Bitcoin as money are priced out in droves, because they can't pay 98% of their UTXOs in fees, so they can only compete by moving orders of magnitude more value than inscriptions. Therefore when inscriptions are setting the floor at $10, they're not just pricing out people trying to transact $10 onchain, but also $20, $50, $100.. maybe up to $500.
>
> I think the linked X thread's analysis and your analysis are both wro
...

🤔 mzumsande reviewed a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765#pullrequestreview-1778050698)
Code Review ACK f56ec8a3b086b0f2f8a0fbde861447e40ffbc3d9

One thing I'm unsure about is how the way we call `ShouldFanoutTo()` multiple times for each transaction would might affect performance.
If we have 120 reconciling peers, and get a dump of `MAX_SET_SIZE=3000` transactions, I think we'd call this function `360000` times within a short timeframe. I wonder how long that would take, maybe we could add a benchmark for this? (could be done a follow-up)

💬 mzumsande commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1424331514)
I think that `deterministic_randomizer` could be passed by reference

📝 furszy opened a pull request: "bench: wallet, fix change position out of range error"
(https://github.com/bitcoin/bitcoin/pull/29065)
Fixes #29061. Only the benchmark is affected.

Since #25273, the behavior of 'inserting change at a random position'
is instructed by passing ´std::nullopt´ instead of -1.

Also, added missing documentation about the meaning of
'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'

📝 hebasto opened a pull request: "doc: Bump minimum required Boost version due to migration to C++20"
(https://github.com/bitcoin/bitcoin/pull/29066)
Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits:
- https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec
- https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1

I tested [`libboost1.71-dev`](https://packages.ubuntu.com/focal/libboost1.71-dev) in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system.

Closes https://github.com/bitcoin/bitcoin/issues/29063.

💬 hebasto commented on issue "build: GCC 10.5 fails to compile `test_bitcoin` using Boost.Test 1.71":
(https://github.com/bitcoin/bitcoin/issues/29063#issuecomment-1852587644)
> Going to do more thorough tests before submitting a PR.

Found one more fixed bug. See #29066.

💬 furszy commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852591428)
Created #29065 solving this.
Only the benchmark has been affected. The GUI side finding was just a false-positive; ugly code that we should clean up sooner rather than later.

💬 shovas commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852591613)
Just wanted to chime in here and say that this is a critical blocker for anyone with a real wallet where this happens

I'm in an bad loop of loading wallet, 'beyond pruned data...download entire blockchain', download entire blockchain, abort on bad block, ...?? Has happened on two computers where Bitcoin Core has been working great for over a year.

💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852600138)
> Just wanted to chime in here and say that this is a critical blocker for anyone with a real wallet where this happens
>
> I'm in an bad loop of loading wallet, 'beyond pruned data...download entire blockchain', download entire blockchain, abort on bad block, ...?? Has happened on two computers where Bitcoin Core has been working great for over a year.

In order to get this functionality merged, we need to get #27837 merged. Which requires #28120 and #28170. Which are under review currentl
...