✅ hebasto closed a pull request: "build: Add CMake-based build system (1 of N)"
(https://github.com/bitcoin/bitcoin/pull/27060)
(https://github.com/bitcoin/bitcoin/pull/27060)
🚀 fanquake merged a pull request: "guix: consolidate to glibc 2.27 for Linux builds"
(https://github.com/bitcoin/bitcoin/pull/27029)
(https://github.com/bitcoin/bitcoin/pull/27029)
💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109621939)
I missed that, changing to `h`.
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109621939)
I missed that, changing to `h`.
📝 vasild opened a pull request: "doc: clarify that LOCK() does AssertLockNotHeld() internally"
(https://github.com/bitcoin/bitcoin/pull/27116)
Constructs like
```cpp
AssertLockNotHeld(m);
LOCK(m);
```
are equivalent to
```cpp
LOCK(m);
```
for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case.
Also remove `
...
(https://github.com/bitcoin/bitcoin/pull/27116)
Constructs like
```cpp
AssertLockNotHeld(m);
LOCK(m);
```
are equivalent to
```cpp
LOCK(m);
```
for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case.
Also remove `
...
💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1109626265)
Opened https://github.com/bitcoin/bitcoin/pull/27116 to amend the docs.
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1109626265)
Opened https://github.com/bitcoin/bitcoin/pull/27116 to amend the docs.
💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1434484944)
Rebased past #27029. Might split some more of this out.
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1434484944)
Rebased past #27029. Might split some more of this out.
💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1109650266)
Hmm looks like this checks for duplicate keys on every single node created whereas we could just check that on the top-level node in `GenNode()`?
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1109650266)
Hmm looks like this checks for duplicate keys on every single node created whereas we could just check that on the top-level node in `GenNode()`?
💬 fanquake commented on pull request "[POC] guix: produce a fully -static-pie x86_64 bitcoind using GCC and glibc":
(https://github.com/bitcoin/bitcoin/pull/25573#issuecomment-1434503043)
Rebased past #27029.
(https://github.com/bitcoin/bitcoin/pull/25573#issuecomment-1434503043)
Rebased past #27029.
💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109664449)
Indeed, dropping these and a few others.
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109664449)
Indeed, dropping these and a few others.
💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109676251)
Not sure. I think I'd rather have an error message for mixed use of `h` and `'`.
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1109676251)
Not sure. I think I'd rather have an error message for mixed use of `h` and `'`.
💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1434522252)
Rebased, added the suggested fuzzer coverage and fixed some `'` that I missed.
> What was the rationale for only normalizing the public-key version with h?
I was thinking along the lines that normalization (moving the xpub to a logical position, converting `'` -> `h`) was an opt-in thing that we only do with the public-key version. However it's pretty much the default in practice, e.g. `listdescriptors` normalizes. We might as well do it for the private key version. The private key versio
...
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1434522252)
Rebased, added the suggested fuzzer coverage and fixed some `'` that I missed.
> What was the rationale for only normalizing the public-key version with h?
I was thinking along the lines that normalization (moving the xpub to a logical position, converting `'` -> `h`) was an opt-in thing that we only do with the public-key version. However it's pretty much the default in practice, e.g. `listdescriptors` normalizes. We might as well do it for the private key version. The private key versio
...
📝 darosior opened a pull request: "fuzz: avoid redundant dup key checks when creating Miniscript nodes"
(https://github.com/bitcoin/bitcoin/pull/27117)
I thought i had done that already in #24149, but it must have slipped through the rebase. It's a 2x speed improvement against the existing corpora and will probably be much more as we extend them with larger nodes.
(https://github.com/bitcoin/bitcoin/pull/27117)
I thought i had done that already in #24149, but it must have slipped through the rebase. It's a 2x speed improvement against the existing corpora and will probably be much more as we extend them with larger nodes.
💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1434523714)
Oops, didn't rebase far enough to get past the merge conflict...
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1434523714)
Oops, didn't rebase far enough to get past the merge conflict...
💬 darosior commented on pull request "fuzz: avoid redundant dup key checks when creating Miniscript nodes":
(https://github.com/bitcoin/bitcoin/pull/27117#issuecomment-1434523841)
cc @sipa
(https://github.com/bitcoin/bitcoin/pull/27117#issuecomment-1434523841)
cc @sipa
💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1109682908)
https://github.com/bitcoin/bitcoin/pull/27117
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1109682908)
https://github.com/bitcoin/bitcoin/pull/27117
💬 hebasto commented on pull request "build: Propagate user-defined tools to native packages":
(https://github.com/bitcoin/bitcoin/pull/23571#issuecomment-1434524289)
Closing. Up for grabs.
(https://github.com/bitcoin/bitcoin/pull/23571#issuecomment-1434524289)
Closing. Up for grabs.
✅ hebasto closed a pull request: "build: Propagate user-defined tools to native packages"
(https://github.com/bitcoin/bitcoin/pull/23571)
(https://github.com/bitcoin/bitcoin/pull/23571)
💬 ajtowns commented on pull request "doc: clarify that LOCK() does AssertLockNotHeld() internally":
(https://github.com/bitcoin/bitcoin/pull/27116#issuecomment-1434539481)
These aren't precisely the same (maybe they should be?). With `AssertLockNotHeld` you just get a report on that lock, on stderr:
```
Assertion failed: lock flock held in net.cpp:1398; locks held:
'flock' in net.cpp:1404 (in thread 'dnsseed')
```
with a bare `LOCK()` you get a less useful report on stderr (it only shows the location of the second lock attempt, not where it's being held):
```
Assertion failed: detected double lock for 'flock' in net.cpp:1399 (in thread 'dnsseed'), det
...
(https://github.com/bitcoin/bitcoin/pull/27116#issuecomment-1434539481)
These aren't precisely the same (maybe they should be?). With `AssertLockNotHeld` you just get a report on that lock, on stderr:
```
Assertion failed: lock flock held in net.cpp:1398; locks held:
'flock' in net.cpp:1404 (in thread 'dnsseed')
```
with a bare `LOCK()` you get a less useful report on stderr (it only shows the location of the second lock attempt, not where it's being held):
```
Assertion failed: detected double lock for 'flock' in net.cpp:1399 (in thread 'dnsseed'), det
...
💬 Sjors commented on pull request "contrib: remove install_db4.sh":
(https://github.com/bitcoin/bitcoin/pull/26834#issuecomment-1434541595)
Post merge: just noticed the script was gone, but the bdb-only-depends approach works.
(https://github.com/bitcoin/bitcoin/pull/26834#issuecomment-1434541595)
Post merge: just noticed the script was gone, but the bdb-only-depends approach works.
📝 fanquake opened a pull request: "depends: harden libevent"
(https://github.com/bitcoin/bitcoin/pull/27118)
Turn on libevents own hardening option (`--enable-gcc-hardneing`, although the options are not GCC specific), and backport a commit I upstreamed (https://github.com/libevent/libevent/pull/1418) to use `FORTIFY_SOURCE=3` over 2.
Solves half of #27038, by giving us some fortified funcs in `bitcoin-cli`.
Also elated to #27027.
(https://github.com/bitcoin/bitcoin/pull/27118)
Turn on libevents own hardening option (`--enable-gcc-hardneing`, although the options are not GCC specific), and backport a commit I upstreamed (https://github.com/libevent/libevent/pull/1418) to use `FORTIFY_SOURCE=3` over 2.
Solves half of #27038, by giving us some fortified funcs in `bitcoin-cli`.
Also elated to #27027.