Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 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.
📝 fanquake opened a pull request: "ci: limit max stack size to 512 KiB"
(https://github.com/bitcoin/bitcoin/pull/33079)
Picks up #31367.
Closes #29840.

Limit adjustment is moved until after compilation, otherwise compilation might not succeed.
I've used compilation flags to limit the stack size in the native macOS jobs, because trying to use `ulimit` resulted in:
```bash
+ '[' -n 1 ']'
+ ulimit -s 262144
/Users/runner/work/bitcoin/bitcoin/ci/test/03_test_script.sh: line 17: ulimit: stack size: cannot modify limit: Operation not permitted
```
💬 fanquake commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-3127294235)
PIcked up in #33079.
💬 pablomartin4btc commented on pull request "doc: Add legacy wallet removal release notes":
(https://github.com/bitcoin/bitcoin/pull/33075#discussion_r2236527647)
nit: `newkeypool` was already mentioned earlier...
```suggestion
`dumpwallet`, `importwallet` and `upgradewallet` are removed.
```
💬 purpleKarrot commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3127368986)
> These ideas are orthogonal to this PR, aren't they?

From the perspective of the implementation, yes. From the perspective of the client, no.

A client may want to build an application that links the kernel. There are two options: One is to import the kernel package, the other is to build the kernel as a subproject. If the client prefers to statically link the kernel, option two should be preferred. This PR optimizes for option one.
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2236689505)
Oh, that makes sense, I thought that you were talking about changing the if-statement on line 393.
💬 enirox001 commented on pull request "test: add option to skip large re-org test in feature_block":
(https://github.com/bitcoin/bitcoin/pull/33003#issuecomment-3127476258)
tACK 8810642 – Ran tests with/without --skipreorg; saw ~40 % speedup; no regressions.
💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2236729580)
User descriptor keys do not follow any derivation specification. They can import keys derived using any path.
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3127506107)
re: https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3114802810

> For reference, the lint task still fails. Not sure if this is intentional or if you missed it.

Thanks! The lint task was failing in https://cirrus-ci.com/task/5047676548415488 because the subtree was modified. The subtree was modified to work around a bug fixed in https://github.com/bitcoin-core/libmultiprocess/pull/181 which is part of https://github.com/bitcoin/bitcoin/pull/32345 but not currently part of master.
...
👋 fanquake's pull request is ready for review: "ci: limit max stack size to 512 KiB"
(https://github.com/bitcoin/bitcoin/pull/33079)
🤔 maflcko reviewed a pull request: "Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3062416347)
there is a bug in the lint config. Also, left some nits.

looked at 72d82211fce78a18bce27a90726d30c85518955c~6 📊

<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+krxU1A3Yu
...
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236234660)
nit in 1f5eb93de45f6a530ba54d799f637c0f1b45bc1b: Could add a comment why this is needed? I guess due to GHA not supporting ipv6 in $current_year?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236357065)
nit in e9cf1c90052620b23745ca0d3fdccda2cf5fa0fc: Same, about `REPO_USE_CIRRUS_RUNNERS`
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236522727)
nit in 35a9dd7352d84292fbc2dcf2e544b380be49b4c3: Same (`REPO_USE_CIRRUS_RUNNERS`)
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236350345)
nit in 1f5eb93de45f6a530ba54d799f637c0f1b45bc1b: Seems like there are several places that hard-code the main repo, which makes it harder for forks to use cirrus runners.

Maybe a global env next to `CIRRUS_CACHE_HOST` could be used to make that simpler?


Maybe `REPO_USE_CIRRUS_RUNNERS: 'bitcoin/bitcoin' # Use cirrus runners and cache for this repo, instead of falling back to the slow GHA runners`?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236548912)
nit in 35a9dd7352d84292fbc2dcf2e544b380be49b4c3: Unused? Maybe you wanted this to be the container name, or the job id instead?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236658377)
nit in 67429ff9ae0ad7236d01fee7d25dceb965741b2f: Please use `-o xtrace`, so that non-Bash experts can also understand the intention, without having to <strike>ask an LLM</strike> read the manpage
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236595677)
nit in c7aa954564ceaf0800be461115a568d3c919228a: This seems somewhat redundant with the container name now. It would be less yaml code to use container name instead of job id here?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236643414)
nit in a3789f2056a76e72fcafbae7396c35f62fb93f56: wrong (unused) suffix
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236619074)
nit in 88816a4ed3f5d528ed7b997241df96fb1f51df44: `apt-get install` will fail with a stale Ubuntu IP list from a cached image. You'll have to call `apt-get update` before every install for each image layer.