🤔 vasild reviewed a pull request: "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)"
(https://github.com/bitcoin-core/gui/pull/836#pullrequestreview-2309063950)
The changes to `src/qt/optionsmodel.cpp` look ok to me.
(https://github.com/bitcoin-core/gui/pull/836#pullrequestreview-2309063950)
The changes to `src/qt/optionsmodel.cpp` look ok to me.
💬 vasild commented on pull request "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)":
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1762813848)
I think that we are not supposed to access the global state from here, @hebasto?
Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?
Since we are here, I have been thinking that those text descriptions are confusing:
"Used for reaching peers via [ ] IPv4 [ ] IPv6 [ ] Tor"
"Use separte SOCKS5 proxy to reach peers via Tor onion services".
It is not that we reach peers **vi
...
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1762813848)
I think that we are not supposed to access the global state from here, @hebasto?
Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?
Since we are here, I have been thinking that those text descriptions are confusing:
"Used for reaching peers via [ ] IPv4 [ ] IPv6 [ ] Tor"
"Use separte SOCKS5 proxy to reach peers via Tor onion services".
It is not that we reach peers **vi
...
👍 theStack approved a pull request: "scripted-diff: Modernize nLocalServices naming"
(https://github.com/bitcoin/bitcoin/pull/30885#pullrequestreview-2309344094)
ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
(https://github.com/bitcoin/bitcoin/pull/30885#pullrequestreview-2309344094)
ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
💬 pablomartin4btc commented on pull request "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)":
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1763010650)
> Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?
I get it, that's correct, but also when `-onlynet` is specified, g_reachable_nets contains only the nets that are specified by each `-onlynet` option (ie so in this case `-onlynet=ipv6`, ipv4 peers won't be reached by the proxy):
https://github.com/bitcoin-core/gui/blob/225718eda89d65a7041c33e1994d3bf7bd71bbdd/src/init.c
...
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1763010650)
> Also, this change is mixing the "reachable nets" with the "which network is accessed via which proxy" which are two different things. Maybe drop this change altogether?
I get it, that's correct, but also when `-onlynet` is specified, g_reachable_nets contains only the nets that are specified by each `-onlynet` option (ie so in this case `-onlynet=ipv6`, ipv4 peers won't be reached by the proxy):
https://github.com/bitcoin-core/gui/blob/225718eda89d65a7041c33e1994d3bf7bd71bbdd/src/init.c
...
💬 nanlour commented on issue "test: WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=32090)":
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2355330277)
This looks like a false positive. ThreadSanitizer incorrectly counts cs_wallet and cs_args as the same lock because they are mapped to the same memory address. I tried adding padding to distribute the memory mapping, as done in commit c551ea17c93f59abb2df2f8a66e381973291a5e3, and it successfully silenced the warning.
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2355330277)
This looks like a false positive. ThreadSanitizer incorrectly counts cs_wallet and cs_args as the same lock because they are mapped to the same memory address. I tried adding padding to distribute the memory mapping, as done in commit c551ea17c93f59abb2df2f8a66e381973291a5e3, and it successfully silenced the warning.
👍 hebasto approved a pull request: "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2309467718)
ACK fab932b4211ea0a4c10e484c08042552f9c58c15.
We were already using an `arm64` CI task before https://github.com/bitcoin/bitcoin/pull/28187.
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2309467718)
ACK fab932b4211ea0a4c10e484c08042552f9c58c15.
We were already using an `arm64` CI task before https://github.com/bitcoin/bitcoin/pull/28187.
👍 stickies-v approved a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2309472462)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb
I verified that all backports are clean, except:
- b329ed739b7311b3b47cae1ef8d576a90e0a36a1 backported from 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac: merge conflict due to a difference in `sha256sum_file` import, trivially resolved
I didn't review each commit in-depth since they're clean, but the changes backported seem sensible.
(https://github.com/bitcoin/bitcoin/pull/30827#pullrequestreview-2309472462)
ACK 06a7df70df30879e0b691d1a252636f703b8cdfb
I verified that all backports are clean, except:
- b329ed739b7311b3b47cae1ef8d576a90e0a36a1 backported from 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac: merge conflict due to a difference in `sha256sum_file` import, trivially resolved
I didn't review each commit in-depth since they're clean, but the changes backported seem sensible.
💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#discussion_r1763085593)
Once such a scenario happens, we can use another call:
```cmake
target_data_sources(test_bitcoin
RAW_FILES
data/some_file_with_another_namespace.raw
RAW_NAMESPACE somewhere::another_namespace
)
```
And I think it is better than (1) specifying `data/some_file_with_another_namespace.raw` to generate a header than (2) adding `${CMAKE_CURRENT_BINARY_DIR}/data/some_file_with_another_namespac.raw.h` to the sources.
(https://github.com/bitcoin/bitcoin/pull/30901#discussion_r1763085593)
Once such a scenario happens, we can use another call:
```cmake
target_data_sources(test_bitcoin
RAW_FILES
data/some_file_with_another_namespace.raw
RAW_NAMESPACE somewhere::another_namespace
)
```
And I think it is better than (1) specifying `data/some_file_with_another_namespace.raw` to generate a header than (2) adding `${CMAKE_CURRENT_BINARY_DIR}/data/some_file_with_another_namespac.raw.h` to the sources.
💬 theStack commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#issuecomment-2355539328)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30849#issuecomment-2355539328)
Concept ACK
👍 willcl-ark approved a pull request: "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2309566365)
ACK fab932b4211ea0a4c10e484c08042552f9c58c15
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2309566365)
ACK fab932b4211ea0a4c10e484c08042552f9c58c15
📝 MichaelXCity opened a pull request: "Bitcoil Project"
(https://github.com/bitcoin/bitcoin/pull/30914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ maflcko closed a pull request: "Bitcoil Project"
(https://github.com/bitcoin/bitcoin/pull/30914)
(https://github.com/bitcoin/bitcoin/pull/30914)
📝 hebasto opened a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915)
Addresses [this](https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939) comment:
> I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
Draft for now, as it is based on https://github.com/bitcoin/bitcoin/pull/30913 and conflicts with https://github.com/bitcoin/bitcoin/pull/30902.
(https://github.com/bitcoin/bitcoin/pull/30915)
Addresses [this](https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939) comment:
> I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
Draft for now, as it is based on https://github.com/bitcoin/bitcoin/pull/30913 and conflicts with https://github.com/bitcoin/bitcoin/pull/30902.
💬 hebasto commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2355556173)
> > ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.
>
> I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
Please consider https://github.com/bitcoin/bitcoin/pull/30915.
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2355556173)
> > ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.
>
> I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
Please consider https://github.com/bitcoin/bitcoin/pull/30915.
📝 fanquake locked a pull request: "Bitcoil Project"
(https://github.com/bitcoin/bitcoin/pull/30914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30914)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
📝 MichaelXCity opened a pull request: "Bitcoil project"
(https://github.com/bitcoin/bitcoin/pull/30916)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30916)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ MichaelXCity closed a pull request: "Bitcoil project"
(https://github.com/bitcoin/bitcoin/pull/30916)
(https://github.com/bitcoin/bitcoin/pull/30916)
📝 fanquake locked a pull request: "Bitcoil project"
(https://github.com/bitcoin/bitcoin/pull/30916)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/30916)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 fanquake commented on pull request "ci: Use clang-19 in msan tasks":
(https://github.com/bitcoin/bitcoin/pull/30639#issuecomment-2355588809)
19.1.0 is now available: https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.0.
(https://github.com/bitcoin/bitcoin/pull/30639#issuecomment-2355588809)
19.1.0 is now available: https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.0.
💬 brunoerg commented on pull request "doc: update Eclipser fuzzing documentation":
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2355624193)
> I think we could even remove eclipser from the docs entirely. As you noted as well, it looks unmaintained and I don't think any of us are actively using it.
Agreed.
(https://github.com/bitcoin/bitcoin/pull/30908#issuecomment-2355624193)
> I think we could even remove eclipser from the docs entirely. As you noted as well, it looks unmaintained and I don't think any of us are actively using it.
Agreed.