Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🚀 fanquake merged a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/33064)
👍 rkrux approved a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075#pullrequestreview-3061782410)
ACK fa21a90c3558c8414aafe0e5b68d8b9590cca127
💬 maflcko commented on pull request "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`":
(https://github.com/bitcoin/bitcoin/pull/33065#issuecomment-3126557564)
review ACK 251d02084688c67523e9ec92ec79ee657454ab93 🌨

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 251d02084688
...
💬 maflcko commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3126594440)
I think the question boils down to: How much UX do we want to provide for data corruption or internal bugs? A valid answer could be: None. In that case you can just use `Assert` to ensure no data corruption or internal bugs happen. (If they do, the program terminates, like with other Asserts that unexpectedly hit in production.) This approach will likely use the least amount of code.

Other valid answers are: Best-effort (use `throw`, and maybe catch it, or not). Full-effort (use `Result`).
👍 rkrux approved a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3061836848)
crACK 251d02084688c67523e9ec92ec79ee657454ab93

Nit: can update the grep command in the PR description.
💬 maflcko commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#issuecomment-3126623052)
lgtm, but I haven't reviewed the hd-split refactor. I'd say it would be good to add a test for this, using a previous release to create an ancient wallet, and then checking that the migration works as expected?

review ACK 60d1042b9a4db8daf9fffdc29053652e99b7126e 🐾

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5Ak
...
💬 fkorotkov commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126627675)
Hey folks,

Could someone with admin permissions please check that `Allow public repositories` checkbox is set on this page:

https://github.com/organizations/bitcoiin/settings/actions/runner-groups/1

Seems this is the reason runners are not picking jobs at the moment.
💬 fanquake commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126631857)
@fkorotkov. Should be fixed now:

<img width="554" height="111" alt="a" src="https://github.com/user-attachments/assets/9f517086-fd25-4261-b356-8fab387ebf12" />
💬 TheCharlatan commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3126634149)
Tested it out over here: https://github.com/TheCharlatan/rust-bitcoinkernel/tree/monolithic_lib
💬 b-l-u-e commented on pull request "[p2p] Fix signed integer overflow in LocalServiceInfo::nScore":
(https://github.com/bitcoin/bitcoin/pull/33072#issuecomment-3126635012)
@gmaxwell Thank you for the excellent feedback!
The saturation is a much better approach than changing to int64_t.
I've implemented the saturation fix as you suggested using if (nScore < std::numeric_limits<int>::max()) nScore++.
This prevents undefined behavior while maintaining memory efficiency and logical correctness after 2 billion connections, the matter is indeed settled.
I've also added a test case to verify the saturation behavior works correctly.
Thank you for pointing me in t
...
💬 maflcko commented on pull request "rpc: use CScheduler for relocking wallet and remove RPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3126641973)
> I'm unable to reproduce so far building master at [6cdc5a9](https://github.com/bitcoin/bitcoin/commit/6cdc5a90cffe9ced4ec50d2028c9896d25a9cb6a)

You'll have to use libc++, I guess?
💬 brunoerg commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#issuecomment-3126696521)
reACK 3d7a0219a9819e67690ed5acfc1331703dd0eb3a
💬 purpleKarrot commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3126699812)
I think we should reduce the amount of "clever hacks" in the CMake code, not increase them. If `.pc` files are not expressive enough for exporting all our target properies (and they will never be, because `.pc` files are not relocatable), we should stop providing them and export target information as CMake packages only. If CMake packages are not expressive enough either, because we don't want to export dependencies in case of a static library, we should simply not export a static library to a p
...
💬 Bost commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-3126767151)
@maflcko Thank. Also I found that reducing parallelism (e.g. to 4 cores) does the trick:
```
bost@ecke ~$ guix build --cores=4 bitcoin-core --no-grafts --check

successfully built /gnu/store/fc849g9lg1d970plrf0zgx0qlhykb2nx-bitcoin-core-28.2.drv
/gnu/store/x49mxs1m10b7hnav9sh85yqg42wc7az5-bitcoin-core-28.2
```
(The `--no-grafts` and `--check` flags ensure that Guix rebuilds the package locally rather than using substitutes.)
🤔 janb84 reviewed a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075#pullrequestreview-3062133613)
ACK fa21a90c3558c8414aafe0e5b68d8b9590cca127

PR adds a release note for several related legacy wallet removal PR's
💬 hebasto commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3126883402)
> Currently, when clients want to use bitcoin as a suproject, they get their build-type and compile flags silently changed, and they get a summary spewed into their configure log. This is what should be fixed instead.

These ideas are orthogonal to this PR, aren't they?
💬 willcl-ark commented on pull request "guix: warn SOURCE_DATE_EPOCH set in guix-codesign":
(https://github.com/bitcoin/bitcoin/pull/33073#issuecomment-3126963884)
My guix build with this change:

```
src/core/bitcoin on  codesign-source-epoch [$] via △ v3.31.6 via 🐍 v3.13.3 via ❄️ impure (nix-shell-env) took 1h33m13s
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
b8961023686b6aa3cef95f9f938c1c4b9c1e87945ea60c994dbe56c430faf43e guix-build-1bed0f734b3f/output/aarch64-linux-gnu/SHA256SUMS.part
90f9c040b2640cc7840cdb9341170c754b107ef7c681c762e8d69e3641378322 guix-build-1bed0f7
...
💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126974485)
FWIW the CI run taking place [here in this PR](https://github.com/bitcoin/bitcoin/actions/runs/16567612264?pr=32989) is using Cirrus Runners on the migrated jobs (thanks @fkorotkov !) and demonstrates an absolute worst-case runtimeof these jobs with:

- No docker build cache
- No ccache cache
- No depends-sources caches
- No depends-build caches
💬 theStack commented on pull request "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`":
(https://github.com/bitcoin/bitcoin/pull/33065#issuecomment-3127061118)
> Nit: can update the grep command in the PR description.

Done.
🚀 fanquake merged a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065)