🤔 instagibbs reviewed a pull request: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-2283792936)
poked at the function tests a bit

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745982880)
nothing fails when I remove this

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1746041322)
Here's a diff to clean up some of the test language and double-checking the right number of clusters
```
diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py
index f75b13a571..201f303525 100755
--- a/test/functional/mempool_package_rbf.py
+++ b/test/functional/mempool_package_rbf.py
@@ -176,95 +176,103 @@ class PackageRBFTest(BitcoinTestFramework):
assert_equal(f"package RBF failed: insufficient anti-DoS fees, rejecting replacement {failure
...

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1745997228)
I think `test_rbf_carveout_disallowed` is meaningless post-cluster mempool, we should be relying on other tests to ensure we never go above cluster count limit

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1746092665)
this test is pretty ancient and tbh not sure worth holding around. But in case we want to hold onto it, modifications to make things more sensible in cluster mempool world

```
diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py
index f7fe381f43..9cf1aa01f5 100755
--- a/test/functional/mempool_packages.py
+++ b/test/functional/mempool_packages.py
@@ -1,92 +1,91 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2022 The Bitcoin Core developers
# Distri
...

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1746201647)
heh, if you try 99 cluster conflicts + sibling eviction, that implies the new transaction has a vsize cap of 1kvB since it's a child, and since it will be north of 5kvB it will be rejected. I don't think we can have sibling eviction count meaningfully towards exceeding max cluster conflicts. Worth the coverage anyways?

```
diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py
index e3fcc45059..4e8e5943fc 100755
--- a/test/functional/mempool_truc.py
+++ b/test/func
...

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1746202560)
the rest of these args can be deleted and the test passes

💬 instagibbs commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1746098370)
can obviously ignore this but instead of deleting keys in two spots, could just do:
`assert_equal({**last_entry, "clusterid": None}, {**new_entry, "clusterid": None})`

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2332719825)
@glozow copied some of the text to the OP

💬 achow101 commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2332722279)
ACK bb08c227de158dbd88a80d904edb209e1734ab91

💬 achow101 commented on pull request "[28.x] rc backports":
(https://github.com/bitcoin/bitcoin/pull/30762#issuecomment-2332729594)
ACK b2a137929a20baed161988e24de592b1f59c0096

🚀 achow101 merged a pull request: "[28.x] rc backports"
(https://github.com/bitcoin/bitcoin/pull/30762)

📝 achow101 opened a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
* #30821

📝 achow101 converted_to_draft a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
* #30821

💬 ariard commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2332788365)
@mzumsande

> I don't get this at all.
> The suggestion above was, if you receive an unrequested TX, and you already have it, ignore it but don't disconnect the
> peer. Which is the same as current behavior on master.

Let’s say you have Peer1 and Peer2 announcing the Transaction ABC, if ABC is concurrently fetch from both Peer1 and Peer2, the `GETDATA` (32-bytes) for ABC would have been wasted as outgoing bandwidth by Local Node. This 32 byte leak can be amplified if batch of transactio
...

📝 ismaelsadeeq opened a pull request: "interfaces: #30697 follow ups"
(https://github.com/bitcoin/bitcoin/pull/30828)

This PR addresses the remaining review comments from #30697

1. Disallowed overwriting settings values with a `null` value.
2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746265728)
Taken your suggestion @ryanofsky this is fixed in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746267705)
I've tested this locally by passing an unknown settings name this does not throw.

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746269227)
Taken in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746269588)
Removed in #30828