💬 vasild commented on issue "Apparently CJDNS network does not work with Tor on mainnet.":
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2854552868)
> Maybe we should consider something like `-proxy=1.2.3.4:5678=ipv4` for a more fine-grained control?

Done in https://github.com/bitcoin/bitcoin/pull/32425

I considered the other options, but did not want to change the semantic of `-proxy` to exclude the CJDNS network (would be a kind of backward incompatible, or breaking, or unexpected change). Also, I did not want to add another option `-cjdnsproxy=addr:port`.

Extending `-proxy` is backwards compatible and no need to add more options. Also,
...

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2854577888)
> Would be good to register this new type into the Qt's meta-object system.

Good catch, thank you. Added it in.



> It's likely that adding only `Q_DECLARE_METATYPE(Txid)` at the top is enough

Thanks, makes sense. I checked the Qt docs too and found this:
"Note that if you intend to use the type in queued signal and slot connections or in [QObject](https://doc.qt.io/qt-5/qobject.html)'s property system, you also have to call [qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#
...

⚠️ theStack opened an issue: "external signer: PSBT error code `EXTERNAL_SIGNER_NOT_FOUND` is never returned"
(https://github.com/bitcoin/bitcoin/issues/32426)
The error code [`PSBTError::EXTERNAL_SIGNER_NOT_FOUND`](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/common/types.h#L20) is never returned anywhere in our codebase, hence code that handles it (currently done [only in the GUI](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/qt/sendcoinsdialog.cpp#L458)) is effectively dead. I have never looked deeper into external signing, but at a first glance the two obvious logical ch
...

🤔 maflcko reviewed a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2818284848)

review ACK 22cff32319de64cb98e1c89b9a7ed35611e89e27 🏣

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 22cff32319
...

🤔 polespinasa reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2818267234)
The code suggested uses the `InitWarning` and handles the errors on the failing test stoping the nodes manually

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075471003)
```python
self.expected_stderr = [
"", # node 0 has no deprecated options
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in future versions.",
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in future versions.",
"Warning: Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated.
...

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075469037)
```c++
InitWarning(_("Options '-datacarrier' or '-datacarriersize' are set but are marked as deprecated. They will be removed in future versions."));
```

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075473533)
At the end of the test add:

```python
for i in range(self.num_nodes):
self.stop_node(i, expected_stderr=self.expected_stderr[i])
```

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075481564)
Code suggested here: https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2818267234

👍 hebasto approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2818293785)
re-ACK 22cff32319de64cb98e1c89b9a7ed35611e89e27.

💬 hebasto commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2854604811)
> > Would be good to register this new type into the Qt's meta-object system. Look at the RegisterMetaTypes() function on src/qt/bitcoin.cpp.
>
> Good point. It's used in at least the `TransactionView::bumpedFee` signal, which might be broken now.
>
> It's likely that adding only `Q_DECLARE_METATYPE(Txid)` at the top is enough as this was enough for uint256.

I don't think it is required in Qt 6.

See: https://www.qt.io/blog/whats-new-in-qmetatype-qvariant

✅ maflcko closed an issue: "ARM Windows build and release"
(https://github.com/bitcoin/bitcoin/issues/31388)

💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2854625982)
Closing for now. This can be re-opened when there is a toolchain to achieve this. Even then, the toolchain would need to make it into guix and ideally other package managers as well.

💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2075520941)
forgot to remove the comment?

💬 stickies-v commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#issuecomment-2854656467)
Force-pushed to remove the `GetAll() const` method (and commit), replacing its callsites with directly accessing `{m_ibd_chainstate.get(), m_snapshot_chainstate.get()}` as [suggested](https://github.com/bitcoin/bitcoin/pull/32364?notification_referrer_id=NT_kwDOBB0EGbQxNjExMTQxNTE5MDo2OTAxMDQ1Nw#pullrequestreview-2810060421) by @mzumsande.

> Given that #30214 wants to removes `GetAll()` anyway in [64fc660](https://github.com/bitcoin/bitcoin/commit/64fc6603525f515c5303f557b038753904c6541e)


...

💬 stickies-v commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#discussion_r2075528317)
Oh yes, keeping only `self` is a better approach, thanks! Marking as resolved because the commit has been [dropped](https://github.com/bitcoin/bitcoin/pull/32364?notification_referrer_id=NT_kwDOBB0EGbQxNjExMTQxNTE5MDo2OTAxMDQ1Nw#issuecomment-2854656467).

👍 willcl-ark approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2818380798)
ACK 22cff32319de64cb98e1c89b9a7ed35611e89e27

💬 andrewtoth commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2075559595)
Sorry, what is the original cache-usage-update? AFAICT this line was added in this PR, and there is no cache-usage-update here?

💬 laanwj commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#discussion_r2075574236)
First thought this change was a bit strange, but "lose all your bitcoin" singular might make more sense even in English?

💬 McBeThC commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854739598)
Concept NACK