💬 brunoerg commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585186363)
Not 100% related to these change, but`fill_mempool` requires -datacarriersize=100000 and -maxmempool=5 and in `rpc_packages` test, this is mentioned:
```py
# Make chain of two transactions where parent doesn't make minfee threshold
# but child is too high fee
# Lower mempool limit to make it easier to fill_mempool
self.restart_node(0, extra_args=[
"-datacarriersize=100000",
"-maxmempool=5",
"-persistmempool=0",
])
```

Perhaps, it would worth to also mention it in `mempoo
...

💬 sipa commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2085961219)
utACK 774359b4a96d2724dc70f900cb71e084a77164da

💬 sr-gi commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1585200234)
Covered in [ec29dc4](https://github.com/bitcoin/bitcoin/pull/29974/commits/ec29dc4266e1fe2eb967a7ce164a59e6c162c592)

💬 theStack commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2086022405)
Concept ACK

💬 dongcarl commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086032180)
If I remember correctly, I think that you can likely just patch OpenSSL here since it's not a package that is "core" to Guix like GnuTLS was?

👍 pinheadmz approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2032047471)
ACK fa2353d9c3ff68995e7478b9b80dcdb1fc638a53

Confirmed the warning on master by setting `-Wall` and then forcing `HAVE_SOCKADDR_UN = 0` in `configure.ac`. This patch fixes the warning in a clean way. I dunno if `(void)unix;` needs an explanatory comment or if thats a recognizable pattern for this kind of thing.

Thanks again @maflcko for cleaning up my mess 😬

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fa2353d9c3ff68995e747
...

👍 BrandonOdiwuor approved a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2032085316)
Code Review ACK 83def1c5a3741878aa63e6f28cc3def99f76c358

👍 hebasto approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2032098076)
re-ACK fa2353d9c3ff68995e7478b9b80dcdb1fc638a53.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2086222091)
rebased on master due to conflict from https://github.com/bitcoin/bitcoin/pull/29906

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086252348)
I wrote:

> we don't want to bump our Guix Time Machine commit just for that.

Actually our Time Machine commit much more recent than I thought. Will defer to @dongcarl.

💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086320606)
> > we don't want to bump our Guix Time Machine commit just for that.
>
> Actually our Time Machine commit is much more recent than I thought.

I presume openssl is used in the bootstrap chain, to bootstrap older software, so it probably can never be removed in a time machine bump?

💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086326853)
> If I remember correctly, I think that you can likely just patch OpenSSL here since it's not a package that is "core" to Guix like GnuTLS was?

Yes, this is workaround 3.

> "Workaround 3: Disable the tests in the Guix source code for this single derivation"

📝 pinheadmz opened a pull request: "test: use sleepy wait-for-log in reindex readonly"
(https://github.com/bitcoin/bitcoin/pull/30006)
Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152

💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1585286939)
> Why is this removed? The block is in the most-work chain. I think this comment meant a block that is _not_ in the most-work chain, no?

Maybe i missinterpreted the TODO comment meaning . To which TODO were you referring here
https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-1952480346 ?

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1585289467)
Sorry, I think this was my bad. It looks like there is no TODO for the test you are adding here.

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086353157)
> Disable the tests in the Guix source code for this single derivation

We already have that recommendation, but I think it's better to offer an alternative. "Don't worry about tests not passing" is something I prefer to only do when I really understand how everything works, which I don't.

💬 maflcko commented on pull request "test: use sleepy wait-for-log in reindex readonly":
(https://github.com/bitcoin/bitcoin/pull/30006#discussion_r1585292029)
```suggestion
with self.nodes[0].assert_debug_log(["Reindexing finished"], timeout=30):
```

I think the timeout needs to be preserved?

💬 maflcko commented on pull request "test: use sleepy wait-for-log in reindex readonly":
(https://github.com/bitcoin/bitcoin/pull/30006#issuecomment-2086358940)
Thanks. lgtm ACK 453c871184e0d62111028d25ec327cc7dd2f0099, after fixed-up timeout value.

💬 pinheadmz commented on pull request "test: use sleepy wait-for-log in reindex readonly":
(https://github.com/bitcoin/bitcoin/pull/30006#discussion_r1585294720)
Gotcha, the default timeout in `wait_for_debug_log()` now `busy_wait_for_debug_log()` is 60 unless im mistaken?

💬 maflcko commented on pull request "test: use sleepy wait-for-log in reindex readonly":
(https://github.com/bitcoin/bitcoin/pull/30006#discussion_r1585296203)
Sure, any value works. But 2 seconds may be a bit on the risky side.