✅ hebasto closed a pull request: "test: Make `mempool_tests/MempoolSizeLimitTest` allocation-neutral"
(https://github.com/bitcoin/bitcoin/pull/26615)

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1478368582)
@mzumsande this can be reduced to one, but requires some additional complexity: https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142467276

💬 furszy commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143819895)
oops, fixed. And added coverage for it.

💬 ishaanam commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143830313)
> It is also not correct to use outputs.size() as outputs will be empty if the user didn't customize them.

In what scenario would we need to make the change output the sole recipient if the user didn't customize the outputs? My understanding was that this is only an issue because of the addition of the "outputs" vector.

💬 furszy commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1143834924)
a send-to-self, single change output tx that is bumped.

💬 jonatack commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1143851851)
@josibake Our clang-format wouldn't agree. It would revert your diff to what was merged (and propose this change instead).

```diff
@@ -4224,7 +4224,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,

if (received_new_header) {
LogPrintfCategory(BCLog::NET, "Saw new cmpctblock header hash=%s peer=%d\n",
- blockhash.ToString(), pfrom.GetId());
+ blockhash.ToString(), pfrom.GetId());

...

💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143839532)
I could be reading this code wrong, but isn't each tx taking inputs from each tx before it? It's still parents-and-child technically but it's also a chain.

```suggestion
// Where each tx takes an input from all prior txns
```

💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143826570)
Why this size exactly - the first tx isn't guaranteed to be the smallest or anything? Why not just 0?

💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143849591)
Why? I don't think it has any impact on the construction performance?

💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143851697)
nit: I think we tend to prefer this
```suggestion
for (size_t i{0}; i < txns.size(); ++i) {
```

💬 glozow commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143841355)
Er, what does this comment mean?

💬 jonatack commented on pull request "bench: Expand eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#issuecomment-1478433249)
Sorry for the drive-by suggestion: from the title, I thought this PR might be about the peer eviction benchmarks. Maybe update the title to s/eviction/mempool eviction/.

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#issuecomment-1478434780)
@jonatack good point, updated

💬 theStack commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1143874803)
nit: seems like this function is never needed in another module, also not in follow-up PRs (checked at #24545)
```suggestion
static void RFC8439Crypt(ChaCha20& c20, const Span<const std::byte> in_bytes, Span<std::byte> out_bytes)
```

💬 theStack commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1143873847)
nit: const-correctness
```suggestion
const Span<const std::byte> aad, const Span<const std::byte> ciphertext,
```

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1143875114)
done.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1143875257)
done.

💬 achow101 commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#discussion_r1143891389)
GPG's manpage suggests using `--status-fd` (or `--status-file`) to for unattended usage of gpg. This outputs machine parseable lines to the specified file descriptor (or file) that look like:

```
[GNUPG:] NEWSIG fanquake@gmail.com
[GNUPG:] KEY_CONSIDERED E777299FC265DD04793070EB944D35F9AC3DB76A 0
[GNUPG:] SIG_ID IpS4EHolPjGh7he2GCBYNha53aY 2022-11-17 1668711593
[GNUPG:] KEY_CONSIDERED E777299FC265DD04793070EB944D35F9AC3DB76A 0
[GNUPG:] GOODSIG 2EEB9F5CC09526C1 Michael Ford (bitcoin-otc)
...

💬 mzumsande commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1478469232)
I changed the approach to @TheCharlatan's idea (added you as coauthor) - now we don't assert anymore but raise the size of the blockfile to the size of the added block if needed.

💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1143904639)
> The net change isn't different however.

No, the net change is you've pointlessly broken things like git bisect. Please try and avoid doing that.