💬 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`).

🤔 furszy reviewed a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3244943259)
utACK 2427939935f3e6669be6bf553be89639e0afabaa

💬 furszy commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2362946041)
nit:
"Fatal error: expected message '%s' not found in the logs\n"
otherwise it reads as you were printing the debug log.

💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3312291709)
Guix Build:
```bash
2267864350c00270c5adac8da0a4a3ca998f7a1ccdd048e1ce7ad449e381fdba guix-build-2cf742c5214a/output/aarch64-linux-gnu/SHA256SUMS.part
6a132794251abc3c926adc0db07c66ceb512d87c5f9100cad3f0317ad8fbe1a1 guix-build-2cf742c5214a/output/aarch64-linux-gnu/bitcoin-2cf742c5214a-aarch64-linux-gnu-debug.tar.gz
9b89a316337713173bbf57ab4cd16201c2209c928e57c2ad83ea89b8c3f6c6db guix-build-2cf742c5214a/output/aarch64-linux-gnu/bitcoin-2cf742c5214a-aarch64-linux-gnu.tar.gz
ba3346a1e931f90f
...

⚠️ fanquake opened an issue: "ci: add an Alpine (musl libc) job"
(https://github.com/bitcoin/bitcoin/issues/33437)
This exists in some nightly repos alreaady, however we could have some libc diversity in the main CI, especially given static (musl based) binary discussion.