π¬ laanwj commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#issuecomment-2854429982)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32404#issuecomment-2854429982)
Concept ACK
π¬ maflcko commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2854432627)
Does it work with 28.x? Could add 29.x to the title?
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2854432627)
Does it work with 28.x? Could add 29.x to the title?
π¬ hebasto commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075396465)
> Would it be possible to put `get_directory_property(precious_variables CACHE_VARIABLES)` here inside the function scope?
If you do that, it will collect additional cache variables set by subsequent commands such as `project()` and `enable_language()`. This list will include `CMAKE_CXX_FLAGS_RELEASE`, which breaks the patch.
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075396465)
> Would it be possible to put `get_directory_property(precious_variables CACHE_VARIABLES)` here inside the function scope?
If you do that, it will collect additional cache variables set by subsequent commands such as `project()` and `enable_language()`. This list will include `CMAKE_CXX_FLAGS_RELEASE`, which breaks the patch.
π¬ hebasto commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075399096)
Thanks! Updated.
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075399096)
Thanks! Updated.
π¬ hebasto commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2854447614)
The [feedback](https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2074117354) from @purpleKarrot has been addressed.
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2854447614)
The [feedback](https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2074117354) from @purpleKarrot has been addressed.
π¬ fanquake commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2854449493)
> Does it work with 28.x?
Yea. This is a CMake regression.
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2854449493)
> Does it work with 28.x?
Yea. This is a CMake regression.
π¬ hebasto commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075409377)
The `var_name` contains no spaces, so I'll leave it as is to avoid cluttering the code.
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075409377)
The `var_name` contains no spaces, so I'll leave it as is to avoid cluttering the code.
π¬ hebasto commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075409656)
The `var_name` contains no spaces, so I'll leave it as is to avoid cluttering the code."
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075409656)
The `var_name` contains no spaces, so I'll leave it as is to avoid cluttering the code."
π¬ hebasto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2854518383)
> I don't know if cross-compilation is a priority for QT?
As far as I know, the required toolchain isnβt even provided by major distributions.
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-2854518383)
> I don't know if cross-compilation is a priority for QT?
As far as I know, the required toolchain isnβt even provided by major distributions.
π vasild opened a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425)
`-proxy=addr:port` specifies the proxy for all networks (except I2P). Previously only the Tor proxy could have been specified separately via `-onion=addr:port`.
Make it possible to specify separately the proxy for IPv4, IPv6, Tor and CJDNS by e.g. `-proxy=addr:port=ipv6`. Or remove the proxy for a given network, e.g. `-proxy=0=cjdns`.
Resolves: https://github.com/bitcoin/bitcoin/issues/24450
(https://github.com/bitcoin/bitcoin/pull/32425)
`-proxy=addr:port` specifies the proxy for all networks (except I2P). Previously only the Tor proxy could have been specified separately via `-onion=addr:port`.
Make it possible to specify separately the proxy for IPv4, IPv6, Tor and CJDNS by e.g. `-proxy=addr:port=ipv6`. Or remove the proxy for a given network, e.g. `-proxy=0=cjdns`.
Resolves: https://github.com/bitcoin/bitcoin/issues/24450
π¬ maflcko commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075446014)
aggregateare -> aggregate are
(missing space, from the DrahtBot LLM linter)
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2075446014)
aggregateare -> aggregate are
(missing space, from the DrahtBot LLM linter)
π¬ 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.
(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.
(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)
(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,
...
(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#
...
(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
...
(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
...
(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
(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.
...
(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.
...