💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786698951)
Have reworked this function in a more general manner just so we can add new args in the future with just one line of parsing code.

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1786733103)
done.

🚀 hebasto merged a pull request: "qt6: Handle deprecated `QLocale::nativeCountryName`"
(https://github.com/bitcoin-core/gui/pull/838)

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2392158642)
Thanks for the review @hodlinator. Updated per feedback.

> Good to see these globals implemented for bench. As already mentioned, I've been working on a partially overlapping change in https://github.com/bitcoin/bitcoin/pull/30737.

Cool. It wasn't on my radar. Adding it now.

💬 JayBitron commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392168251)
@hebasto lld is installed
```
root@localhost:~/.build/bitcoin-macos# make HOST=x86_64-apple-darwin -C depends -j32
make: Entering directory '/root/.build/bitcoin-macos/depends'
Configuring libevent...
-- The C compiler identification is Clang 20.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - failed
-- Check for working C compiler: /usr/bin/clang
-- Check for working C compiler: /usr/bin/clang - broken
CMake Error at /usr/share/cmake-3.30/Modules/CMakeTestCComp
...

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786755937)
True. I'll leave this for a potential follow-up

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786756748)
I'll leave this one for follow-up as well.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1786766139)
We seem to be limited/selective in testing for hidden RPCs in functional tests (i.e. there are more hidden RPCs than `addpeeraddress` and `getrawaddrman`). Maybe that sort of check would/could be a larger scope than just `getorphantxs`?

I'm on the fence about testing that RPCs are hidden. Changing from hidden to another category would be noticed in a PR review, and having a functional test for it means that more code needs to be maintained.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2392209854)
Thanks for the review @danielabrozzoni and @pablomartin4btc!

> Tested on `mainnet`. Not sure if would be use cases to add other arguments on a follow-up (count, filter by node id)

Yes, those sound interesting for a follow-up or something like `getorphanageinfo`.

👍 hebasto approved a pull request: "ci: set a ctest test timeout of 1200 (20 minutes)"
(https://github.com/bitcoin/bitcoin/pull/31026#pullrequestreview-2346588950)
ACK 93dda4c70254750a5b5c0e7f7d2d89032519b999, I agree with the PR goal and the choice of timeout value.

💬 hebasto commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2392236722)
> I tried for quite some time to get this to work using `CTEST_TEST_TIMEOUT` from within _CMakeLists.txt_ but it does not appear to be possible.

According to CMake docs, [`CTEST_TEST_TIMEOUT`](https://cmake.org/cmake/help/latest/variable/CTEST_TEST_TIMEOUT.html)
> Specify the CTest `TimeOut` setting in a `ctest` [dashboard client script](https://cmake.org/cmake/help/latest/manual/ctest.1.html#ctest-script).

which is a use case different from ours.

💬 hodlinator commented on pull request "Windows bitcoind stall debugging [WIP, NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2392267812)
Had [2 scheduled runs](https://github.com/hodlinator/bitcoin/actions) on my [modified master](https://github.com/bitcoin/bitcoin/compare/refs/heads/master...hodlinator:bitcoin:refs/heads/master), 3 hours apart.
- Dump file generation upon bitcoind connection failure is enabled for functional tests, and no longer artificially forced.
- Re-enabled unit tests to keep the setup as close as possible to regular CI.

💬 1440000bytes commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2392275622)
> Instead of a gethostname hack, use the official way of calling GetAdaptersAddresses to get local network addresses on Windows.

Concept ACK

💬 tdb3 commented on issue "ci: `feature_settings.py` failed with `Invalid value detected for '-wallet' or '-nowallet'` in macOS 14 CI":
(https://github.com/bitcoin/bitcoin/issues/31019#issuecomment-2392329519)
> @tdb3 it might be helpful to share link to the full C.I logs?

Thanks. Link now included in the description.

💬 sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1786845164)
I guess it may be worth crating an issue / opening a separate PR to fix this, given it is unrelated

💬 1440000bytes commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2392352182)
I think it's too early to add this option but it will remain unused by most and off by default. I don't see anything wrong with some users testing it.

This doesn't hide the fact that a user is running a bitcoin node (most IPv4 nodes use default port). It just ensures all peers are using v2 for P2P.

👍 1440000bytes approved a pull request: "netinfo: add peer services column and outbound-only peers list"
(https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2346741932)
ACK https://github.com/bitcoin/bitcoin/pull/30930/commits/683b558a020f1632b0a3cbdaa165adbd1423281c

💬 1440000bytes commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#issuecomment-2392391647)
Concept ACK

💬 hebasto commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392448801)
Here are the steps to build depends without Qt on Debian 12:
```
apt update
apt install git bison cmake curl make pkg-config xz-utils wget
# Install minimum required version of LLVM toolchain
apt install clang-16 llvm-16 lld-16
# Create non-versioned symlinks of the tools
ln -s /usr/bin/clang-16 /usr/bin/clang
ln -s /usr/bin/clang++-16 /usr/bin/clang++
ln -s /usr/bin/lld-16 /usr/bin/lld
ln -s /usr/bin/ld.lld-16 /usr/bin/ld.lld
ln -s /usr/bin/lld-link-16 /usr/bin/lld-link
ln -s /usr/b
...

💬 JayBitron commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2392474110)
`/usr/bin/ld: unrecognised emulation mode: llvm`