💬 embetrix commented on issue "Wallets not automatically loaded":
(https://github.com/bitcoin/bitcoin/issues/31832#issuecomment-2647555660)
thanks for the suggestions:
the following command:
`bitcoin-cli loadwallet "Wallet-01" true`
make a wallet automatically loaded
(https://github.com/bitcoin/bitcoin/issues/31832#issuecomment-2647555660)
thanks for the suggestions:
the following command:
`bitcoin-cli loadwallet "Wallet-01" true`
make a wallet automatically loaded
✅ embetrix closed an issue: "Wallets not automatically loaded"
(https://github.com/bitcoin/bitcoin/issues/31832)
(https://github.com/bitcoin/bitcoin/issues/31832)
👍 fanquake approved a pull request: "ci: Use clang-20 for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/31793#pullrequestreview-2605356354)
ACK fa5a02bcfa2f3769859332fddb8954d6217de7fc - tested 20 in some other infra, we just needed to fix the same deprecation warnings we'd seen, in cryptofuzz: https://github.com/fanquake/cryptofuzz/commit/09ca550c3ef7d6645869cb1ac014a4413d5388ba.
(https://github.com/bitcoin/bitcoin/pull/31793#pullrequestreview-2605356354)
ACK fa5a02bcfa2f3769859332fddb8954d6217de7fc - tested 20 in some other infra, we just needed to fix the same deprecation warnings we'd seen, in cryptofuzz: https://github.com/fanquake/cryptofuzz/commit/09ca550c3ef7d6645869cb1ac014a4413d5388ba.
🚀 fanquake merged a pull request: "ci: Use clang-20 for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/31793)
(https://github.com/bitcoin/bitcoin/pull/31793)
💬 maflcko commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948815820)
> `bit_cast`
While this will likely work in practise, I don't think there is any inherent guarantee that the layout of span is identical for all underlying types. So my recommendation would be to just use `std::as_bytes` instead.
(Same for the reinterpret_cast: While it works, it is a bit verbose and `std::as_bytes` is the existing alias in the std lib, which is already used in the code today.)
Obviously, anything is fine. I just left a comment to say it is possible to convert :sweat_sm
...
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948815820)
> `bit_cast`
While this will likely work in practise, I don't think there is any inherent guarantee that the layout of span is identical for all underlying types. So my recommendation would be to just use `std::as_bytes` instead.
(Same for the reinterpret_cast: While it works, it is a bit verbose and `std::as_bytes` is the existing alias in the std lib, which is already used in the code today.)
Obviously, anything is fine. I just left a comment to say it is possible to convert :sweat_sm
...
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948830315)
Will leave it as `uint8_t` because it is already used in `master` in `ReceiveMsgBytes()` to which we have to pass that variable.
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948830315)
Will leave it as `uint8_t` because it is already used in `master` in `ReceiveMsgBytes()` to which we have to pass that variable.
👋 fanquake's pull request is ready for review: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/31422)
(https://github.com/bitcoin/bitcoin/pull/31422)
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1948835528)
Yes. Flow will not go after the assert.
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1948835528)
Yes. Flow will not go after the assert.
💬 stickies-v commented on pull request "cmake: Install man pages for configured targets only":
(https://github.com/bitcoin/bitcoin/pull/31765#discussion_r1948838287)
> Edit: actually, I guess we need to actually set components for the other bins first. I'll work on a PR for that.
I can no longer reproduce the output I provided [earlier](https://github.com/bitcoin/bitcoin/pull/31765#discussion_r1937696640), so I think I must have done something wrong while testing it. You're right, it seems the components must be specified with this patch, so @hebasto's new approach with `${ARGN}` seems preferable. Sorry for the confusion!
(https://github.com/bitcoin/bitcoin/pull/31765#discussion_r1948838287)
> Edit: actually, I guess we need to actually set components for the other bins first. I'll work on a PR for that.
I can no longer reproduce the output I provided [earlier](https://github.com/bitcoin/bitcoin/pull/31765#discussion_r1937696640), so I think I must have done something wrong while testing it. You're right, it seems the components must be specified with this patch, so @hebasto's new approach with `${ARGN}` seems preferable. Sorry for the confusion!
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1948846003)
It is still there in the `.h` file.
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1948846003)
It is still there in the `.h` file.
💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948853767)
(Might be worth noting that in commit message for the change that fixes the bug, 60f6cbb9b9f83e25217d30c889147ad517960ec7 / "net: index nodes in CConnman by id").
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948853767)
(Might be worth noting that in commit message for the change that fixes the bug, 60f6cbb9b9f83e25217d30c889147ad517960ec7 / "net: index nodes in CConnman by id").
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2647664569)
Will need a rebase due to #31793, which might even catch new things.
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2647664569)
Will need a rebase due to #31793, which might even catch new things.
💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948868714)
Thanks! (PR summary is out of date, but I understand if you don't want to constantly manually update it).
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948868714)
Thanks! (PR summary is out of date, but I understand if you don't want to constantly manually update it).
👍 laanwj approved a pull request: "depends: Make default `host` and `build` comparable"
(https://github.com/bitcoin/bitcoin/pull/30584#pullrequestreview-2605445586)
Concept and code review ACK b28917be363fb5a82effffeadbe4ba27bb1c70ce
(https://github.com/bitcoin/bitcoin/pull/30584#pullrequestreview-2605445586)
Concept and code review ACK b28917be363fb5a82effffeadbe4ba27bb1c70ce
✅ marcofleon closed a pull request: "refactor: ensure type safety for txid and wtxid in `RelayTransaction`"
(https://github.com/bitcoin/bitcoin/pull/31001)
(https://github.com/bitcoin/bitcoin/pull/31001)
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948876683)
I am considering this, but just realized that the distinction is not "proxy vs no-proxy" because in both cases of the `std::variant` we may end up connecting via the proxy. The logic (as convoluted as it is, it is the same in `master`) is:
* if connecting to `CService`
* if to an I2P `CService` then proxy must be used
* otherwise the proxy is optional, if provided it will be used
* if connecting to string host, then proxy must be used
So, if `std::variant` is to be avoided and two f
...
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948876683)
I am considering this, but just realized that the distinction is not "proxy vs no-proxy" because in both cases of the `std::variant` we may end up connecting via the proxy. The logic (as convoluted as it is, it is the same in `master`) is:
* if connecting to `CService`
* if to an I2P `CService` then proxy must be used
* otherwise the proxy is optional, if provided it will be used
* if connecting to string host, then proxy must be used
So, if `std::variant` is to be avoided and two f
...
💬 hebasto commented on pull request "build: set build type and per-build-type flags as early as possible":
(https://github.com/bitcoin/bitcoin/pull/31711#issuecomment-2647711577)
I'd like to summarize here a few observations regarding CMake behaviour.
1. Single-config generators, such as "Ninja":
- `CMAKE_BUILD_TYPE` is defined in `enable_language()`.
- Compiler checks in `enable_language()` ignore any configuration, unless `CMAKE_TRY_COMPILE_CONFIGURATION` is set beforehand.
2. Single-config generators, such as "Ninja Multi-Config":
- `CMAKE_CONFIGURATION_TYPES` is defined in `project()`.
- Compiler checks in `enable_language()` use the "Debug" configuration (
...
(https://github.com/bitcoin/bitcoin/pull/31711#issuecomment-2647711577)
I'd like to summarize here a few observations regarding CMake behaviour.
1. Single-config generators, such as "Ninja":
- `CMAKE_BUILD_TYPE` is defined in `enable_language()`.
- Compiler checks in `enable_language()` ignore any configuration, unless `CMAKE_TRY_COMPILE_CONFIGURATION` is set beforehand.
2. Single-config generators, such as "Ninja Multi-Config":
- `CMAKE_CONFIGURATION_TYPES` is defined in `project()`.
- Compiler checks in `enable_language()` use the "Debug" configuration (
...
👍 stickies-v approved a pull request: "cmake: Install man pages for configured targets only"
(https://github.com/bitcoin/bitcoin/pull/31765#pullrequestreview-2605507226)
tACK c783833f7c74bb184a74f3813b0bb973ea0b9e11
(https://github.com/bitcoin/bitcoin/pull/31765#pullrequestreview-2605507226)
tACK c783833f7c74bb184a74f3813b0bb973ea0b9e11
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948908109)
Done.
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948908109)
Done.
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647781404)
`7d84f431f9...7b63b4ca1c`: minor repush to elaborate a commit message as suggested.
Are you in the mood of reviewing a change to this PR, https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952, that stores the "client" objects in `SockMan` (as `shared_ptr`, not as raw pointers)? In the case of `CConnman` that is `CNode`. This will:
* make the interaction between `SockMan` and `CConnman` simpler
* probably faster (less lookups by id)
* resolve the issue in https://github.com/
...
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647781404)
`7d84f431f9...7b63b4ca1c`: minor repush to elaborate a commit message as suggested.
Are you in the mood of reviewing a change to this PR, https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2485715952, that stores the "client" objects in `SockMan` (as `shared_ptr`, not as raw pointers)? In the case of `CConnman` that is `CNode`. This will:
* make the interaction between `SockMan` and `CConnman` simpler
* probably faster (less lookups by id)
* resolve the issue in https://github.com/
...