Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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.
💬 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