💬 yuvicc commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#issuecomment-3380164166)
ACK a1226bc760c70a22ef4a197d5690aca4d83cb74c

This will be useful for new devs while verifying the subtree.

💬 purpleKarrot commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3380174569)
ACK 362d43a659224f9969b268dcc859162006460214

> Note that some of the std::runtime_error instances can likely be std::invalid_argument or std::logic_error - if the reviewers have stronger preferences, I can adjust.

Long term, it should be evaluated whether some contracts could be tightened by turning runtime error cases into preconditions.

💬 fazle1337-del commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3380203211)
I don't understand why we want to limit he ability of the node operator. Does this not disenfranchise the people and move to a more centralised governance. The problem is, once malicious data is onchain, it's going to need a hard fork to remove it. Can this be discussed further before the release happens please

💬 rkrux commented on pull request "wallet: Track no-longer-spendable TXOs separately":
(https://github.com/bitcoin/bitcoin/pull/27865#issuecomment-3380212696)
Concept ACK 7b2718f

Seems like a natural step after #27286, will get to this PR soon.

🤔 hodlinator reviewed a pull request: "refactor: replace `const char*` exceptions with `std::runtime_error`"
(https://github.com/bitcoin/bitcoin/pull/33569#pullrequestreview-3313395594)
Reviewed 362d43a659224f9969b268dcc859162006460214

(I agree with the general rule of not throwing random pointer types, but when its `const char*` from the static text section of the executable that are never deallocated during the process lifetime I think it's acceptable).

💬 hodlinator commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2412874356)
nit: Using `runtime_error` everywhere in these heavily `consteval`/`constexpr` contexts feels slightly off, so I agree with what you suggested in the PR description.
```suggestion
throw std::invalid_argument{"Only lowercase hex digits are allowed, for consistency"};
```

💬 hodlinator commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2412967418)
I don't get it, https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401 logs:
`
test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation on the raised exception through predicate "HasReason{error}"
`

Looking at where the error message comes from - https://www.boost.org/doc/libs/1_87_0/boost/test/tools/old/interface.hpp, it has:
```C++
#define BOOST_CHECK_THROW_I
...

💬 hebasto commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2413024997)
See https://github.com/mstorsjo/llvm-mingw/issues/522.

💬 Dorex45 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3380342538)
Each node can configure it's policy rules as it sees fit and transactions must first pass consensus rules before considering policy. This means that there is no risk that using different policy rules will break consensus. Another way to say this is that standard transactions are a subset of valid transactions.

💬 Dorex45 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3380347327)
Candidate Release V30 ACK

🚀 fanquake merged a pull request: "[27.x] Fix Qt download URLs"
(https://github.com/bitcoin/bitcoin/pull/33564)

💬 fanquake commented on pull request "[28.x] ci: Fix Qt 5.15 URL":
(https://github.com/bitcoin/bitcoin/pull/33561#issuecomment-3380416127)
Backported to 27.x in #33564.

💬 fanquake commented on pull request "[29.x] build: fix depends Qt download link":
(https://github.com/bitcoin/bitcoin/pull/33563#issuecomment-3380417586)
Backported to 27.x in #33564.

💬 fanquake commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3380466804)
Binaries for `v30.0rc3` are available: https://bitcoincore.org/bin/bitcoin-core-30.0/test.rc3/.

💬 willcl-ark commented on issue "build: depends sqlite compile failure for FreeBSD Clang cross":
(https://github.com/bitcoin/bitcoin/issues/33560#issuecomment-3380469053)
Cross-compiling for freebsd from linux using clang is just currently untested/unused.

This branch includes a patch to fix sqlite use of `mremap` and a build script demonstrating a freebsd crosscompile from linux using clang and a freebsd sysroot:

https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:cross-compile-freebsd-depends

(tested with clang18 and freebsd 14.2 and 14.3 only).

If we moved to using clang in the guix builds a similar process could perhaps be used to provi
...

💬 fanquake commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3380545850)
> MinGW also defines environ ... causing the same inconsistent linkage warning.

Can you improve the PR description & commit message? We use mingw-w64 in the CI & Guix build, and it doesn't produce this warning.

💬 maflcko commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3380547548)
lgtm, but it seems this is a workaround for an upstream bug?

In any case, the reason why a raw string literal was thrown, because the primary goal of this is inside a consteval context, and it avoids an include. I guess you'd have to add the include here, if you start using runtime_error. Though, that feels off for this context, as mentioned previously.

Out of curiosity, I wonder, would it work if a string_view was thrown?

💬 hodlinator commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2413151212)
Great! Would prefer something like this diff applied to the PR description and commit message to make it even clearer that this is primarily a workaround for a compiler issue and secondarily abiding by guidelines:
```diff
Fixes MinGW/libc++ test failures where `BOOST_CHECK_EXCEPTION` with `const char*` exceptions
- failed due to platform-specific exception handling differences.
+ is broken, see https://github.com/mstorsjo/llvm-mingw/issues/522
```

⚠️ fanquake opened an issue: "build: Qt deprecated-declarations warnings"
(https://github.com/bitcoin/bitcoin/issues/33571)
Building master (b510893d00760083ac36948747aa6ebd84656192) against Qt 6.10.0:
```bash
/root/bitcoin/src/qt/transactionfilterproxy.cpp: In member function ‘void TransactionFilterProxy::setDateRange(const std::optional<QDateTime>&, const std::optional<QDateTime>&)’:
/root/bitcoin/src/qt/transactionfilterproxy.cpp:57:21: warning: ‘void QSortFilterProxyModel::invalidateFilter()’ is deprecated: Use begin/endFilterChange() instead [-Wdeprecated-declarations]
57 | invalidateFilter();
|
...

💬 maflcko commented on pull request "DRAFT: add a freebsd job using systemlibs":
(https://github.com/bitcoin/bitcoin/pull/33562#discussion_r2413156742)
no strong opinion, but is there a rationale about the scope here? why disable the fuzz binary?

💬 willcl-ark commented on pull request "DRAFT: add a freebsd job using systemlibs":
(https://github.com/bitcoin/bitcoin/pull/33562#discussion_r2413178459)
No opinion from me either, this is just a tranlation mistake from https://github.com/hebasto/bitcoin-core-nightly/blob/36cc3ef2703d6f662e9a96c21db5d83d80e7efb9/.github/workflows/freebsd.yml#L58 where it is indeed enabled.

I will re-enable it in next push.