📝 theStack opened a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939)
Different MiniWallet instances using the same mode (either ADDRESS_OP_TRUE, RAW_OP_TRUE or RAW_P2PK) currently always create and spend UTXOs with identical output scripts, which can can cause unintentional tx dependencies (see e.g. the discussion in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465). In order to avoid mixing of UTXOs between instances, this PR introduces the possibility to provide a MiniWallet tag name, that is used to derive a different internal key for the t
...

👍 stickies-v approved a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2016873417)
ACK b7a3f8e375b87e4f3ccfa7994905dfbf46a23bf2

Good to remove this unnecessary libevent dependency, and nice work on restructuring the commits etc. Easy to review PR. Left some nits and happy to quickly review if you decide to address them.

---

Not essential for this PR, but I noticed we're doing two unnecessary string copies (through `std::string::substr()`) in code touched by this PR:

<details>
<summary>git diff on b7a3f8e375</summary>

```diff
diff --git a/src/common/url.cpp b/s
...

💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576038708)
nit: `std::errc` [is part of `system_error`](https://en.cppreference.com/w/cpp/error/errc)

💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576033526)
nit: this should move up a bit:

<details>
<summary>git diff on b7a3f8e375</summary>

```diff
diff --git a/src/Makefile.am b/src/Makefile.am
index 239113a056..7387eb1eaa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -679,6 +679,7 @@ libbitcoin_common_a_SOURCES = \
common/run_command.cpp \
common/settings.cpp \
common/system.cpp \
+ common/url.cpp \
compressor.cpp \
core_read.cpp \
core_write.cpp \
@@ -711,8 +712,6 @@ libbitcoin_common_a_SOURCES = \

...

💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576047415)
typo nit
```suggestion
BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) {
```

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