👍 hebasto approved a pull request: "ci: Fix lint runner selection (and docker cache)"
(https://github.com/bitcoin/bitcoin/pull/33744#pullrequestreview-3411360968)
ACK 0b3b8a3be1a0db0dfc634acca1d9305dc0fbfae6, I have reviewed the code and it looks OK.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486733227)
I check the avg, which is computationally easier to obtain. It would make sense to check the median as well, and maybe compare the median with the average. However, I prefer to avoid the O(N log N) complexity of the sorting.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486736452)
Yes, nontheless I reduce the count from 30000 to 10000, that's also enough for the test.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486781519)
Removed

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486783350)
I've adjusted the numbers used and added comments

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486784728)
Changed to `BOOST_CHECK_EQUAL`, added comments

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486787928)
Done

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3480999335)
Review comments addressed, mostly minor:

- Reduce the number of iterations to 10000 (from 30000), for faster test execution.
- Add comments on choice of test inputs.
- Use BOOST_REQUIRE_EQUAL or BOOST_CHECK_GT where possible.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3481005723)
@sipa, as this is triggered by #33515 of yours, I would kindly ask for a review.

💬 willcl-ark commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#issuecomment-3481075031)
Sorry for the slow turnaround.

I pushed an additional commit to address (half of) @maflcko's nits. He is correct that GHA apparently does not validate inputs (even if you set `type: choice`), so it makes sense to remove the `options`. In order that we not miss if the wrong option is passed here in the future I added a sanity check which will annotate the CI run with a `Warning` when an unknown input is detected. See the bottom of the annotations of this run for what that looks like: https://g
...

💬 stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3481078520)
Approach ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596, except for progress indication I don't see the need to unconditionally log every roll{back,forward} operation, so I strongly prefer logging it every n iterations over adding another non-ratelimited unconditional site.

💬 fanquake commented on pull request "prevector: simplify implementation of comparison operators and match behavior of `std::vector`":
(https://github.com/bitcoin/bitcoin/pull/33772#issuecomment-3481100244)
As indicated by the CI, the macOS SDK we are targeting doesn't (yet) support `lexicographical_compare_three_way`. A Guix build will fail the same way:
```bash
In file included from /distsrc-base/distsrc-21318e8b5df5-arm64-apple-darwin/src/chainparams.cpp:6:
In file included from /distsrc-base/distsrc-21318e8b5df5-arm64-apple-darwin/src/chainparams.h:9:
In file included from /distsrc-base/distsrc-21318e8b5df5-arm64-apple-darwin/src/kernel/chainparams.h:11:
In file included from /distsrc-base
...

💬 fanquake commented on pull request "depends: disable variables, rules and suffixes.":
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3481126835)
Pushed up the long form for both options.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3481130144)
@vasild @ryanofsky

Kindly asking you to refresh your ACKs after the comment [update](https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3457125965).

👍 hebasto approved a pull request: "depends: disable variables, rules and suffixes."
(https://github.com/bitcoin/bitcoin/pull/33045#pullrequestreview-3411598078)
re-ACK 52b1595850f63b65701a405d31045faa59231c75.

✅ hebasto closed a pull request: "Add a menu action to restore then migrate a legacy wallet"
(https://github.com/bitcoin-core/gui/pull/877)

📝 hebasto reopened a pull request: "Add a menu action to restore then migrate a legacy wallet"
(https://github.com/bitcoin-core/gui/pull/877)
Some users will have a backup of their legacy wallet. These cannot be restored since the "Restore Wallet" action expects to be able to load the wallet after restoring, and this fails for legacy wallets now that they are deleted. Furthermore, the "Migrate Wallet" action only allows users to migrate wallets that are in the wallets directory, so such backups cannot be migrated from the GUI.

This PR resolves this issue by adding a menu item in the "Migrate Wallet" menu which allows users to selec
...

🤔 pinheadmz reviewed a pull request: "zmq: Log bind error at Error level, abort startup on init error"
(https://github.com/bitcoin/bitcoin/pull/33727#pullrequestreview-3411664324)
concept ACK

💬 pinheadmz commented on pull request "zmq: Log bind error at Error level, abort startup on init error":
(https://github.com/bitcoin/bitcoin/pull/33727#discussion_r2486961157)
Would be nice to assert the `expected_msg` here. Also, to more explicitly cover the issue in #33715 I'd expect to see a test where zmq is set to a valid tcp address but one that is already in use (you should be able to attempt a bind to node's p2p or rpc port)

💬 pinheadmz commented on pull request "zmq: Log bind error at Error level, abort startup on init error":
(https://github.com/bitcoin/bitcoin/pull/33727#issuecomment-3481214931)
Would be nice to get review from @promag or @laanwj because of the discussion in https://github.com/bitcoin/bitcoin/issues/17185 and https://github.com/bitcoin/bitcoin/pull/17445