💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2123366159)
> > Default behaviour in powershell is to continue on error
>
> Do we have that turned off for all of our CIs?

We did: https://github.com/bitcoin/bitcoin/blob/e872a566f251c73908de8b6d243c94a6679c2eac/.github/workflows/ci.yml#L24-L28

💬 fanquake commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123366909)
> Since the attack only matters when peers can influence us, could we simply disable rate-limiting for all logs during IBD

?

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2934543453)
It fails at this step, in the `generate_public_nonces` function:

```
print("\n🐮 Requesting pubnonce (Round 1)")
signer_1.generate_public_nonces(psbt)
```

Just in case, I restarted the signing session by having Bitcoin Core make a fresh `musig2_pubnonces` for its key.

Are your e2e tests running against the latest version of this branch?

🤔 rkrux reviewed a pull request: "rpc: Use type-safe HelpException"
(https://github.com/bitcoin/bitcoin/pull/32660#pullrequestreview-2891673768)
ACK fa0cf42380dec17e73d38aa69dd70662a0bc344a

<details>
<summary>
Diff for testing
</summary>

```diff
➜ bitcoin git:(2506-rpc-help) ✗ git diff --color | cat
diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 5597f8d237..18af6d110e 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -17,7 +17,7 @@
static RPCHelpMan verifymessage()
{
return RPCHelpMan{"verifymessage",
- "Verify a signed message.",
+ "\nVerify a signed mes
...

💬 rkrux commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2123375103)
Ah! Took me some time to understand how this fix worked. In case of erroneous help text, a `NonFatalCheckError` exception is thrown from here that was earlier caught by the catch-all handler in `server.cpp`. Now only `HelpException` is caught and the rest, including `NonFatalCheckError`, is bubbled up all the way to the RPC response. https://github.com/bitcoin/bitcoin/blob/e872a566f251c73908de8b6d243c94a6679c2eac/src/rpc/util.cpp#L795-L796

💬 rkrux commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2123391836)
Nit: Maybe returning the result by throwing an exception is obvious to others but it wasn't to me. Does it necessarily need to be called `HelpException`? Calling it `HelpResult` might make it more explicit or `HelpResultException` (not my preference).

<details>
<summary>
Diff
</summary>

```diff
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index d5c790e89b..bd99615bbf 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -96,7 +96,7 @@ std::string CRPCTable::help(const
...

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123404055)
Can you be more specific :)?

💬 fanquake commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2934599327)
Guix Build:
```bash
9607cec72e511df72d1d07ef850015172d6c2707604f46694c019cd37903c65b guix-build-74dfa4155037/output/aarch64-linux-gnu/SHA256SUMS.part
30916e57efaa2bfd4265d840cde7049c863c4d35cec9d5e70681e7bc428e2d14 guix-build-74dfa4155037/output/aarch64-linux-gnu/bitcoin-74dfa4155037-aarch64-linux-gnu-debug.tar.gz
c462783e946c5e3e61a6a4ab4be1b8ddb6db9be07194d3241b43a8cc4cbe81f7 guix-build-74dfa4155037/output/aarch64-linux-gnu/bitcoin-74dfa4155037-aarch64-linux-gnu.tar.gz
23b60343ce14152f
...

💬 0xB10C commented on pull request "test: add skip_if_running_under_valgrind()":
(https://github.com/bitcoin/bitcoin/pull/32492#issuecomment-2934603491)
LGTM post-merge ACK

💬 maflcko commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2123414365)
thx, I may change to `HelpResult`, if I have to re-touch for other reasons.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2123414640)
@hebasto but there are a few places where we use pwsh, which will still fail fast if using exclusively cmdlets but doesn't appear to fail when calling native executables.

Here is a commit to demonstrate it won't always failfast: https://github.com/bitcoin/bitcoin/commit/4bf10310fb4ec4d6c2f479f008c487cca1f90c27

and the job run that didn't fail: https://github.com/m3dwards/bitcoin/actions/runs/15415032095/job/43375709475

Suggest we set `PSNativeCommandUseErrorActionPreference` to `true` e
...

💬 fanquake commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123418578)
What do you mean by "peers not being able to influence us during IBD"?

💬 maflcko commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-2934644539)
> We could bump the version in `.python-version` and/or just mention a minimum Python version for this tool?

Wholesale bumping to 3.12 seems a bit early. Maybe just add an option to this tool `--force-non-determinstic-old-python` to enable 3.10 and 3.11 and otherwise use 3.12+ by default?

🤔 hebasto reviewed a pull request: "deps: Bump lief to 0.16.6"
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2891775764)
My Guix build:
```
aarch64
9607cec72e511df72d1d07ef850015172d6c2707604f46694c019cd37903c65b guix-build-74dfa4155037/output/aarch64-linux-gnu/SHA256SUMS.part
30916e57efaa2bfd4265d840cde7049c863c4d35cec9d5e70681e7bc428e2d14 guix-build-74dfa4155037/output/aarch64-linux-gnu/bitcoin-74dfa4155037-aarch64-linux-gnu-debug.tar.gz
c462783e946c5e3e61a6a4ab4be1b8ddb6db9be07194d3241b43a8cc4cbe81f7 guix-build-74dfa4155037/output/aarch64-linux-gnu/bitcoin-74dfa4155037-aarch64-linux-gnu.tar.gz
23b60343
...

