Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061332697)
nit: could use `LookupHelper` here (for slightly better readability imho)
```suggestion
std::vector<CPubKey> participant_pubkeys;
LookupHelper(aggregate_pubkeys, pubkey, participant_pubkeys);
return participant_pubkeys;
```
💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061338625)
nit: should also include ` <optional>`, since it's used for the return types below
```suggestion
#include <optional>
#include <vector>
```
💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061339360)
nit: could turn this into a one-liner by using `std::any_of`
💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061339636)
nit: could turn this into a one-liner by using `std::all_of`
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2832298968)
Restored the deserialization validation in `Obfuscation::Unserialize`, we can't assert, but we can safely throw an `std::logic_error`, since during mempool fuzzing https://github.com/bitcoin/bitcoin/blob/master/src/node/mempool_persist.cpp#L141 catches and ignored these errors safely (managed to reproduce it on Linux, not sure why it's not reproducible on Mac).
I've also split out all renamed to a single commit before any other refactor or optimization to simplify the higher-risk changes.
PR i
...
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2832401037)
ACK 0302eefd9a826e1d8e81b3684ff8ebc6de9d4217

Latest push fixed dead code concerns, uses counts instead of find for non-regex cases consistently and uses less encoding rountrips.
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061444289)
Thanks for fixing this!
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061411267)
What's the exact purpose of this, why would someone set `--tmr=x` or `--tmdi=y`?
And `if not tmpdir_arg` is basically the same as the previous loop which inserts to existing location, it just appends and uses a different root folder, right? We could use those as defaults I think, and slice it in, something like:

```suggestion
i, path = next(((i, m[1]) for i, arg in enumerate(parent_args) if (m := re.match(r'--tmpdir=(.+)', arg))),
(len(parent_args), self.opti
...
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061435588)
I haven't fully given up on this :D. It's not very important, just had to find a solution, feel free to ignore.
Since empty strings are also falsy, the 🦭 operator can come to the rescue again.
```suggestion
assert not (msg := '\n'.join(errors)), f"Child test didn't contain (only) expected errors:\n{msg}\n<CHILD OUTPUT BEGIN>\n{output}\n<CHILD OUTPUT END>\n"
```
👍 darosior approved a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355#pullrequestreview-2796264751)
utACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398

Makes sense and code looks good to me.
📝 hebasto opened a pull request: "depends: Fix cross-compiling `qt` package from macOS to Windows"
(https://github.com/bitcoin/bitcoin/pull/32357)
Native packages cannot be used during cross-compiling. However, Qt still unconditionally tries to find them, which causes issues in some cases, such as when [cross-compiling from macOS to Windows](https://github.com/bitcoin/bitcoin/issues/32346).

This PR explicitly disables this unnecessary Qt behaviour.

Fixes https://github.com/bitcoin/bitcoin/issues/32346.

Here is a full workflow on my macOS Sequoia 15.4.1 (Intel):
```
% brew install make cmake ninja mingw-w64 nsis
% gmake -C depen
...
💬 hebasto commented on pull request "cmake: Do not override flags set by the user when replacing them":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2832504310)
The commit message has been amended.
💬 hebasto commented on issue "test: different error message fails rpc_signer.py":
(https://github.com/bitcoin/bitcoin/issues/31506#issuecomment-2832568646)
I'm struggling to reproduce the 'execve failed: Not a directory (20)' error message on Ubuntu 24.04:

```
$ bitcoind -daemon -signet -signer=fake.py
Bitcoin Core starting
$ bitcoin-cli -signet enumeratesigners
error code: -1
error message:
execve failed: No such file or directory (2)
```
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061570932)
Thanks! Took a variant of that, while still maintaining support for shortened forms of `--tmpdir`. The Python `ArgumentParser` supports doing that and I've personally used `--time=2` in place of `--timeout-factor=2` since it's shorter and I don't remember if it's `--timeout_factor`.
💬 hebasto commented on issue "build: CMake caching failing PIE check":
(https://github.com/bitcoin/bitcoin/issues/32323#issuecomment-2832590273)
Quoting from https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2801696317:

> ... a recommended debugging step is to discard any cached results from an existing build directory...

And in this particular case, I don't think that CMake cache is worth reusing.
💬 hebasto commented on pull request "depends: Fix cross-compiling `qt` package from macOS to Windows":
(https://github.com/bitcoin/bitcoin/pull/32357#issuecomment-2832596900)
My Guix build:
```
aarch64
8ba76371ec4b297da8d29674c7ec31a01eead8131a8e0bbf9bc7e66fb4e1adff guix-build-35e57fbe336c/output/aarch64-linux-gnu/SHA256SUMS.part
d16137ebd3ca5eabb9614a6d4c322e05c43727621d1b8eca98ef4321c5964f21 guix-build-35e57fbe336c/output/aarch64-linux-gnu/bitcoin-35e57fbe336c-aarch64-linux-gnu-debug.tar.gz
3f0078d7df0a54497a52541f2252a52d5b50737b96b83f88d8f2aecd76b54cca guix-build-35e57fbe336c/output/aarch64-linux-gnu/bitcoin-35e57fbe336c-aarch64-linux-gnu.tar.gz
778792dc
...
💬 abdallh12345 commented on pull request "depends: Fix cross-compiling `qt` package from macOS to Windows":
(https://github.com/bitcoin/bitcoin/pull/32357#issuecomment-2832602105)
.
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061611643)
But this doesn't just match prefixes, it makes every parameter optional, so the middle one can be missing - which would be weird.

Maybe you meant `--tm(?:p(?:d(?:ir?)?)?)?=` which would only match the prefixes
* `--tmpdir=`
* `--tmpdi=`
* `--tmpd=`
* `--tmp=`
* `--tm=`

but the intermediary letters aren't optional, so
* `--tmr=`
won't be matched (and the groups are non-capturing so you can still use `m[1]` for the value)
💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2832616772)
ACK b19439b1eee57041fc37dcf509d5ee9f248263bc