💬 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.
(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" />
(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
(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
...
(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?
(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
(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
...
(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.)
(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
(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?
(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
...
(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
(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.
(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)
(https://github.com/bitcoin/bitcoin/pull/33065)
👍 theStack approved a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-3062535291)
ACK 1c10b7351e194fc788766347f65f4512f61f05e8
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-3062535291)
ACK 1c10b7351e194fc788766347f65f4512f61f05e8
💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236368139)
I'm seeing a couple of potential reasons. First, when we receive an announcement from a non-wtxid-relay peer, we put that inv's hash (a txid) into the known inventory. And then second, when we receive an orphan we add the parent txids that we don't already have into that peer's known inventory.
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236368139)
I'm seeing a couple of potential reasons. First, when we receive an announcement from a non-wtxid-relay peer, we put that inv's hash (a txid) into the known inventory. And then second, when we receive an orphan we add the parent txids that we don't already have into that peer's known inventory.
💬 glozow commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236464694)
You're right. Might think about this some more but would be out of scope here anyway. Resolving!
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236464694)
You're right. Might think about this some more but would be out of scope here anyway. Resolving!
💬 glozow commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236460557)
Might be worth a comment because non-obvious: `m_tx_inventory_known_filter` hashes are based on the wtxidrelay-ness of the peer, so it's important that we compare the hash from `inv` and not the wtxid that was in `m_tx_inventory_to_send`.
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2236460557)
Might be worth a comment because non-obvious: `m_tx_inventory_known_filter` hashes are based on the wtxidrelay-ness of the peer, so it's important that we compare the hash from `inv` and not the wtxid that was in `m_tx_inventory_to_send`.
🚀 glozow merged a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954)
(https://github.com/bitcoin/bitcoin/pull/29954)
📝 stickies-v opened a pull request: "kernel: improve BlockChecked ownership semantics"
(https://github.com/bitcoin/bitcoin/pull/33078)
Subscribers to the BlockChecked validation interface event may need
access to the block outside of the callback scope. Currently, this
is only possible by copying the block, which makes exposing this
validation interface event publicly either cumbersome or with significant
copy overhead.
By using shared_ptr, we make the shared ownership explicit and allow
users to safely use the block outside of the callback scope.
(https://github.com/bitcoin/bitcoin/pull/33078)
Subscribers to the BlockChecked validation interface event may need
access to the block outside of the callback scope. Currently, this
is only possible by copying the block, which makes exposing this
validation interface event publicly either cumbersome or with significant
copy overhead.
By using shared_ptr, we make the shared ownership explicit and allow
users to safely use the block outside of the callback scope.