💬 0xB10C commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2934661860)
LGTM post-merge ACK. Will run this with compact block logging enabled on a few of my nodes.

💬 naiyoma commented on issue "external signer: PSBT error code `EXTERNAL_SIGNER_NOT_FOUND` is never returned":
(https://github.com/bitcoin/bitcoin/issues/32426#issuecomment-2934669487)

> * simply get rid of this error code and its handling (if e.g. a more generic error thrown in this case already provides enough information to the user)
> * implement the returning of the error condition; the areas that need to be touched are likely [`ExternalSignerScriptPubKeyMan::GetExternalSigner`](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/wallet/external_signer_scriptpubkeyman.cpp#L48) and its call-site in `::FillPSBT` below

I'm in favour of de
...

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2123460031)
I meant that during IBD we’re mostly replaying validated history, and my understanding is that rate-limiting is meant to protect logs caused by peer-supplied invalid data, not the deterministic IBD progress logs (e.g. `-debug=bench` or `-debug=leveldb`, which are *very* verbose (way over the 1MiB/h), but isn't related to a user-sent serialization error or similar cheap attack). "Attacking" a node that is still in the original IBD seems less severe to me than skipped logs. I also understand if yo
...

💬 maflcko commented on pull request "refactor: Drop unused `#include <boost/operators.hpp>`":
(https://github.com/bitcoin/bitcoin/pull/32668#issuecomment-2934697644)
My understanding is that operators.hpp is an internal header (unless you use it to derive operators) and only indirectly included via other boost headers. So there is no universal mapping that could be used.

The possible workarounds could be:

* https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-no_include (verbose and ugly)
* Possibly a "forwarding-only" header: `src/util/boost_multi_index.h`, which is listed as exporting all needed mu
...

💬 fanquake commented on issue "external signer: PSBT error code `EXTERNAL_SIGNER_NOT_FOUND` is never returned":
(https://github.com/bitcoin/bitcoin/issues/32426#issuecomment-2934709732)
@hebasto @Sjors

💬 John-zhan commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2934726054)
Now the network problem over, and when continue execute `cmake -B build --preset vs2022-static` have this output:

> -- Building x64-windows-dbg
CMake Error at scripts/cmake/vcpkg_execute_build_process.cmake:134 (message):
Command failed: C:/Users/win11/AppData/Local/vcpkg/downloads/tools/jom/jom-1_1_4/jom.exe /J 17
Working Directory: F:/workspace/code/github/bitcoin-29.0/build/vcpkg_installed/vcpkg/blds/qt5-base/x64-windows-dbg
See logs for more information:
F:\workspace\code\
...