Bitcoin Core Github
42 subscribers
125K links
Download Telegram
👍 stickies-v approved a pull request: "fix: remove redundant mempool lock in ChainImpl::isInMempool()"
(https://github.com/bitcoin/bitcoin/pull/33946#pullrequestreview-3510522471)
ACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
💬 maflcko commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3580941170)
review ACK 2e27bd9c3af91eb9fcc626fe65d065df0a80974d 🖊

<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 2e27bd9c3af9
...
💬 maflcko commented on pull request "interfaces: remove redundant mempool lock in ChainImpl::isInMempool()":
(https://github.com/bitcoin/bitcoin/pull/33946#issuecomment-3580958940)
lgtm ACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2564710993)
I agree but we use `sp` in related functions too.
👍 theStack approved a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3510583287)
re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db

as per `$ git range-diff fa2fd0ba1f...fad6118586`
💬 Sjors commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2564711983)
Will change if I need to retouch.
💬 stickies-v commented on pull request "Change Parse descriptor argument to string_view":
(https://github.com/bitcoin/bitcoin/pull/33914#discussion_r2564733112)
There is value in consistency, but I don't think we shouldn't improve naming because it's used somewhere else, especially when it's an implementation detail.

That said, this is a nit, and I don't think it's worth invalidating acks as it looks RFM.
💬 stickies-v commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-3581039213)
ACK 3e01b5d0e7 modulo the new .tar SDK being uploaded
💬 naiyoma commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2564780295)
Done
💬 GarmashAlex commented on pull request "docs: sync external-signer.md with current external signer flow":
(https://github.com/bitcoin/bitcoin/pull/33947#issuecomment-3581066113)
> Thanks, however there is an existing PR at #33765. I'd suggest reviewing there first.

Oh, actually didn't notice that PR
💬 naiyoma commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2564803085)
thanks for the review, I’ve pushed an update and made the function more generic
💬 TheCharlatan commented on pull request "kernel: don't use assert to handle invalid user input":
(https://github.com/bitcoin/bitcoin/pull/33943#issuecomment-3581241991)
I'm Concept ACK on improving the behaviour around bad input, but not sure about adding more error handling surface to the API with the current approach. In my opinion the cases addressed are all programming errors, and should be treated as such. The argument that this duplicates the error handling code for the wrappers is not convincing to me and the PR itself as it currently stands provides the counter argument for it - an out of bounds case is still added and handled by the respective language
...
🤔 janb84 reviewed a pull request: "depends: latest config.guess & config.sub"
(https://github.com/bitcoin/bitcoin/pull/33945#pullrequestreview-3510885486)
Post merge ACK 3e4355314b1abf8e4456ea41ba738aaae25abb73

I had a guix build still running, outcome is equal to the rest :)

my Guix Build Output

**Host architecture:** `aarch64`
**Commit:** `3e4355314b1a`

```shell
50d2ead905b87b4064fd66a0c0a2dc88a7ada65e2224f1ff992bc7ec685e493d guix-build-3e4355314b1a/output/aarch64-linux-gnu/SHA256SUMS.part
923b405f4eb8cb7158f2b4aef0918e48e7e57ef097871d0702dbbd5286013d32 guix-build-3e4355314b1a/output/aarch64-linux-gnu/bitcoin-3e4355314b1a-aa
...
🤔 rkrux reviewed a pull request: "Change Parse descriptor argument to string_view"
(https://github.com/bitcoin/bitcoin/pull/33914#pullrequestreview-3510903589)
lgtm ACK c0bfe72f6e1f63e05772eda959137b3d0bbbf6c3
💬 laanwj commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3581295709)
Sorry, we had a few delays and technical difficulties😓 (#33873). But good news, the riscv64 guix build just finally completed, and matches the other ones:
```
riscv64
05fc5cae41b19bbfdec5942e334e7b7d78ee0406ba69c221381b0b07b0a9e886 guix-build-f06c6e189831/output/aarch64-linux-gnu/SHA256SUMS.part
961613408c4d0b3b169488b43edc838135be0b712740b4015457f4f2808e103b guix-build-f06c6e189831/output/aarch64-linux-gnu/bitcoin-f06c6e189831-aarch64-linux-gnu-debug.tar.gz
906437c316d50e1f60fdfe2098fe7
...
💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3581307795)
@laanwj thanks for following up!
💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3581401865)
reACK 70d9e8f0a15d07a27ae37befb5c1bce71c98d8de
💬 instagibbs commented on pull request "interfaces: remove redundant mempool lock in ChainImpl::isInMempool()":
(https://github.com/bitcoin/bitcoin/pull/33946#issuecomment-3581411365)
utACK 2909655fba91a7cc59c484fc74afafdf7ccc0cfa
📝 fanquake opened a pull request: "ubsan: add another suppression for InsecureRandomContext"
(https://github.com/bitcoin/bitcoin/pull/33949)
We've been suppressing this in infra for a little while, it's now appearing elsewhere. i.e
https://github.com/willcl-ark/bitcoin/actions/runs/19702968577/job/56443491265#step:9:2738:
```bash
Run bip324_cipher_roundtrip with args ['/home/runner/work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch/qa-assets/fuzz_corpora/bip324_cipher_roundtrip')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2138761624
INFO: Loaded 1 modules (612416 inlin
...
💬 fanquake commented on pull request "ubsan: add another suppression for InsecureRandomContext":
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3581424072)
cc @willcl-ark @dergoegge