👍 willcl-ark approved a pull request: "doc: update Guix INSTALL.md"
(https://github.com/bitcoin/bitcoin/pull/33574#pullrequestreview-3415100841)
ACK b4d0288c467f82a94041b51d10d38e66bb5c33ae

Changes here are fine. "various versions of Debian" is arguably a bit generous if it's planned to be dropped from (all?) upcoming versions, but it's not incorrect and the repology link provides up-to-date info in any case.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3484908644)
Changes applies:
- Fix lint issue
- Added check on the median as well
- some minor formatting

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489540239)
Indeed. Inline is only a hint, not crucial for modern compliers.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489550775)
I did that for the longer values, for shorter values with only a few ones is not really need.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489552705)
Added median check as well. Much lower than the avg!

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

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489555855)
Oops, fixed!

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

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489560183)
All cycles go from 1 to n-1; 0 is the genesis.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489569251)
A fuzz test for this functionality (essentially a one-liner) seems a bit too much.

🚀 fanquake merged a pull request: "test: cover invalid codesep positions for signature in taproot"
(https://github.com/bitcoin/bitcoin/pull/32301)

⚠️ Sjors opened an issue: "ci: spurious linter failure?"
(https://github.com/bitcoin/bitcoin/issues/33773)
Run is marked as cancelled but looks fine:

- https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876

💬 alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3485080149)
re-ACK https://github.com/bitcoin/bitcoin/commit/6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969

🚀 fanquake merged a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445)

💬 maflcko commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485151628)
This looks like a nice and small CI-only/test-only change to flatten the way for https://github.com/bitcoin/bitcoin/issues/30210. There is already a pull open for the guix changes in https://github.com/bitcoin/bitcoin/pull/33593, which can then pick up any follow-ups from here and also conclude by removing the msvcrt CI config and msvcrt workarounds in the code.

review ACK 3b135a8fc4451c93b3ea50b3f4621e0d19f35daf 🐂

<details><summary>Show signature</summary>

Signature:

```
untrusted
...

💬 fanquake commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2489816883)
Is there a way to do this without copy-pasting/duplicating > 100 lines of CI config (where both copies need to be kept in sync).

🤔 vasild reviewed a pull request: "[wip] wallet: Add separate balance info for non-mempool wallet txs"
(https://github.com/bitcoin/bitcoin/pull/33671#pullrequestreview-3415535438)
Concept ACK

This extends the `getbalances` RPC. Would be good to also update the GUI (not necessary in this PR).

The new type of transactions would have somehow to be fed to the wallet so that they would be returned by `wallet.GetTXOs()`, called by `GetBalance()`. How would that be done?

💬 maflcko commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2489849930)
I guess yaml anchors could be used, but this would increase the diff and review effort again when it is deleted. An alternative would be to use a matrix, but I guess this requires depending both cross-test tasks on both cross-compile tasks? I'd say the current version is fine, but no strong opinion.

💬 fanquake commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485238772)
> This one simply addresses your comments https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3460836590 and https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978.

Somewhat, but if this is the approach, it means the not only the compiler, but also the version of the headers being tested isn't going to match Guix?

💬 maflcko commented on issue "ci: spurious linter failure?":
(https://github.com/bitcoin/bitcoin/issues/33773#issuecomment-3485279327)
This simply times out, because the Ubuntu PPAs are so slow/rate-limited/ddos'd:

The task starts at 8:50, but download only concludes after 9:06:

* https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876#step:4:1110

`#11 951.6 Fetched 173 MB in 6min 31s (443 kB/s)`

* https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876#step:4:298:

`#11 549.5 Fetched 181 MB in 7min 56s (380 kB/s)`


See https://github.com/bitcoin/bitcoin/pull/33744
...