📝 fanquake opened a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884)
This dates from the introduction of depends, and has not been the case for some time now.

💬 sr-gi commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-1812964820)
There was an issue with the interaction of the change and using a proxy. It should be fixed now

💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1812965773)
@mzumsande I think so, yes, along with the other races mentioned in the commit message https://github.com/bitcoin/bitcoin/pull/28248/commits/21f8426c823fd83c40fd81f08ff2bf39ec6a4575. I'll add the case you describe to it (thanks!)

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812967822)
Removed `LDFLAGS=-Wno-error=unused-command-line-argument` from the CI in the depends bump commit. Also added a commit to remove the no-longer used `pyhton3-setuptools` dep. Also rebsaed on recent merges. Still have to investigate the non-determinism.

💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1812970597)
I'd be interested to hear anyone's thoughts on this, from that commit message:

*Finally, there does not seem to be a reason to make block-relay or short-lived feeler connections to addnode peers, as the addnode logic will ensure we connect to them if they are up, within the addnode connection limit.*

💬 maflcko commented on pull request "doc: remove x86_64 build assumption from depends doc":
(https://github.com/bitcoin/bitcoin/pull/28884#issuecomment-1812987723)
lgtm ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18

💬 jonatack commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1394550037)
CI linter needs appeasing here
```suggestion
```

💬 sr-gi commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1394555196)
I even left some debug comments there. My bad

💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1813004612)
My SDK hash:
```
$ sha256sum Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz
6afb7c24461729167c5f5976ce56e774657a97b052f44f1618e9717da67fddb7 Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz
```
and Guix builds:
```
x86_64
fa1db57217fe8646c529f312ba5af6fee9f3085768d2033359b2f1ec9f1c4624 guix-build-cf8353a9cf9c/output/arm64-apple-darwin/SHA256SUMS.part
e64ee8bf9901e5a37cbb67cb5ed23daaa7d0086b256b64870b26a94d5ac3ee4e guix-build-cf8353a9cf9c/output/arm64-apple-da
...

👍 hebasto approved a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884#pullrequestreview-1732628851)
ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18.

💬 theuni commented on pull request "Remove version field from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/28878#issuecomment-1813031718)
Playing around locally, I was also able to drop the include in:
```bash
$ git diff --stat
src/bitcoin-util.cpp | 1 -
src/coins.cpp | 1 -
src/core_read.cpp | 1 -
src/kernel/coinstats.cpp | 1 -
src/rest.cpp | 1 -
src/script/bitcoinconsensus.cpp | 1 -
src/zmq/zmqpublishnotifier.cpp | 1 -
```

(I didn't mess with the tests, maybe could drop some there too).

Also worth pointing out: libbitcoinkernel now only r
...

💬 brunoerg commented on pull request "fuzz: Delete wallet_notifications":
(https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1813037774)
> Can you add steps to reproduce?

The way it creates the wallet, calls `SetupDescriptorScriptPubKeyMans` without passing a key, it means that the function will create it internally.

```cpp
// Make a seed
CKey seed_key;
seed_key.MakeNewKey(true);
CPubKey seed = seed_key.GetPubKey();
assert(seed_key.VerifyPubKey(seed));

// Get the extended key
CExtKey master_key;
master_key.SetSeed(seed_key);

SetupDescriptorScriptPubKeyMans(master_key);
```

💬 TheCharlatan commented on pull request "doc: remove mention of missing bdb being a configure error":
(https://github.com/bitcoin/bitcoin/pull/28881#issuecomment-1813066650)
ACK 30bd4b1e4aee00edbe77da7c20bf80e28f0561cc

💬 brandonblack commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1813094549)
In FROST

> > > Is it possible to support descriptors that include only the "aggregated xpub" and not every key as in the proposed `agg()` expression, leading to a O(1) size instead of O(number-of-keys)?
> >
> >
> > You should be able to use Shamir secret sharing between the private key data so that a spending threshold can recover all pubkey data needed to reconstruct the agg() expression.
>
> As I understand it, the `agg()` expression requires all `n` public keys. I don't see how a t
...

💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#discussion_r1394675893)
After adding unit tests, this logic seems needed not in `GetAddedNodeInfo()` but only in `AddNode()`, where it is simpler and may be sufficient to compare by `CNetAddr` for cjdns entries only, in order to handle them correctly.

```diff
bool CConnman::AddNode(const AddedNodeParams& add)
{
- const CService resolved(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_added_node)));
+ const CService resolved{MaybeFlipIPv6toCJDNS(LookupNumeric(add.m_added_node, GetDefaultPort(add.m_ad
...

💬 mzumsande commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1813126001)
> Maybe it would be prudent to allow feelers to addnodes.

I think it's fine to skip feelers for addnode peers. When we would skip these addresses for automatic connection, I don't see what making a feeler connection to them would achieve, considering that the point of feelers is to have better options for future automatic connections.

💬 whitslack commented on pull request "policy: Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1813162870)
What's the point of having a non-malleable version? Okay, so a miner cannot add extra pushes to the input script that spends the anchor, but there is nothing to stop the miner from mining an alternative child transaction that spends *only* the anchor. Thus, the party that had broadcast the original child transaction will be forced to construct a new spending transaction with a different TxID anyway. Indeed, anyone (not only a miner) is free to propose a replacement child that spends *only* the a
...

📝 willcl-ark converted_to_draft a pull request: "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes"
(https://github.com/bitcoin/bitcoin/pull/28824)
Closes: #27795
Closes: #7996

Previously `ScriptToAsmStr` returned hex-encoded values, except if data length was <= 4 bytes, in which case it displayed using decimal encoding.

Remove the decimal encoding carve-out for small pushes and always display hex results from `decodescript` RPC.

There were [other suggestions](https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1692870456) for approaches in #27795, but IMO moving to hex-only-always makes the most sense.

👍 theuni approved a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884#pullrequestreview-1732859147)
ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18

👋 denavila's pull request is ready for review: "wallet: Deniability API (Unilateral Transaction Meta-Privacy)"
(https://github.com/bitcoin/bitcoin/pull/27792)