💬 hebasto commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
This code emits `-Wunused-variable` and fails to build with `-Werror` on *BSD systems:
```
/home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/src/common/system.cpp:116:10: error: unused variable 'clamp' [-Werror,-Wunused-variable]
116 | auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
| ^~~~~
1 error generated.
*** Error code 1

Stop.
```

See: https://github.com/hebasto/bitcoin-core-nightly

💬 hebasto commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362554073)
This should work:
```suggestion
std::optional<size_t> GetTotalRAM()
{
auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
#ifdef WIN32
if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) return clamp(m.ullTotalPhys);
#else
if (long p{sysconf(_SC_PHYS_PAGES)}, s{sysconf(_SC_PAGESIZE)}; p > 0 && s > 0) return clamp(1ULL * p * s);
#endif
return std::nullopt;
}

```

See CI outputs for:
- Fr
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3311820597)
`3f6a61e3c5...3aa74b5176`: rebase due to conflicts

💬 vasild commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362610449)
I came here for the same reason ;-) Here is the patch I devised:

```diff
std::optional<size_t> GetTotalRAM()
{
- auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
+ [[maybe_unused]] auto clamp{[](uint64_t v) { return size_t(std::min(v, uint64_t{std::numeric_limits<size_t>::max()})); }};
#ifdef WIN32
if (MEMORYSTATUSEX m{}; (m.dwLength = sizeof(m), GlobalMemoryStatusEx(&m))) return clamp(m.ullTotalPhys);
-#elif define
...

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3311940236)
> I don't have anything against automatically toggling silent-payments flag when a silent payment descriptor is imported.

It later occurred to me that we shouldn't do this because setting the silent-payments flag also disables "fast rescan"; the user needs to make a conscious decision to enable silent-payments and disable "fast rescan" in the process.

Making it possible to toggle "silent-payments" via "setwalletflag" makes more sense to me, but then we have to also worry about what we shou
...

💬 hebasto commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362723602)
> I did not go with `#else` since I am not sure that will compile and work on all possible platforms that are not Windows.

Right. We could end up with macros for all tested platforms.

💬 Crypt-iQ commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3312041876)
crACK 2427939

🤔 hebasto reviewed a pull request: "build: Remove lingering Windows registry & shortcuts (#32132 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/33422#pullrequestreview-3244721631)
Concept ACK.

@hodlinator Thank you!

Another alternative is to ask the user to uninstall any previous installation before installing a new one, but this is suboptimal.

📝 fanquake opened a pull request: "depends: static libxcb-cursor"
(https://github.com/bitcoin/bitcoin/pull/33434)
Haven't tested this, or looked for potential side effects (i.e #33432 claims that installing the dep breaks dark mode).

💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3312081110)
@benthecarman any chance you can test a Guix built binary?

💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3312099790)
> if we take the last commit: fixup

I feel like this is going to be taken, no? Especially as we think ahead with relay (say, 1p1pc packages), seems necessary to do the INV/getdata cycle.

💬 instagibbs commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#discussion_r2362811890)
"unknown-not-validated" needs some functional coverage? :pray:

📝 vasild opened a pull request: "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/33435)
This patch achieves two things:
1. Fix unused variable warning (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
2. Enable `GetTotalRAM()` on FreeBSD. The exact same code works and produces correct results on FreeBSD, so just extend the `#elif` condition.

Prior discussion: https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046

💬 vasild commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362830323)
PRed in https://github.com/bitcoin/bitcoin/pull/33435

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3312186763)
@instagibbs, yes, this was already taken some time ago and squashed into the relevant commit: https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2795829514

💬 john-moffett commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#discussion_r2362907654)
Currently not reachable as the [RPC](https://github.com/bitcoin/bitcoin/blob/74fa028da1ea38c5348f988464074899684cebcf/src/rpc/mempool.cpp#L1054):

`CHECK_NONFATAL(m_tx_results.size() == txns.size() || m_tx_results.empty())`

and acceptance [code](https://github.com/bitcoin/bitcoin/blob/74fa028da1ea38c5348f988464074899684cebcf/src/validation.cpp#L1852):

`Assume(results_final.size() == package.size())`

both are all-or-nothing in terms of package result reporting. I put that there as a
...

💬 hebasto commented on pull request "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2362917905)
```suggestion
#elif defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__illumos__)
```

🤔 hebasto reviewed a pull request: "system: silence unused variable warning and make GetTotalRAM() work on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3244906271)
Concept ACK.

💬 instagibbs commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#discussion_r2362921924)
may be redundant but could add a more local assertion to this fact; easier for future reviewers to see what is unexpected?

📝 fanquake opened a pull request: "ci: run s390x job"
(https://github.com/bitcoin/bitcoin/pull/33436)
How fast does this run? (`-md`).