💬 instagibbs commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-2597189917)
@whitslack yes I authored the migration, and v2<->v0 roundtripping is supported in libwally as well. Would be nice to move the industry a bit closer to making v2 the defacto default.

💬 davidgumberg commented on pull request "depends: Use base system's `sha256sum` utility on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/31626#issuecomment-2597211469)
Tested depends build on a fresh BSD 14.2 install[^1], `sha256sum` is available and `shasum` is not.

```console
root@freebsd:~ # freebsd-version
14.2-RELEASE
root@freebsd:~ # shasum
-sh: shasum: not found
root@freebsd:~ # sha256sum --version
sha256sum (FreeBSD) 14.2
```

<details> <summary> Full build log</summary>

```console
root@freebsd:~ # fetch https://github.com/bitcoin/bitcoin/archive/refs/heads/master.zip
root@freebsd:~ # sha256sum master.zip
30cadd9f1293505f4e70173d9d1e
...

💬 davidgumberg commented on pull request "depends: Use base system's `sha256sum` utility on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/31626#discussion_r1919427041)
```suggestion
build_freebsd_DOWNLOAD = fetch --timeout 180 --retry -o
```

related but probably out-of-scope: freebsd's `fetch` utility could be used to also drop the curl dependency, there is a regression since it is unable to limit number of retries([16-year old patch awaits feedback](https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=131427)), and to set a handshake timeout, but these can be approximated with a single connection timeout

💬 davidgumberg commented on pull request "init: Lock blocksdir in addition to datadir":
(https://github.com/bitcoin/bitcoin/pull/31674#issuecomment-2597266508)
Concept ACK

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#discussion_r1919456572)
Thanks, fixed.

👍 tdb3 approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2557869188)
re ACK cb305da72df501991a6700cd20be79dde4591184

Nice cleanup. It seems reasonable to have removed the spend test.

`git range-diff e7c479495509c068215b73f6df070af2d406ae15..712e3ffde5fa4fd26c0f217942b4a289cf37f361 e7c479495509c068215b73f6df070af2d406ae15..cb305da72df501991a6700cd20be79dde4591184`

👍 tdb3 approved a pull request: "rpc: fix mintime field testnet4, expand timewarp attack test"
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2557911778)
code review ACK 4d8dea9cb92efa362f72b0c2a5b89d312b452c0a

Comments were addressed. Clarity increased.

💬 NicolaLS commented on pull request "doc: Improve dependencies documentation":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1919662581)
thank you. so the PR to reference for the "version used" would be #27172 correct ?

[manifest](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/manifest.scm) uses `((gnu packages cmake) #:select (cmake-minimal))` which is why we can find the version ([relevant comment in cmake.scm](https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/cmake.scm?id=713ac1fe47de1a5f29f6186e51d9ffb2b8083258#n164))

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2597622776)
Rebased b776e40554c8a315d832f3996d14ff2e3ae7b8cb -> 7f484b319bdc9e646e270b6d6d9a1c7e52296dc0 ([blockmanDB_7](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_7) -> [blockmanDB_8](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_8), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_7..blockmanDB_8))

* Resolved conflict with #31483

💬 Sjors commented on pull request "gui, psbt: Use SIGHASH_DEFAULT when signing PSBTs":
(https://github.com/bitcoin-core/gui/pull/850#issuecomment-2597680511)
@pablomartin4btc it's part of #31622.

The description in `interpreter.h` is ambiguous. It says "Taproot only". It also says "equivalent to SIGHASH_ALL", but doesn't say that it gets converted (and obviously a constant can't guarantee that actually happens).

💬 hebasto commented on pull request "doc: Improve dependencies documentation":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1919735896)
>so the PR to reference for the "version used" would be #27172 correct ?

That refers to a Darwin-specific change. https://github.com/bitcoin/bitcoin/pull/29725 is a PR, which made `cmake-minimal` a global requirement.

However, in the future, updates like https://github.com/bitcoin/bitcoin/pull/30730 might bump the `cmake-minimal` version.

💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1919757571)
I look the latter, and removed the `_linux` and `_darwin` specific variables.

cc @fanquake

💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2597719684)
Testing https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1919757571 with cherry-picked #31675.

💬 TheCharlatan commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2597722897)
Guix builds (aarch64):
```
463ef2208f9c5ce3cf884fd2cf8fba8f2aabded19ba43620178aea41e8c8fd1d guix-build-a1add80c80bc/output/aarch64-linux-gnu/SHA256SUMS.part
3a87fe313164fed822437e35511febdbdf0d8e9feda099dfef4fd9031735916f guix-build-a1add80c80bc/output/aarch64-linux-gnu/bitcoin-a1add80c80bc-aarch64-linux-gnu-debug.tar.gz
a20f31417cbb9994eed0e88700c3e1f8c714282425cbe5fb33a5c7ac7b7c68c2 guix-build-a1add80c80bc/output/aarch64-linux-gnu/bitcoin-a1add80c80bc-aarch64-linux-gnu.tar.gz
51fbc8b8a
...

💬 hebasto commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1919762346)
FWIW, building the `capnp` package still [fails](https://github.com/bitcoin/bitcoin/pull/31484#issuecomment-2543029821) on OpenBSD.

💬 Sjors commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-2597726644)
@whitslack @instagibbs thanks for mentioning CLN. Is it possible to turn off its round tripping so I can test this PR against it? And are there any good nodes on signet or testnet4?

👍 maflcko approved a pull request: "ci: Supply `--platform` argument to `docker` commands."
(https://github.com/bitcoin/bitcoin/pull/31657#pullrequestreview-2558295404)
lgtm ACK e1c7c29727faf2d0a5c6fa01505418aac26958fe

I think some code can be removed to be more streamlined, but not a blocker.

💬 maflcko commented on pull request "ci: Supply `--platform` argument to `docker` commands.":
(https://github.com/bitcoin/bitcoin/pull/31657#discussion_r1919767906)
Why the code above? Seems easier to just force "linux" (the fallback/default), which documents the requirements. It seems to work with podman, and I presume also works for the others?



```suggestion
export CI_IMAGE_PLATFORM=${CI_IMAGE_PLATFORM:-"linux"} # native arch by default
```

💬 maflcko commented on pull request "ci: Supply `--platform` argument to `docker` commands.":
(https://github.com/bitcoin/bitcoin/pull/31657#discussion_r1919775445)
> Don't fully grasp the context of #28138 but `native_nowallet_libbitcoinkernel`

If it is no longer happening, I guess the workaround can be removed.

🤔 hebasto reviewed a pull request: "build: Make config warnings fatal if -DWERROR=ON"
(https://github.com/bitcoin/bitcoin/pull/31665#pullrequestreview-2558311790)
I don't like the idea of overloading the `WERROR` build option with additional functionality, as this is not what users would expect.

However, if this PR proceeds in its current direction, the new functionality must be clearly documented in the `WERROR` build option's help string.

> Although one thing that could be improved is making the error more clear, as in the current output, it's hidden far back in the scrollback.

There is no need to continue after encountering an error:
```diff
...