👍 theStack approved a pull request: "test: Check already deactivated network stays suspended after dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/30892#pullrequestreview-2302738124)
utACK 72c9a1fe94f927220d3159f516fda684ae9d4caa

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2348588797)
Some examples:

* https://cirrus-ci.com/task/5353088029032448 has missing logs (404). However, this one is not a timeout.
* https://cirrus-ci.com/task/5260390286753792 is a timeout, but has logs
* https://cirrus-ci.com/task/4532788332855296 is a timeout, and has missing logs

👍 hebasto approved a pull request: "Fix crash when closing wallet"
(https://github.com/bitcoin-core/gui/pull/835#pullrequestreview-2302743498)
ACK a965f2bc07a3588f8c2b8d6a542961562e3f5d0e.

Now, the `BitcoinGUI::removeWallet` signal is emitted once per wallet for both the "Closet Wallet..." and "Close All Wallets..." menu actions.

💬 Ianilfy commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1758606550)
What do I do

🤔 stratospher reviewed a pull request: "fuzz: reduce number of iterations in `crypto_aeadchacha20poly1305` target"
(https://github.com/bitcoin/bitcoin/pull/30826#pullrequestreview-2302751871)
ACK f482d0e. saw similar coverage stats
1. on branch
`#3002361 REDUCE cov: 568 ft: 3821 corp: 275/28Kb lim: 4096 exec/s: 1559 rss: 861Mb L: 154/867 MS: 4 InsertByte-EraseBytes-ShuffleBytes-ChangeBinInt-`

2. on master
`#3004483 REDUCE cov: 567 ft: 4067 corp: 270/47Kb lim: 4096 exec/s: 1917 rss: 842Mb L: 278/2782 MS: 1 EraseBytes-`

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758617119)
My thought here was that we set a higher factor when things are generally expected to run slower, so in that case it seems natural to me that we will also want to wait longer. But yeah, I didn't take into account that people will set this factor unreasonably high to not see timeouts at all. I have removed it for now but I think an alternative could be to set a reasonable upper bound like 10x.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758617767)
Done and renamed to `duration`.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2348604652)
The first two commits of https://github.com/Sjors/bitcoin/pull/62 could be included here to put it behind a `-DWITH_SV2=ON` feature flag.

I'll include that in a future push once it had some review. It shouldn't be a blocker for this PR, because the classes introduced here are not used yet.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758618169)
sounds good, done

💬 fanquake commented on pull request "build: add `standard branch-protection` to hardening flags for aarch64-linux":
(https://github.com/bitcoin/bitcoin/pull/30433#issuecomment-2348605539)
Sorry, fixed the buggy previous version.

✅ hebasto closed an issue: "Closing a wallet using the fa46088440 28.x QT client segfaults"
(https://github.com/bitcoin/bitcoin/issues/30887)

🚀 hebasto merged a pull request: "Fix crash when closing wallet"
(https://github.com/bitcoin-core/gui/pull/835)

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2348606489)
Addressed feedback from @maflcko and @jonatack and fixed the missing `lambda:` causing the CI failure.

💬 hebasto commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2348608460)
@achow101

Could you please add https://github.com/bitcoin-core/gui/pull/835 as well?

💬 maflcko commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758629596)
```suggestion
def ensure_helper_internal(predicate, *, duration):
```

forgot to rename this one?

💬 maflcko commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758629070)
```suggestion
def ensure(self, test_function, *, **kwargs):
```

I am not sure about a default value here. I feel like `time.sleep()`, or similar constructs in tests should be easily visible. Otherwise, this may be used as a drop-in replacement for `assert`, because someone may think this is just an assert that accepts a lambda.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758637789)
I didn't think it matters much because this is internal anyway, but I renamed it now as well.

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1758642035)
Ok, that might be more of a naming issue with `ensure` then which I am happy to reconsider if someone has a better idea but I removed the default value for now.

👍 tdb3 approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2302812511)
ACK b9aded42252969cc81acf4122ca5e2d1e00ecf90

💬 dergoegge commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1758657903)
> There is also an overhead that we probably don't want a separate fuzz target for every trivial function, because each fuzz target may be put into a separate binary and is allocated a separate folder.

I don't think the binary sizes would be a problem after we merge something like https://github.com/bitcoin/bitcoin/pull/30882 and the extra folder for the corpus is also not a huge cost (they compress really nicely).

I'd prefer to follow best practices over having to weigh triviality against
...