💬 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)

👍 maflcko approved a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130#pullrequestreview-2394558598)
Almost lgtm. Didn't review the gui changes and the rebase was borked.

ACK 2a541e93cf13362e52c62e4d01d34a602843471e 📏

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU
...

💬 maflcko commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1816223837)
3c0fe09cc2fac18175aac35a54d64c9f6cd4482b: This merge conflict was solved incorrectly. Please be careful when rebasing. See also diff3, which should make rebasing easier, if used correctly: https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md#rebasingmerging-code

💬 laanwj commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#issuecomment-2437200247)
maybe... im not sure it's a super good first issue, implementing it properly isn't super complicated but does require some knowledge of how various parts work and interact

💬 garlonicon commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2437215836)
> where a network with realistic mining dynamics is important

What about using "getblocktemplate" on mainnet, and mining blocks, which will quickly become stale? If you want to have worthless coins, then they should be stale, in other cases, they will be traded.

Another possibility is to use "Pay to Proof of Work" addresses on signet/regtest. Then, moving a coin will require grinding a DER signature, to get it below N bytes, so it will require some mining power.

> just pointing out that
...