💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1940941450)
Nit: This change and the corresponding function call updates can be the first commit followed by the actual repairing. But I can understand if it's a bit much for this PR given its limited focus area.

💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940966104)
why is this needed after 6f5ae1a574562bf3fe938f704592c5374b43091a, which has this as a fallback?

💬 willcl-ark commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940970290)
It is not needed since that is now merged

🤔 TheCharlatan reviewed a pull request: "Double check all block rules in `ConnectBlock`, not only `CheckBlock`"
(https://github.com/bitcoin/bitcoin/pull/31792#pullrequestreview-2592504179)
Concept ACK

💬 TheCharlatan commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1940976853)
In commit cddc0e2a0f2b8f19ef243edc8a85e206c31f9e62:
Can you add some context to the commit message for why this move is being done? Maybe:
"A call to ContextualCheckBlockHeader is added to ConnectBlock in the following commits, but it should not re-check the timestamp of the header there. Move the timestamp check into a separate function to avoid this."

💬 TheCharlatan commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1940977644)
Why is this not `LogError`?

💬 willcl-ark commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940990613)
As far as I can tell, running `bash -c '( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && RUST_BACKTRACE=1 cargo run )'` in a worktree already works fine for me, because the .git directory (referenced by the `.git` file that a worktree creates) is available to me, unlike in the containerised scenario where this path must be explicitly mounted into the container at runtime, which is what the helper script does.

I added some more comments to describe why the mount is needed in the s
...

💬 willcl-ark commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1940993491)
I don't think the file touch method will work for the same reasons we discussed above (needing the actual .git dir with all its objects and refs for tidy), so have not opted to go for that currently.

💬 maflcko commented on pull request "rpc: collect transaction fees on generateblock":
(https://github.com/bitcoin/bitcoin/pull/31775#issuecomment-2633641702)
The code doesn't pass existing tests, and doesn't have new tests. Also, the approach won't work anyway, because the transactions may not be in the mempool. You'd have to calculate the fees based on the prevout amounts for each transaction.

💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1941030929)
```suggestion
docker run "${DOCKER_ARGS[@]}" bitcoin-linter
```

I don't think this env var will affect anything?

Also, style-wise I wonder if the contents of this file should be put into `ci/lint_run_all.sh`, so that there is only one bash script, instead of multiple.

Ideally there would only be one script, used by ci and locally, but that can be a follow-up. For now, just a single large if in `ci/lint_run_all.sh` should be fine to switch between the cirrus_ci code paths ( current con
...

💬 Sjors commented on pull request "test: added additional coverage to waitforblock and waitforblockheight rpc's":
(https://github.com/bitcoin/bitcoin/pull/31784#issuecomment-2633729812)
utACK 7e0db87d4fff996c086f6e86b62338c98ef30c55

💬 sipa commented on pull request "consensus: Remove checkpoints (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31649#issuecomment-2633746201)
utACK 301017c6217378ca868e17c7cb8ffe3447abb11d

💬 Sjors commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2633772617)
Silent merge conflict with bitcoin-core/gui#850.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2633863663)
re: https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2633426678

> I'm unable to make a Guix build.

Thanks, created https://github.com/chaincodelabs/libmultiprocess/issues/150 to track this

💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2633897096)
> macOS cross compile fails (in a different way): https://cirrus-ci.com/task/5052270577975296
>
> ```
> [09:45:29.772] /bin/sh: 1: /ci_container_base/depends/x86_64-apple-darwin/bin/capnp: Exec format error
> ```

It should call `/ci_container_base/depends/x86_64-apple-darwin/native/bin/capnp`.

💬 maflcko commented on pull request "rpc: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1941170754)
What is the connection to RPC? I was under the impression that `-debug` is a setting related to logging.

💬 hebasto commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2634033316)
I cannot reproduce the issue on my MacOS Sequoia 15.2 (24C101):
```
% clang --version
Apple clang version 16.0.0 (clang-1600.0.26.6)
Target: arm64-apple-darwin24.2.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
% cmake -B build -DWITH_BDB=ON
<snip>
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
-- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Success
<snip>
```

Can anyone else provide the exact steps to reproduce the is
...

💬 brunoerg commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1941236428)
You're right, could happen. I just added a try/except around it, now it checks the duration is greater than 0 or if the field is not returned into `getwalletinfo` response.

💬 ryanofsky commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2634071115)
Thanks was able to get a stack trace by running `cd /ci_container_base/ci/scratch/build-i686-pc-linux-gnu`, `gdb -ex run --args ./src/test/test_bitcoin -t ipc_tests`, and `bt` in container:

```c++
#0 0xf7f4b5b9 in __kernel_vsyscall ()
#1 0xf79e6037 in ?? () from /lib32/libc.so.6
#2 0xf7994c51 in raise () from /lib32/libc.so.6
#3 0xf797c2b7 in abort () from /lib32/libc.so.6
#4 0xf7d4bf71 in ?? () from /lib32/libstdc++.so.6
#5 0xf7d65a97 in ?? () from /lib32/libstdc++.so.6
#6 0xf7d4b8f9 in
...

💬 jonatack commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1941252509)
Yes, I suggested these in https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1940010147.