Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ 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\
...
πŸ’¬ TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2934750745)
Rebased 15ff9cb1789b7fe69c0ad24d737712aa6955e9cb -> df0eb65bd98f300618cccd667720f1ccc6145865 ([spendblock_4](https://github.com/TheCharlatan/bitcoin/tree/spendblock_4) -> [spendblock_5](https://github.com/TheCharlatan/bitcoin/tree/spendblock_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_4..spendblock_5))

* Fixed conflict with #32644
πŸ’¬ hebasto commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2934759550)
You could:
1. Check the free disk space on your machine.

2. Check whether the issue is resolved as described in https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows-msvc.md#4-building-with-static-linking-with-gui

As a last resort, please report the issue [upstream](https://github.com/microsoft/vcpkg).
πŸ“ Jokacar10 opened a pull request: "b9de2eb "
(https://github.com/bitcoin/bitcoin/pull/32671)
b9de2eb

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests
...
βœ… fanquake closed a pull request: "b9de2eb"
(https://github.com/bitcoin/bitcoin/pull/32671)
πŸ“ m3dwards opened a pull request: "ci: update pwsh to use custom shell that fails-fast"
(https://github.com/bitcoin/bitcoin/pull/32672)
Github by default sets fail fast behaviour on pswh shell which means that if any powershell cmdlet fails the script will stop and exit. The problem is that this behaviour doesn't apply when calling native executables, it only applies to powershell cmdlets.

I think the safest thing is to whenever we use pwsh to enable `$PSNativeCommandUseErrorActionPreference = $true` which will also fail calling any exe that returns a non-zero exit code.

Technically the step `Adjust paths in test/config.in
...