💬 fanquake commented on pull request "build: re-enable deprecated warning copy":
(https://github.com/bitcoin/bitcoin/pull/30236#issuecomment-2152016135)
> Yeah, could open a tracking issue in the cmake repo (hebasto/bitcoin)?

Opened one here so it isn't forgotten: https://github.com/hebasto/bitcoin/issues/223.

💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629164462)
```suggestion
// Implements the full legacy wallet behavior
// Slated for deletion once Berkeley DB library dependency is removed.
class LegacyScriptPubKeyMan : public LegacyDataSPKM
```

💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1629293058)
Haven't fully grokked the code yet, so I understand if you'd rather not answer my questions, but if you want:

> I don't think this needs to have any additional explanation. keypool_size only matters for ranged descriptors. Non-HD keys do not make ranged descriptors, so the value here is ignored.

We're in an HD chain `for`-loop, how is this not involving HD keys?
Surely legacy wallets could be HD wallets.

> The loop below is for an entirely different set of descriptors and is unrelated
...

💬 maflcko commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1629294986)
This should also check that the background validation succeeds. Otherwise there could be a bug where the diverging chain is not rewound?

📝 AngusP opened a pull request: "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`"
(https://github.com/bitcoin/bitcoin/pull/30237)
This test uses the `extra_txn` (`vExtraTxnForCompact`) vector of optional orphan/conflicted/etc. transactions to provide transactions to a PartiallyDownloadedBlock that are not otherwise present in the mempool, and check that they are used.

This also covers a former nullptr deref bug that was fixed in #29752 (bf031a517c79cec5b43420bcd40291ab0e9f68a8) where the `extra_txn` vec/circular-buffer was null-initialized and not yet filled when dereferenced in `PartiallyDownloadedBlock::InitData`.

👍 hebasto approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2101695692)
re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4.

My Guix build:
```
x86_64
f33ae90e6d190245d34712f544cd458f282a2c90890029dfbd88d74e8823fb50 guix-build-c7376babd19d/output/aarch64-linux-gnu/SHA256SUMS.part
ee444d54e0b9cebfa6ab0c91fc8b543d414a98338d46d299f3b302273139c81f guix-build-c7376babd19d/output/aarch64-linux-gnu/bitcoin-c7376babd19d-aarch64-linux-gnu-debug.tar.gz
fdc7400c04fd6bfec5d0ab3148b0f4da110974ecda0859184adad29f38422f6f guix-build-c7376babd19d/output/aarch64-linux-gnu/bitc
...

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2152172999)
Hi all,
an early alpha of the Ledger Bitcoin Testnet app with MuSig2 support is available for testing. (NB: the app is called `Bitcoin Test Musig` and not `Bitcoin Test`). It should be compatible with the [latest draft of the specs](https://github.com/bitcoin/bips/pull/1540).

Instructions and an easy end-2-end script for MuSig signing to play with it is available here for anyone interested in trying it out:

https://github.com/bigspider/moosig

It works for both keypath and script path s
...

💬 rocholojavinar commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2152186980)

💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1629407063)
Sure, can be in another PR

🤔 glozow reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2101775625)
rebased

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1629413337)
fixed, thanks

💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2152268436)
@sipa
> Would you mind benchmarking with smaller realistic numbers (say fee and size both between 0 and 2^30)?

I switched to `FastRandomContext::randbits(30)`.

> If the top limb is equal, it's possible the naive code does worse.

Numbers have not changed:
- the master branch:
```
> build_msvc\x64\Release\bench_bitcoin.exe -filter=FeefracMultipication

| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------
...

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1629427388)
Changed to use our own `reverse_iterator`. Opted to instanciate the `std::span` from an iterator instead of adding a dependency on https://github.com/bitcoin/bitcoin/pull/30229. Happy to rebase on top of it if it's merged first.

💬 glozow commented on pull request "refactor: TxDownloadManager":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2152299577)
rebased

💬 glozow commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#issuecomment-2152302281)
Sorry, I kept thinking I would get back to this but haven't. Closing + marking up for grabs.
Note for anybody who wants to pick this up, note you'll want to write the doc described in https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1378636536

✅ glozow closed a pull request: "rpc: distinguish between vsize and sigop-adjusted mempool vsize"
(https://github.com/bitcoin/bitcoin/pull/27591)

💬 glozow commented on pull request "refactor: policy: Pass kernel::MemPoolOptions to IsStandard[Tx] rather than long list of individual options":
(https://github.com/bitcoin/bitcoin/pull/30232#issuecomment-2152305711)
Mind providing some motivation for the refactor? PR description is empty

💬 fanquake commented on pull request "Sanitizing ports of -rpcconnect and -rpcport.":
(https://github.com/bitcoin/bitcoin/pull/27820#issuecomment-2152305956)
Picked up in #29521.

💬 theStack commented on pull request "test: Remove struct.pack from almost all places":
(https://github.com/bitcoin/bitcoin/pull/29401#discussion_r1629496043)
nit: for serializing single bytes I personally prefer `bytes([value])` over the `.to_bytes` method, e.g.
```suggestion
r = bytes([253]) + l.to_bytes(2, "little")
```
(but no blocker, feel free to ignore)

👍 theStack approved a pull request: "test: Remove struct.pack from almost all places"
(https://github.com/bitcoin/bitcoin/pull/29401#pullrequestreview-2101884249)
Code-review ACK fa52e13ee81fcc7543890dbd6986fcb55168583f