💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2816587753)
You could rebase your change before the fix reverted in https://github.com/bitcoin/bitcoin/pull/32302 to make it green

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051422887)
Or we could avoid repetition as well by a more functional approach of separating data from algorithm:
```python
patterns = {
"Traceback": "tracebacks - expecting exactly one with no knock-on exceptions",
expected_exception: f"occurrences of the specific exception: {expected_exception}",
"Test failed. Test logging available at": "test failure output messages"
}
errors = [f"Found {count}/1 {message}."
for pattern, message in patterns.items()
if (count := le
...

👍 l0rinc approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2779902319)
Concept ACK, I like the new `FeatureFrameworkStartupFailures`, it's untangled a lot better now in my opinion. I assume from the last commit message that it's not final, right?

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051423400)
nit: can probably be `if arg.startswith('--internal_test='):`
or walrus to the rescue again:
```python
if arg.startswith(prefix := '--internal_test='):
class_name = arg[len(prefix):]
```

💬 l0rinc commented on pull request "ci: Temporarily disable `WalletMigration` benchmark":
(https://github.com/bitcoin/bitcoin/pull/32306#discussion_r2051424583)
Yet another alternative would be to comment out `BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW);` instead (so that when we fix it we won't need to tough ci workflows again)

💬 hebasto commented on pull request "ci: Temporarily disable `WalletMigration` benchmark":
(https://github.com/bitcoin/bitcoin/pull/32306#discussion_r2051425168)
Will it have a broader impact beyond the CI?

📝 hebasto opened a pull request: "ci: Drop no longer necessary `-Wno-error=array-bounds`"
(https://github.com/bitcoin/bitcoin/pull/32308)
The build log of the "Linux->Windows cross" CI job no longer shows any `-Warray-bounds` compiler warnings. Therefore, there's no need to suppress them with `-Wno-error=array-bounds`.

I likely overlooked this when reviewing https://github.com/bitcoin/bitcoin/pull/29881, as I can run that CI job locally without such warnings even at commit 785649f3977517a4ba45c5d2fedfbda778fb52cb.

👍 rkrux approved a pull request: "docs: remove passing CI section in guidelines for PR merging as it's common sense"
(https://github.com/bitcoin/bitcoin/pull/32006#pullrequestreview-2779915055)
ACK 17a1630f226e1c8eb13eb7c7425290e933ad9368

📝 hodlinator converted_to_draft a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660)
Improves handling of startup errors in functional tests and puts tests in place to ensure knock-on errors don't creep in.
- `wait_for_rpc_connection()` now appends specific failures leading up to the `Unable to connect to bitcoind` error to that error message:
`[node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 1, 'OSError.ECONNREFUSED': 239}, latest error: ConnectionRefusedError(111, 'Connection refused'))`
- Fixes Windows Python issue where `socket.t
...

👍 TheCharlatan approved a pull request: "ci: Temporarily disable `WalletMigration` benchmark"
(https://github.com/bitcoin/bitcoin/pull/32306#pullrequestreview-2779922342)
ACK 18a035145d6abb41e49711de60c4feabe3ba31e9

Hope this gets fixed properly soon though.

👍 TheCharlatan approved a pull request: "ci: Drop no longer necessary `-Wno-error=array-bounds`"
(https://github.com/bitcoin/bitcoin/pull/32308#pullrequestreview-2779922820)
ACK e34f12bdd417862874fe75f5c4c2126a35724de6

🤔 TheCharlatan reviewed a pull request: "miner: timelock the coinbase to the mined block's height"
(https://github.com/bitcoin/bitcoin/pull/32155#pullrequestreview-2779926919)
Concept ACK

💬 hebasto commented on pull request "ci: Temporarily revert "Drop bench -priority-level in win cross CI"":
(https://github.com/bitcoin/bitcoin/pull/32302#issuecomment-2816640714)
May this be closed in favour of #32306?

🚀 hebasto merged a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/32213)

📝 l0rinc opened a pull request: "bench: close wallets after migration"
(https://github.com/bitcoin/bitcoin/pull/32309)
Fixes the failure after https://github.com/bitcoin/bitcoin/commit/27f11217ca63e0f8f78f14db139150052dcd9962 The performance of the benchmarks is the same as before the change

It's an alternative to https://github.com/bitcoin/bitcoin/pull/32302 and https://github.com/bitcoin/bitcoin/pull/32306

💬 l0rinc commented on pull request "ci: Temporarily revert "Drop bench -priority-level in win cross CI"":
(https://github.com/bitcoin/bitcoin/pull/32302#issuecomment-2816646348)
I've pushed https://github.com/bitcoin/bitcoin/pull/32309 which seems to fix the build (based on https://github.com/l0rinc/bitcoin/pull/9)

💬 l0rinc commented on pull request "ci: Temporarily disable `WalletMigration` benchmark":
(https://github.com/bitcoin/bitcoin/pull/32306#discussion_r2051446820)
I've pushed https://github.com/bitcoin/bitcoin/pull/32309 which seems to fix the build (based on https://github.com/l0rinc/bitcoin/pull/9)

💬 hebasto commented on pull request "bench: close wallets after migration":
(https://github.com/bitcoin/bitcoin/pull/32309#discussion_r2051447678)
Should the fact that the migrated wallet databases remain open be documented in `MigrateLegacyToDescriptor`?

💬 l0rinc commented on pull request "bench: close wallets after migration":
(https://github.com/bitcoin/bitcoin/pull/32309#discussion_r2051448501)
I needed to close both for the build to pass (`res->wallet->Close()` alone wasn't enough).
Maybe it passes with only `res->watchonly_wallet->Close()` - I can check if you think it makes sense.

💬 muaawiyahtucker commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible (after unclean shutdown)":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2816654839)
Running Raspberry pi 4, 8GB ram, I put the dbcache at 5500, thinking I was smart and would get a faster sync, but then it crashed at around 833432.
I got the error in the log:
` ERROR: Commit: Failed to commit latest txindex state`
I didn’t understand why it did that, so I tried to reduce the dbcache to 5000, but it seems that when I did that, it ‘unclearly restarted’, because when it rebooted, I got this rolling business. It is slow, but for me, every time it got to a certain point, near where
...