💬 maflcko commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075447740)
> Should I just be stopping the node myself with the optional arg?

I'd say yes, if you go down the route. Not sure about adding implicit fields to the test framework for this.

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2075449394)
note: `-bind` was added here. I guess before it was omitted because this loop did not support `=whatever` suffix. Now it does.

🤔 janb84 reviewed a pull request: "cmake: Respect user-provided configuration-specific flags"
(https://github.com/bitcoin/bitcoin/pull/32356#pullrequestreview-2818239704)
ACK [edde963](https://github.com/bitcoin/bitcoin/commit/edde96376a2961dec3730331b3d171ddf972589f)

💬 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