💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2389936885)
Ah, I guess I could have written the comment clearer. I meant `Use the earliest Xcode requiring at least the version of macOS denoted in ...`. Otherwise, we'd unnecessarily limit our Xcode features to EOL versions of macOS.

Whether to pick the earliest minor version of Xcode or earliest major version of Xcode is not specified, but I think going with the earliest major version here seems fine, to be able to remove the fuzz.cpp workaround.

💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3350089468)
> The minimum supported version in the release notes should only be changed with the along with changing the minimum supported version in depends.

Thx, done. However, I can't test the resulting depends change on macOS myself.

🤔 Eunovo reviewed a pull request: "test: Fix rpc_getblockstats for no-wallet environments"
(https://github.com/bitcoin/bitcoin/pull/33184#pullrequestreview-3282780757)
I think this test could be simplified to always generate data with the MiniWallet and test with generated data, instead of dumping it to a file and loading it later. I don't see the advantage of "generate" and "load" paths, and it's creating unnecessary complexity.

💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2389993284)
https://github.com/bitcoin/bitcoin/pull/33184/commits/12c9c7d927091f3be72f4acd99c166a41c789ae9:
IIUC, `MiniWallet` doesn't require wallet rpc, so what caused the "-disablewallet" error?

💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2389995639)
https://github.com/bitcoin/bitcoin/pull/33184/commits/316cf41c25438d61f4e9568e20878fa374476a5a:
I see that the CI fails because this argument conflicts with the argument defined in `rpc_blockstats`.

💬 maflcko commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2390009870)
You'll also have to check the fee? Otherwise, this just seems like a duplicate of `sendmsgtopeer 0 "tx" "$txhex"`?

💬 maflcko commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#issuecomment-3350177873)
> I don't see the advantage of "generate" and "load" paths, and it's creating unnecessary complexity.

I guess it depends on how long it takes to generate, but if it is just a few seconds, it seems easier not to cache.

💬 Eunovo commented on pull request "test: Fix rpc_getblockstats for no-wallet environments":
(https://github.com/bitcoin/bitcoin/pull/33184#issuecomment-3350187225)
@maflcko This is from my 8-core laptop
```
☁ bitcoin [2e1836d744] build/test/functional/test_runner.py rpc_getblockstats --gen-test-data
Temporary test directory at /tmp/test_runner_₿_🏃_20250930_083649
Remaining jobs: [rpc_getblockstats.py]
1/1 - rpc_getblockstats.py passed, Duration: 1 s

TEST | STATUS | DURATION

rpc_getblockstats.py | ✓ Passed | 1 s

ALL | ✓ Passed | 1 s (accumulated)
Runtime: 1 s

```

💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3350214088)
My Guix build:
```
aarch64
b286e83a69f60494b86be866ce3b4aaa7c58aeb2e9410a38b898c2afe6946f59 guix-build-980e4987db04/output/aarch64-linux-gnu/SHA256SUMS.part
c9bcfe3c18dd5a7130c3ccbb17f8f51f7486a9bbfdb57c9ee59164003013de39 guix-build-980e4987db04/output/aarch64-linux-gnu/bitcoin-980e4987db04-aarch64-linux-gnu-debug.tar.gz
f72be7de9bb5d037e9e27db645d8578f031e6a7b6b9096c573dfba56cbcb8a0c guix-build-980e4987db04/output/aarch64-linux-gnu/bitcoin-980e4987db04-aarch64-linux-gnu.tar.gz
ff9ff073
...

💬 maflcko commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#issuecomment-3350234120)
lgtm ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd

🤔 janb84 reviewed a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3282863818)
lgtm ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd

💬 hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3350313336)
@davidgumberg
> Trying to do a guix build on this branch I get the following error:

Is this consistently reproducible, or does it occur intermittently?

👍 hebasto approved a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3282958097)
ACK d4f47f97715c7b6a2879e99c62f09ccead8cc4cd, I have reviewed the code and it looks OK.

🚀 hebasto merged a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487)

👍 vasild approved a pull request: "depends: Update URL for `qrencode` package source tarball"
(https://github.com/bitcoin/bitcoin/pull/33494#pullrequestreview-3282991826)
ACK 980e4987db0450cabb8761433001841f7e9f4c73

I confirm the diff from the OP. The old archive is not available anymore at https://fukuchi.org/works/qrencode/qrencode-4.1.1.tar.gz, so I downloaded it from https://bitcoincore.org/depends-sources/qrencode-4.1.1.tar.gz instead. The new one is at https://github.com/fukuchi/libqrencode/archive/refs/tags/v4.1.1.tar.gz.

Then I extracted each archive, did
```sh
for f in $(find ./ -type f |sort) ; do sha256sum "$f" ; done
```
and compared the resulting d
...

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3350439700)
`82e46a7248...2ae966b222`: address suggestions, thanks!

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2390185247)
Right! Changed to `target_addr`.

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2390186651)
I like `proxy_override`, changed.

💬 hebasto commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390237740)
So people on macOS 14.4 with Xcode 15.4 and Apple Clang 1500.3.9.4 based on LLVM 16.0 will fail to compile `fuzz.cpp`, right?

💬 luke-jr commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#discussion_r2390259053)
Seems kind of ugly to lose the package name in the filename. How about `$(package)-$($(package)_version)-github.tar.gz` or something?