💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742421605)
Fixed in https://github.com/bitcoin/bitcoin/pull/30803.

🤔 hebasto reviewed a pull request: "doc: Clarify libbitcoin_consensus in design/libraries.md"
(https://github.com/bitcoin/bitcoin/pull/30802#pullrequestreview-2278050191)
Concept ACK.

💬 hebasto commented on pull request "doc: Clarify libbitcoin_consensus in design/libraries.md":
(https://github.com/bitcoin/bitcoin/pull/30802#discussion_r1742425451)
Why `libbitcoin_node` removed? The diagram below still shows this dependency.

👍 maflcko approved a pull request: "build: Minor build system fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30803#pullrequestreview-2278054511)
review ACK c393acadf213ddf58a69759eaf341595ccbf4889

💬 maflcko commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1742427765)
```suggestion
# However, as of 2024, mainstream distributions do not yet provide
# CMake config files for ZeroMQ packages.
# If they do in the future, find_package(ZeroMQ) may be used instead.
```

nit: Could remove the TODO above and clarify that it is waiting on the distros.

💬 maflcko commented on pull request "doc: Clarify libbitcoin_consensus in design/libraries.md":
(https://github.com/bitcoin/bitcoin/pull/30802#discussion_r1742432173)
Well, it is used by everything that uses common. Probably best to just refer to the diagram.

Let me know if I should remove the "used by" completely.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2327073294)
I pushed the rebase, so that stratum v2 miner will produce valid testnet4 blocks. See #29775. Tagged [sv2-tp-0.1.8 ](https://github.com/Sjors/bitcoin/releases/tag/sv2-tp-0.1.8) and will binaries shortly.

💬 hebasto commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1742434506)
Thanks! Reworked per your feedback.

🤔 glozow reviewed a pull request: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-2277909745)
had a look at some tests that are disabled or have TODOs

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742339252)
9047d620b81
Suggestion for adding the deltas to chunk feerates instead of deleting the fields:
```
diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py
index a8183dc8e14..6c3e02b8862 100755
--- a/test/functional/mining_prioritisetransaction.py
+++ b/test/functional/mining_prioritisetransaction.py
@@ -101,21 +101,19 @@ class PrioritiseTransactionTest(BitcoinTestFramework):
self.nodes[0].prioritisetransaction(txid=txid_c, f
...

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742343258)
9047d620b81 Just deleting the clusterid is sufficient
```suggestion
del last_entry["clusterid"]
```

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742376309)
d0576c134a5
I don't think it's possible to adapt this test so that the tx has 100 direct conflicts, given that it needs to be within 1000vB (the only transactions allowed to have sibling eviction are TRUC children). Having 100 inputs makes it at least 3000-something vB. This would be the case for any new rule that requires 100 inputs.

So this test could just be deleted until we allow sibling eviction for transactions that don't have this size restriction.

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742345016)
line should be deleted

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742343699)
9047d620b81 similarly
```suggestion
del new_entry["clusterid"]
```

💬 glozow commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1742419135)
d0576c134a5
I think we only need to adapt 1 of these 2 tests and it should be quite easy to do. Currently, `test_too_many_replacements` requires a high descendant limit because it's 1 parent with 100 children, but we can just add a `self.generate(node, 1)` in between and it can use node 1 (the default one). Then, we can just delete `test_too_many_replacements_with_default_mempool_params`

```
diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py
index f1de04008c4..578
...

💬 maflcko commented on pull request "doc: Clarify libbitcoin_consensus in design/libraries.md":
(https://github.com/bitcoin/bitcoin/pull/30802#discussion_r1742437523)
Actually, reverted. Can be changed in the future, if there is need to.

👍 hebasto approved a pull request: "doc: Clarify libbitcoin_consensus in design/libraries.md"
(https://github.com/bitcoin/bitcoin/pull/30802#pullrequestreview-2278101828)
ACK fa78ed83be1f6831416a6f6632e2161f12d359e4.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2327111683)
Reworked the PR to relax `IsStandardTx` to allow a single dust output, and hopefully simplified the remaining checks that enforces spend of dusty parent's dust. This should assure the same mempool invariants as before, but with more understandable and minimal changes to logic flow.

@petertodd

> It's more important that this feature can be used securely by L2 implementations, without having to worry about security problems from complex edge cases.

I can't speak for all wallets, but at l
...

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2327148258)
reACK 100e1b9ecb29c76187112fda9fed1c01da192c99

via `git range-diff master 12760a57b3a6cd1aeb3b7532311f648de2b45aa4 100e1b9ecb29c76187112fda9fed1c01da192c99`

just some comment updates

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2327172907)
1f720ffe25443b37624d4d52e040d29ecf93278a adds a temporary fix for a crash after finding a block. This was introduced by the interface changes. Will investigate later, as part of https://github.com/Sjors/bitcoin/pull/49.

New tag [sv2-tp-0.1.9](https://github.com/Sjors/bitcoin/releases/tag/sv2-tp-0.1.9)