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

💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1143907496)
I switched to your suggestion, removing the assert!

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143918290)
hmmm, I don't recall. I can just set to 0

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143923923)
will remove

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#discussion_r1143932896)
looks like something that doesn't matter, based on me getting rid of that factor. Removed.

💬 instagibbs commented on pull request "bench: Expand mempool eviction benchmarking":
(https://github.com/bitcoin/bitcoin/pull/27019#issuecomment-1478511512)
fixups included, squashed

💬 theuni commented on pull request "build: Link `libbitcoinkernel` to `libbitcoin_util`":
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1478520029)
Agree with the NACKs here. It would be nice if we could share the objs to reduce compilation time, but there are good reasons to keep things separate.

But to add another practical problem, eventually libbitcoinkernel will need to be built with a `-DLIBBITCOINKERNEL_SHARED` or so once it gains an API. That would mean that objects are no-longer identical to (or shareable with) our internal libs.

💬 dhruv commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1143941594)
@fanquake not sure I understand. At which commit is the build broken? I specifically did it in a way that avoided that but may be misunderstanding something.

💬 fanquake commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1143951406)
@dhruv ah, my misread was that you'd left the linker errors between commits 2 & 4. Although, I think introducing this, just to change in later commits is still not ideal. Will re-mark this as resolved, and post new review below.

💬 achow101 commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1478541182)
ACK 730bff20c1f76c46a7ab6c8d981126fe3ad2cec9

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1143958464)
Done with one more tweak. I'll use an RAII wrapper if I have to touch the code again, and if that's the case, could you pls point me to an example already here in the bitcoin core code if already exists? Thanks.

💬 brunoerg commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1478543142)
Rebased

💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1478551323)
> Something like: connect to one generic block-relay-only peer, plus an additional block-relay-only peer for every network we support? I think that should only improve things, including during IBD. It wouldn't do anything to improve tx propagation of course, so wouldn't make this PR redundant.

Also, should we have (at least) one anchor per network instead of only two generic ones?