Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 hodlinator commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1948755987)
nit: Could add a warning when skipping the test because of user permissions:
```suggestion
if platform.system() != 'Windows':
if os.geteuid() == 0:
self.log.warning('Skipping "permission denied"-test requiring non-root user.')
else:
```

(Ran into something similar on my own branch as sanitizer-tests apparently run with root permissions ([CI log](https://github.com/hodlinator/bitcoin/actions/runs/13219450078/job/36902673995#step:7:5018))).
🚀 fanquake merged a pull request: "TxOrphanage: account for size of orphans and count announcements"
(https://github.com/bitcoin/bitcoin/pull/31810)
💬 hebasto commented on pull request "guix: remove test-security/symbol-check scripts":
(https://github.com/bitcoin/bitcoin/pull/31818#discussion_r1948766776)
The block above, which sets the `exe_format` variable, can now be deleted entirely.
💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948784579)
Certainly won't insist, but another possibility is to use `bit_cast` for trivially constructible types:
```c++
void f(std::span<uint8_t> s);
...
std::span<std::byte> a;
f(std::bit_cast<std::span<uint8_t>>(a));
```
💬 fanquake commented on pull request "guix: remove test-security/symbol-check scripts":
(https://github.com/bitcoin/bitcoin/pull/31818#discussion_r1948786940)
Dropped.
fanquake closed a pull request: "build: fix MSVC ccache reporting"
(https://github.com/bitcoin/bitcoin/pull/31813)
💬 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
embetrix closed an issue: "Wallets not automatically loaded"
(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.
🚀 fanquake merged a pull request: "ci: Use clang-20 for sanitizer tasks"
(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
...
💬 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.
👋 fanquake's pull request is ready for review: "[27.x] More backports"
(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.
💬 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!
💬 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.
💬 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").
💬 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.
💬 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).
👍 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