💬 tdb3 commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815848810)
Generating 160 blocks might be overkill. The following generates one and moves `node = self.nodes[0]` to just under the opening log (to match other functions).

```diff
diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py
index 0a88b4c9587..5276814a087 100755
--- a/test/functional/p2p_orphan_handling.py
+++ b/test/functional/p2p_orphan_handling.py
@@ -572,9 +572,9 @@ class OrphanHandlingTest(BitcoinTestFramework):
@cleanup
def test_max_o
...

🤔 glozow reviewed a pull request: "functional test: Additional package evaluation coverage"
(https://github.com/bitcoin/bitcoin/pull/31152#pullrequestreview-2393934991)
ACK b318959bf8c2b9b02f718649adec03f7e07ac79d, tested that adding a trim at the end of subpackage evaluation causes this to fail. Happy to reack if you take the nits.

💬 glozow commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1815846424)
forgot to delete?

💬 glozow commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1815859993)
nit: strictly speaking...
```suggestion
max_parent_feerate = max([entry["fees"]["modified"] / (Decimal(entry["vsize"]) / 1000) for entry in parent_entries])
```

💬 glozow commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1815846947)
nit: to be possibly be
```suggestion
# Last parent is higher feerate causing other parents to possibly
# be evicted if trimming was allowed, which would cause the package to end up failing
```

💬 glozow commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1815863021)
nit: relying on the fact that mempool min feerate isn't persisted seems a little bit brittle.

💬 glozow commented on pull request "functional test: Additional package evaluation coverage":
(https://github.com/bitcoin/bitcoin/pull/31152#discussion_r1815870276)
Comment could be clearer: This test is specifically trying to test that intra-package trimming isn't happening. We already know that a transaction with a high feerate descendant shouldn't be evicted; we specifically want to make sure that the eviction decision doesn't happen until the package is all the way in. So we check effective-includes to ensure that the transactions were evaluated separately.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1815873284)
Fixed, thanks :)

💬 glozow commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815876255)
nit: why is this a `wait_until` instead of an assert?

💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1815883458)
nice catch

💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815888974)
Thanks! Updated in [5c299ec](https://github.com/bitcoin/bitcoin/pull/31040/commits/5c299ecafe6f336cffa145d28036b04b87e26712)

💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815889024)
Thanks! Updated in [5c299ec](https://github.com/bitcoin/bitcoin/pull/31040/commits/5c299ecafe6f336cffa145d28036b04b87e26712)

💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815889280)
Thanks! Updated in [5c299ec](https://github.com/bitcoin/bitcoin/pull/31040/commits/5c299ecafe6f336cffa145d28036b04b87e26712)

💬 ariard commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2436659077)
> This looks like a separate issue. I was talking about orphan resolution removing the tx, you are talking about TxRequestTracker expiration removing it.

Yes, see my comments above. I got it there are current issues in the orphan resolution removing the transaction with a `ForgetTxHash` before we received it again, and wrongly see it as unsolicited.

> I wasn't changing a random line of code: the added code is guarded by a future P2P version upgrade and therefore unreachable and untestable
...

💬 ariard commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2436662747)
if one can go and publish on the mailing list in which core release this is expected to land and indicates the expected deployment timelines for the downstream softwares. learning a bit from the ecosystem controversies raised when `mempoolfullrbf` was actually introduced back in 24.0.1.

💬 pythcoiner commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2436768523)
concept ACK

💬 maflcko commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2437086690)
rfm?

🤔 hebasto reviewed a pull request: "doc: replace `-?` with `-h` and `-help`"
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2394439589)
Post-merge ACK 33a28e252a7349c0aa284005aee97873b965fcfe.

💬 i-am-yuvi commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2437131445)
Concept ACK!!

🚀 fanquake merged a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936)