💬 janb84 commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2387475262)
This comment line, is this intended as a reminder to remove this module if the code is added upstream ?
if so, Nit: please make the intent more explicit. e.g
```suggestion
#TODO remove file if fixed upstream:
# https://gitlab.kitware.com/cmake/cmake/-/issues/26920
```
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2387475262)
This comment line, is this intended as a reminder to remove this module if the code is added upstream ?
if so, Nit: please make the intent more explicit. e.g
```suggestion
#TODO remove file if fixed upstream:
# https://gitlab.kitware.com/cmake/cmake/-/issues/26920
```
🤔 janb84 reviewed a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279090957)
reACK 444409ff2b78d8f3e541bd6e883af8da7adfd264
changes since last ACK:
- changed container size to md. CI still works with this size. ✅
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279090957)
reACK 444409ff2b78d8f3e541bd6e883af8da7adfd264
changes since last ACK:
- changed container size to md. CI still works with this size. ✅
👍 willcl-ark approved a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279091535)
ACK 444409ff2b78d8f3e541bd6e883af8da7adfd264
This took 29 minutes for an alpine depends, gui build, with no docker/ccache/depends caches (as no merges to master yet) which seems in line with other jobs.
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279091535)
ACK 444409ff2b78d8f3e541bd6e883af8da7adfd264
This took 29 minutes for an alpine depends, gui build, with no docker/ccache/depends caches (as no merges to master yet) which seems in line with other jobs.
📝 AmelkaDzikwk opened a pull request: "Create based point of view"
(https://github.com/bitcoin/bitcoin/pull/33490)
#2
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/33490)
#2
<!--
*** 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
...
💬 AmelkaDzikwk commented on pull request "Create based point of view":
(https://github.com/bitcoin/bitcoin/pull/33490#issuecomment-3346308216)
not sure what does it mean?
(https://github.com/bitcoin/bitcoin/pull/33490#issuecomment-3346308216)
not sure what does it mean?
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346353073)
> Thanks for the rebase :)
>
> I updated darosior's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:
Unfortunately, I don't think this is a meaningful backtrace, since it is just showing the trace of a thread at the time it receives a SIGPIPE signal (`Thread 3 "b-capnp-loop" receiv
...
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346353073)
> Thanks for the rebase :)
>
> I updated darosior's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:
Unfortunately, I don't think this is a meaningful backtrace, since it is just showing the trace of a thread at the time it receives a SIGPIPE signal (`Thread 3 "b-capnp-loop" receiv
...
💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346397449)
Err, right. Updated the description with your suggested trace filtering.
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346397449)
Err, right. Updated the description with your suggested trace filtering.
👍 TheCharlatan approved a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3279259103)
ACK
0465574c127907df9b764055a585e8281bae8d1d
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3279259103)
ACK
0465574c127907df9b764055a585e8281bae8d1d
💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3346459809)
Friendly ping @laanwj :)
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3346459809)
Friendly ping @laanwj :)
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2387660586)
There were previous discussions about the names of these methods: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2054679774 and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916
I now renamed the method discussed above, so all 3 are changed from:
```cpp
// previous version of the PR
AlreadyConnectedToAddressPort(const std::string& addr_port);
AlreadyConnectedToAddressPort(const CService& addr_port);
AlreadyConnectedToAddress(const CNetAddr& addr);
```
to
...
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2387660586)
There were previous discussions about the names of these methods: https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2054679774 and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2072443916
I now renamed the method discussed above, so all 3 are changed from:
```cpp
// previous version of the PR
AlreadyConnectedToAddressPort(const std::string& addr_port);
AlreadyConnectedToAddressPort(const CService& addr_port);
AlreadyConnectedToAddress(const CNetAddr& addr);
```
to
...
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-3346520156)
`8668b0c64d...c9ffb6d710`: rebase and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372244246
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-3346520156)
`8668b0c64d...c9ffb6d710`: rebase and https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372244246
📝 AureliuszNada opened a pull request: "Create promise"
(https://github.com/bitcoin/bitcoin/pull/33491)
111
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/33491)
111
<!--
*** 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
...
💬 AureliuszNada commented on pull request "Create promise":
(https://github.com/bitcoin/bitcoin/pull/33491#issuecomment-3346537768)
!!! fix
(https://github.com/bitcoin/bitcoin/pull/33491#issuecomment-3346537768)
!!! fix
📝 AureliuszNada opened a pull request: "Create no promises"
(https://github.com/bitcoin/bitcoin/pull/33492)
111
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/33492)
111
<!--
*** 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
...
💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3346569174)
`ec7c215345...82e46a7248`: rebase to hopefully fix CI and elaborate the return value of ``OpenNetworkConnection()` in the commit message.
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3346569174)
`ec7c215345...82e46a7248`: rebase to hopefully fix CI and elaborate the return value of ``OpenNetworkConnection()` in the commit message.
💬 maflcko commented on pull request "test: Run bench sanity checks in parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33142#issuecomment-3346604822)
rebased after https://github.com/bitcoin/bitcoin/pull/33313 (queue refactor)
(https://github.com/bitcoin/bitcoin/pull/33142#issuecomment-3346604822)
rebased after https://github.com/bitcoin/bitcoin/pull/33313 (queue refactor)
👍 hodlinator approved a pull request: "ci: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3279511439)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3279511439)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f
💬 hodlinator commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387762653)
I guess the amount of Mac-specific code we have that behaves differently between 14 and 15 is minimal.
If it hasn't been a problem previously I'm okay with bumping it so we don't need to re-touch in 1 year - Although if we are going to keep bumping the minimum supported version and CI version every year anyway... I would lean towards keeping them in sync. (GitHub deprecating and removing older versions would then also serve as an extra reminder if we are late to update).
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387762653)
I guess the amount of Mac-specific code we have that behaves differently between 14 and 15 is minimal.
If it hasn't been a problem previously I'm okay with bumping it so we don't need to re-touch in 1 year - Although if we are going to keep bumping the minimum supported version and CI version every year anyway... I would lean towards keeping them in sync. (GitHub deprecating and removing older versions would then also serve as an extra reminder if we are late to update).
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-3346773372)
`0cd86a5c70...8218daac0f`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-3346773372)
`0cd86a5c70...8218daac0f`: rebase due to conflicts
💬 vasild commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-3346791115)
`632eb1a3c9...a42449cb30`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-3346791115)
`632eb1a3c9...a42449cb30`: rebase due to conflicts
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-3346806725)
`22d4c57a99...9a49877f26`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-3346806725)
`22d4c57a99...9a49877f26`: rebase due to conflicts