💬 Davidson-Souza commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2438501129)
In 7864871. If I fire a `getblocktemplate` **before** sync is done, I get a `segfault`.

<details>

<summary>Valgrind stack trace</summary>

```
==1025895== Thread 7 b-httpworker.3:
==1025895== Invalid read of size 4
==1025895== at 0x36A95F: node::(anonymous namespace)::MinerImpl::rollbackTestnet4() (interfaces.cpp:997)
==1025895== by 0x42F759: getblocktemplate()::{lambda(RPCHelpMan const&, JSONRPCRequest const&)#1}::operator()(RPCHelpMan const&, JSONRPCRequest const&) const [clo
...

💬 mzumsande commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2438504815)
I opened #31156 which just adds an arg. This seemed the least invasive solution to me.

>A pre-mined testnet4 chain can possibly be used for the test as well, instead.

That should be possible too, but would require enabling `setmocktime` for testnet, which I didn't love.

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817154923)
aside from reorgs?

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817158265)
when would removals or additions happen outside of a changeset?

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817154002)

```suggestion
* set of new transactions and compare with the existing mempool.
```

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817158983)
?
```suggestion
* Apply() will apply the removals and additions that are staged into the mempool.
```

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817161237)
logically seems to work, probably not worth nitting

💬 jonatack commented on issue "MIN_STANDARD_TX_NONWITNESS_SIZE prevents efficient spending of P2A outputs":
(https://github.com/bitcoin/bitcoin/issues/31155#issuecomment-2438531310)
See https://github.com/bitcoin/bitcoin/pull/26265 (merged, above 64 bytes allowed) and alternative https://github.com/bitcoin/bitcoin/pull/26398 (only 64 bytes disallowed).

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817171820)
```suggestion
CTxMemPool::setEntries all_conflicts;
```

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817170430)
we're still applying on failure, so it's not really a belt and suspenders for that anymore is it?

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817172422)
```suggestion
CTxMemPool::setEntries all_conflicts;
```

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1817177783)
ah right

📝 l0rinc converted_to_draft a pull request: "optimization: pack util::Xor into 64/32 bit chunks instead of doing it byte-by-byte"
(https://github.com/bitcoin/bitcoin/pull/31144)
The recently merged [XOR obfuscation](https://github.com/bitcoin/bitcoin/pull/28207) is a non-trivial part of serialization, encountered during IBD and `reindex`.

In this PR, I've added a new test (comparing randomized inputs against a trivial implementation), updated the benchmark to validate a [more realistic scenario](https://github.com/bitcoin/bitcoin/blob/master/src/node/mempool_persist.cpp#L180) and optimized the XOR function to do the work in batches (instead of per bytes).

```bash
...

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1817230404)
Done.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1817231681)
It was both. Removed outdated comments and dead code in the latest push. I cleaned up this function in the follow-up.

📝 darosior opened a pull request: "Cleanups to port mapping module post UPnP dropt"
(https://github.com/bitcoin/bitcoin/pull/31157)
Based on #31130, this does a couple cleanups to `src/mapport.*` to clarify the logic now that there is a single protocol option for port mapping.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2438634685)
Pushed two commits to remove outdated comments and dead code. I opened https://github.com/bitcoin/bitcoin/pull/31157 as a follow-up to clean up some of the logic which became unnecessary or confusing now that there is only a single protocol option.

🤔 BrandonOdiwuor reviewed a pull request: "init: Correct coins db cache size setting"
(https://github.com/bitcoin/bitcoin/pull/31064#pullrequestreview-2396283480)
Code Review ACK 3a4a788ee0db83d20607f14801dbed2ee932943c

📝 hebasto opened a pull request: "build, ci: Fix linking `bitcoin-chainstate.exe` to `bticoinkernel.dll` on Windows"
(https://github.com/bitcoin/bitcoin/pull/31158)
On the master branch @ 9a7206a34e3179d7280de98a818a13374178ee7b, linking `bitcoin-chainstate.exe` to `bticoinkernel.dll` fails:
```
> cmake -B build --preset vs2022-static -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_KERNEL_LIB=ON -DBUILD_SHARED_LIBS=ON
> cmake --build build --config Release -j 8 -t bitcoin-chainstate
...
LINK : fatal error LNK1181: cannot open input file 'kernel\Release\bitcoinkernel.lib' [C:\Users\hebasto\source\repos\bitcoin\build-static\src\bitcoin-chainstate.vcxproj]
```

Thi
...

💬 TheCharlatan commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#discussion_r1817300867)
Do you want to take this opportunity to also add a better description to the test name? Maybe something like:
`Win64 native, VS 2022, no depends, libbitcoinkernel`?