💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576053878)
nit: includes not sorted

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576145432)
Good point, I don't think this is too complex so I've added it.

While working on this, I remembered/realized that if there are any other unconfirmed parents at all, we will reject it because the package must be child-with-unconfirmed-parents, enforced here:
https://github.com/bitcoin/bitcoin/blob/256e1703197fdddd78bc6d659431cd0fc3b63cde/src/validation.cpp#L1558-L1564

IIRC this was the way to check that a package was "2 generations only". But I don't know how useful this is, and it's quite
...

👋 Eunovo's pull request is ready for review: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576164670)
FWIW: I added logging here to see what number of parents and given by different peers than the orphan. After a day of running it's 0 out of 162.

Is there a reason to think this should be a common pattern?

💬 sdaftuar commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576171557)
If a parent+child are relayed back to back across the network, then I would expect there to be times when we download the transactions out of order (eg because we send a request for the parent to a different peer than the request for the child).

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576185766)
I guess it'll likely be something like:

1) peer A sends child, child is put in orphanage and parent tx request queued(but not yet sent)
2) peer B has slightly out of date feefilter for your node, sends INV for parent

💬 maflcko commented on issue "AssumeUTXO Mainnet Readiness Tracking":
(https://github.com/bitcoin/bitcoin/issues/29616#issuecomment-2072220456)
Why is #29720 / https://github.com/bitcoin/bitcoin/issues/29328 not mentioned here?

💬 maflcko commented on pull request "test: add missing tests for Assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2072222283)
Are you still working on this?

💬 maflcko commented on pull request "test: loading assumeutxo snapshot start states":
(https://github.com/bitcoin/bitcoin/pull/29681#issuecomment-2072224164)
Are you still working on this?

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576208038)
fwiw it's definitely not common, but I do see them occasionally and have a handful of acceptances of packages from 2 different peers

👍 BrandonOdiwuor approved a pull request: "test: Validate transaction without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2017176927)
ACK 1240610fcf0651f217fd01de387e1047dc18485f

Well done on including the unit test

📝 Umiiii opened a pull request: "test(mempool): add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29940)
Add missing comparison for TODO comments in `mempool_packages.py`

Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576214320)
> though I'm not convinced this really will make a difference

💬 theStack commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1576218587)
Thanks, done. I passed `NULL` rather than `nullptr`, cause that's what's used for the other null-parameter as well, but it shouldn't make a difference.

💬 sdaftuar commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576220246)
> I guess it'll likely be something like:
>
> 1. peer A sends child, child is put in orphanage and parent tx request queued(but not yet sent)
> 2. peer B has slightly out of date feefilter for your node, sends INV for parent

What I'd expect to happen is that we get inv's for parent+child from both Peer A and Peer B, and we happen to request the parent from A and the child from B, but the child arrives first -- it's put in the orphanage, and since a request for the parent is already in fli
...

👍 hebasto approved a pull request: "util: remove unused cpp-subprocess options"
(https://github.com/bitcoin/bitcoin/pull/29865#pullrequestreview-2017207809)
re-ACK 899be1a7643040c32510903dab06cea5ef7a5ffd.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1576227708)
nit: Maybe a wording more consistent with other comment around:
```suggestion
NULL, // use parent's environment
```

💬 theStack commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072269247)
Force-pushed with the last commit changed to tackle https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1571141464.

<details>
<summary>Diff to the previous version</summary>

```diff
diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp
index c7f9a685d1..a85c0b0bff 100644
--- a/src/util/subprocess.hpp
+++ b/src/util/subprocess.hpp
@@ -1210,8 +1210,6 @@ inline void Popen::k
...

👍 hebasto approved a pull request: "util: remove unused cpp-subprocess options"
(https://github.com/bitcoin/bitcoin/pull/29865#pullrequestreview-2017224404)
re-ACK 13adbf733f09c73c3cf0025d94c52f9cec5dba3b.

💬 theStack commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#discussion_r1576236387)
Thanks, done.