💬 waketraindev commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3552070213)
```
f37da8124a6392893fc7b18cfd72c616cac022eaed66725b1632b9e0011e1391 guix-build-8558902e576e/output/x86_64-w64-mingw32/SHA256SUMS.part
aa44003342fd3e4fb9d8c1693d12d31788626988cd9f47840e21afe9203deef0 guix-build-8558902e576e/output/x86_64-w64-mingw32/bitcoin-8558902e576e-win64-codesigning.tar.gz
ad14fac93313dcaaba144f944cd0058ef1581bc582645d20f5c414a0db116e14 guix-build-8558902e576e/output/x86_64-w64-mingw32/bitcoin-8558902e576e-win64-debug.zip
3ae1c3daa2a7932e83a84b2ea1b9a8ebcc912fbfe2dfa
...

💬 waketraindev commented on pull request "rpc: Add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#issuecomment-3552076557)
> the commit message is wrong?

Thanks. Must've replaced it by mistake when I rebased. Fixed now

💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2541520465)
No we cannot as:

```cpp
enum class ModeState {
M_VALID, //!< everything ok
M_INVALID, //!< network rule violation (DoS value may be set)
M_ERROR, //!< run-time error
}
```
`!state.IsValid()` returns true when the state is either `M_INVALID` or `M_ERROR`
`state.IsInvalid()` returns true only when the state is `M_INVALID`, misses the `M_ERROR` or run-time error case.

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541520923)
Have attempted to make different defafults clearer in 34c424ea500

💬 hodlinator commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#issuecomment-3552108811)
Thanks for the feedback! "test max 6 ancestor commits of H..." is still pretty informative IMO. When not bolded, on larger resolutions, one can still see the whole name. And when selecting a job one sees the full name (as in this "macOS native, no depends" example to the right).

<img width="1051" height="261" alt="image" src="https://github.com/user-attachments/assets/487adc80-48bd-4307-ad2f-3b83f5056e50" />

I think documenting at least some limitations of tests in this way makes knowledge
...

💬 purpleKarrot commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#issuecomment-3552114302)
Both @w0xlt and @yuvicc, please don't introduce more [boolean trap](https://ariya.io/2011/08/hall-of-api-shame-boolean-trap)s.

💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3552171802)
Redid the rebase locally, besides an import collision the following was added:
```patch
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
--- a/src/test/miniscript_tests.cpp (revision 31196b79cabb6e4519e8a27165e1be75c5044403)
+++ b/src/test/miniscript_tests.cpp (date 1763547689484)
@@ -727,7 +727,7 @@
g_testdata.reset();
}

-// Confirm that ~Node() and Node::Clone() are stack-safe.
+// Confirm that ~Node(), Node::Clone() and operator=(Node&&) are stack-s
...

💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2541499665)
nit, if you touch again, we could simply omit leaves with no subs

```suggestion
if (i.subs) queue.push_back(std::move(i.subs));
```

💬 hebasto commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541619806)
Other native package, not only `native_qt`, require `g++`.

👍 hebasto approved a pull request: "Document compiler configuration for native depends packages"
(https://github.com/bitcoin/bitcoin/pull/33902#pullrequestreview-3482217585)
ACK b1b559f83f629c7d0976c05d07cbc6ec04e84095.

💬 hebasto commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541624816)
```suggestion
- `build_CC`: C compiler for native tools (default: `gcc` on Linux, `clang` on macOS/FreeBSD/OpenBSD)
- `build_CXX`: C++ compiler for native tools (default: `g++` on Linux, `clang++` on macOS/FreeBSD/OpenBSD)
```

💬 hebasto commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541630314)
```suggestion
You might want to override native tool compilers when your default build compiler (set in `./depends/builders/*.mk`) is not available. For example, when using a Linux host without gcc/g++.
```

💬 hebasto commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541632430)
```suggestion
Example using Clang for native build tools on Linux:
```

🤔 l0rinc reviewed a pull request: "ci: Make the max number of commits tested explicit"
(https://github.com/bitcoin/bitcoin/pull/33909#pullrequestreview-3482229939)
I didn't realize this, so concept ACK.

I wouldn't include the value of a variable in the variable's name, though, and I think the job's name should likely also be updated - left a suggestion

💬 l0rinc commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2541635179)
6 is an implementation detail, the high-level point is to signal that not every commit is checked, but that the last few commits in the current PR are.
Also, the job's name should also likely be adjusted, maybe `ci-test-each-commit-exec.py` as well:

```suggestion
test-latest-commits:
name: 'test latest commits'
```

💬 fanquake commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3552230525)
Can we just update Qt?

⚠️ johnsonella2208-oss opened an issue: "Mighty Hacker Recovery Hope for Support Available for Your Ethereum/Crypto/Asset Recovery Case/ Private Wallet Key"
(https://github.com/bitcoin/bitcoin/issues/33910)
### Motivation

**Mighty Hacker Recovery Hope for Support Available for Your Ethereum/Crypto/Asset Recovery Case/ Private Wallet Key**

### Possible solution

**Mighty Hacker Recovery Hope for Support Available for Your Ethereum/Crypto/Asset Recovery Case/ Private Wallet Key**

### Useful Skills

* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...


### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, pl
...

💬 hebasto commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3552247066)
> Can we just update Qt?

Updating Qt would require raising the minimum GCC version for Windows cross-compilation to [13.1](https://doc.qt.io/qt-6.8/supported-platforms.html#windows). Are we comfortable with that?

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662001)
I agree, but going to leave that from this PR for now.

💬 willcl-ark commented on pull request "Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2541662319)
Taken in 75155c073d7