💬 Umiiii commented on pull request "test(mempool): add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29940#issuecomment-2072299641)
Believe the CI failed for lint problems (trailing whitespace and blank line contains whitespace), I've tested the code locally and passed the regression test.

💬 hebasto commented on pull request "util: remove unused cpp-subprocess options":
(https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2072308000)
> This seems likely to have a high maintenance cost.

As discussed in https://github.com/bitcoin/bitcoin/pull/28981 and during the IRC [meeting](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-07#988888;), we are going to treat new code as our own:
> [14:16](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-07#988909) \<fanquake\> Obviously having a static header in our repo is an improvement, similar to nanobench/tinyformat

💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576261445)
> Is the suggestion to drop this and only try same-peer packages?

If this is hitting actual usage, no, because it's coded and I actually reviewed it, and tests seem to cover it. I'm unsure if in BIP331-like world it would be used since it would be receiver-driven, but that might be thinking too far ahead.

> (In this scenario I guess I'm assuming no feefilter, or that the feefilter value is slightly stale.)

💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-2072324885)
> ... with the expectation that this code is going to be maintained as our own. Next PRs should:
>
> * Remove linter exclusions and fix all issues.

See:
- https://github.com/bitcoin/bitcoin/pull/29849
- https://github.com/bitcoin/bitcoin/pull/29910

> * Delete all unused code.

The work has been started in https://github.com/bitcoin/bitcoin/pull/29865.

✅ Umiiii closed a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29940)

📝 Umiiii opened a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29941)
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 reviewed a pull request: "test: add missing comparison of node1's mempool in MempoolPackagesTest"
(https://github.com/bitcoin/bitcoin/pull/29941#pullrequestreview-2017352438)
Thanks for the pull, please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits)

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576316804)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576316974)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317113)
done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317267)
both done

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576317764)
done, I took this naming from libevent exactly fwiw but since we change the behavior and tests anyway, it's not that relevant anymore to keep it consistent

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2072397502)
Addressed latest feedback from @hebasto and @stickies-v , thank you!

💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576323273)
Indentation?

💬 Umiiii commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29941#issuecomment-2072406459)
> Thanks for the pull, please [squash your commits](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits)

squashed.

💬 laanwj commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1576327472)
CI linter complains, apparently the option needs to be added in another place:
```
AssertionError: Please add {'-swapbdbendian'} to the hidden args in DummyWalletInit::AddWalletOptions
```

💬 fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1576348255)
should be fixed now, weird, I must have some plugin that inserts these tabs :/

💬 glozow commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#issuecomment-2072442023)
Nice, concept ACK

💬 laanwj commented on issue "guix: SOURCE_DATE_EPOCH is already set in some environments":
(https://github.com/bitcoin/bitcoin/issues/29935#issuecomment-2072443037)
Sounds good to me. I think it's fine to rename it, I think there's really very few edge cases in which one would really want to pass in a custom epoch to the guix build?

🤔 BrandonOdiwuor reviewed a pull request: "test: create assert_not_equal util"
(https://github.com/bitcoin/bitcoin/pull/29500#pullrequestreview-2017441166)
Approach ACK

Good job approaching this using Scripted diffs