Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 theStack approved a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(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.
💬 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!
💬 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`.
🚀 glozow merged a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(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.
📝 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`?