💬 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.

💬 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.

💬 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()`?

💬 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.

💬 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.

💬 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 `'`.

💬 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
...

📝 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.

💬 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...

💬 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

💬 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

💬 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.

✅ hebasto closed a pull request: "build: Propagate user-defined tools to native packages"
(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
...

💬 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.

📝 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.

🤔 vasild requested changes to a pull request: "p2p: return `CSubNet` in `LookupSubNet`"
(https://github.com/bitcoin/bitcoin/pull/26078)

💬 vasild commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1109707211)
nit: maybe add `assert(!subnet.IsValid());` after `CSubNet subnet;` to make it obvious that we return an invalid object and to guard against future changes that construct a valid `CSubNet` by default.

💬 vasild commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1109701750)
nit, here and elsewhere:

```suggestion
const CSubNet subnet = LookupSubNet(strAllow);
```

💬 vasild commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1109711519)
Hmm, `possibleSubnet.has_value()` will always be `true`. I guess this is an artifact from a previous incarnation of this PR that used `std::optional`.

💬 vasild commented on pull request "p2p: return `CSubNet` in `LookupSubNet`":
(https://github.com/bitcoin/bitcoin/pull/26078#discussion_r1109752206)
Now `ResolveSubNet()` is identical to `LookupSubNet()`. Maybe ditch the former and change the callers to use directly `LookupSubNet()`?