💬 willcl-ark commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126343860)
Some noteworthy observations from the last two weeks of runs:
1. We have seen variance in network speeds occasionally which has been raised with Cirrus. e.g. in [this run](https://github.com/testing-cirrus-runners/bitcoin/actions/runs/16564301804/job/46840539119#step:4:48) we can see a cache restore hanging for 5 minutes:
<img width="2078" height="498" alt="image" src="https://github.com/user-attachments/assets/f87526f7-386b-40c8-9b3a-f9e7ca284c8f" />
It seems to manifest intermitte
...
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3126343860)
Some noteworthy observations from the last two weeks of runs:
1. We have seen variance in network speeds occasionally which has been raised with Cirrus. e.g. in [this run](https://github.com/testing-cirrus-runners/bitcoin/actions/runs/16564301804/job/46840539119#step:4:48) we can see a cache restore hanging for 5 minutes:
<img width="2078" height="498" alt="image" src="https://github.com/user-attachments/assets/f87526f7-386b-40c8-9b3a-f9e7ca284c8f" />
It seems to manifest intermitte
...
📝 willcl-ark opened a pull request: "guix: warn SOURCE_DATE_EPOCH set in guix-codesign"
(https://github.com/bitcoin/bitcoin/pull/33073)
#32678 added a sanity check for this environment variable when running `guix-build` but missed that `guix-codesign` also relies on `SOURCE_DATE_EPOCH`, which can result in non-determinism in the codesigning step: https://github.com/bitcoin-core/guix.sigs/pull/1720#issuecomment-3124332676
To avoid repeating the logic move common functionality into the prelude and call the function in both guix actions.
(https://github.com/bitcoin/bitcoin/pull/33073)
#32678 added a sanity check for this environment variable when running `guix-build` but missed that `guix-codesign` also relies on `SOURCE_DATE_EPOCH`, which can result in non-determinism in the codesigning step: https://github.com/bitcoin-core/guix.sigs/pull/1720#issuecomment-3124332676
To avoid repeating the logic move common functionality into the prelude and call the function in both guix actions.
💬 rkrux commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2235557541)
Done
(https://github.com/bitcoin/bitcoin/pull/32866#discussion_r2235557541)
Done
💬 rkrux commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#issuecomment-3126359553)
> ACK [9d25880](https://github.com/bitcoin/bitcoin/commit/9d25880bb720bc675a533098268b9e02f86e17ce)
>
> Happy to re-ack if you touch it again to address the LLM linter suggestion.
Done
(https://github.com/bitcoin/bitcoin/pull/32866#issuecomment-3126359553)
> ACK [9d25880](https://github.com/bitcoin/bitcoin/commit/9d25880bb720bc675a533098268b9e02f86e17ce)
>
> Happy to re-ack if you touch it again to address the LLM linter suggestion.
Done
📝 fanquake opened a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33074)
Backports:
* #33070
(https://github.com/bitcoin/bitcoin/pull/33074)
Backports:
* #33070
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2235578787)
Yeah agreed, to be left as follow-up if others agree.
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2235578787)
Yeah agreed, to be left as follow-up if others agree.
📝 maflcko opened a pull request: "doc: Add legacy wallet removal release notes"
(https://github.com/bitcoin/bitcoin/pull/33075)
This spans over several pulls, so add a single note for all of them.
(https://github.com/bitcoin/bitcoin/pull/33075)
This spans over several pulls, so add a single note for all of them.
💬 fanquake commented on pull request "doc/zmq: fix unix socket path example":
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126383385)
Backported to 29.x in #33074.
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126383385)
Backported to 29.x in #33074.
📝 fanquake opened a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33076)
Backports:
* #33001
* #33070
(https://github.com/bitcoin/bitcoin/pull/33076)
Backports:
* #33001
* #33070
💬 fanquake commented on pull request "doc/zmq: fix unix socket path example":
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126392391)
Backported to `28.x` in #33076.
(https://github.com/bitcoin/bitcoin/pull/33070#issuecomment-3126392391)
Backported to `28.x` in #33076.
💬 fanquake commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#issuecomment-3126393675)
Backported to `28.x` in #33076
(https://github.com/bitcoin/bitcoin/pull/33001#issuecomment-3126393675)
Backported to `28.x` in #33076
💬 fanquake commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/33064#issuecomment-3126398686)
> Question: would it make sense to also test "abortrescan" during an active rescan to hit the True path and ensure the scan halts as expected?
Feel free to open a new PR, adding additional test changes.
(https://github.com/bitcoin/bitcoin/pull/33064#issuecomment-3126398686)
> Question: would it make sense to also test "abortrescan" during an active rescan to hit the True path and ensure the scan halts as expected?
Feel free to open a new PR, adding additional test changes.
💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719)
> meganit: This newline was logically separating the two commented code blocks and should probably be brought back.
Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)
> nit: It would be good to hyphen-prefix to make it stand out visually (from fields like vsize etc) and also call it "setting" or "argument" rather than "parameter" as those are the terms used in the code.
Fixed as suggested
> nit: I think the end is a bit
...
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719)
> meganit: This newline was logically separating the two commented code blocks and should probably be brought back.
Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)
> nit: It would be good to hyphen-prefix to make it stand out visually (from fields like vsize etc) and also call it "setting" or "argument" rather than "parameter" as those are the terms used in the code.
Fixed as suggested
> nit: I think the end is a bit
...
📝 hebasto opened a pull request: "kernel: create monolithic kernel static library"
(https://github.com/bitcoin/bitcoin/pull/33077)
Currently, consuming `libbitcoinkernel.a` requires all its dependency static libraries to be available. A switch to a monolithic variant, which contains object files from its dependencies, was discussed in the Kernel WG. The necessary preparations in the libsecp256k1 build scripts were completed in https://github.com/bitcoin-core/secp256k1/pull/1678, which are now available in this repository since https://github.com/bitcoin/bitcoin/pull/33036.
The changes in this PR were picked from https://
...
(https://github.com/bitcoin/bitcoin/pull/33077)
Currently, consuming `libbitcoinkernel.a` requires all its dependency static libraries to be available. A switch to a monolithic variant, which contains object files from its dependencies, was discussed in the Kernel WG. The necessary preparations in the libsecp256k1 build scripts were completed in https://github.com/bitcoin-core/secp256k1/pull/1678, which are now available in this repository since https://github.com/bitcoin/bitcoin/pull/33036.
The changes in this PR were picked from https://
...
✅ fanquake closed an issue: "test: RPC coverage check doesn't work?"
(https://github.com/bitcoin/bitcoin/issues/27593)
(https://github.com/bitcoin/bitcoin/issues/27593)
🚀 fanquake merged a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/33064)
(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
(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
...
(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`).
(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.
(https://github.com/bitcoin/bitcoin/pull/33065#pullrequestreview-3061836848)
crACK 251d02084688c67523e9ec92ec79ee657454ab93
Nit: can update the grep command in the PR description.