💬 ajtowns commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1556553095)
> In particular, I wonder if the fact that we'll now try additional peers will lead to redundancies / increased traffic under non-adversarial conditions where there's no staller but delivering the block just needs some time - especially when blocks contain a large number of txns that weren't in our mempool, or on slower networks like Tor. Did you consider the possibility of giving the original peer a few seconds to deliver before requesting from additional peers?

Yeah, this is the behaviour I
...

👍 MarcoFalke approved a pull request: "ci, iwyu: Double maximum line length for includes"
(https://github.com/bitcoin/bitcoin/pull/27707#pullrequestreview-1435841742)
lgtm

💬 MarcoFalke commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1556608479)
concept ACK

🤔 MarcoFalke reviewed a pull request: "test: Explicitly specify directory where to search tests for"
(https://github.com/bitcoin/bitcoin/pull/27561#pullrequestreview-1435948057)
lgtm ACK c44f3f231988dc05c4c7a8a96bc2e7b1a54da277 💸

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK c44f3f231988dc05
...

🤔 MarcoFalke reviewed a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435962922)
nice ACK bbfbcd0360e486d47025a412f2c0f4e8535ab255 😇

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK bbfbcd0360e486d4
...

💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1200080302)
nit in 4452707edec91c7d7991f486dd41ef3edb4f7fbf: not sure what the convention is, but most other places seem to use `Untranslated("")` over this?

💬 MarcoFalke commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1200080447)
same?

💬 ajtowns commented on issue "Enable consistency checks by default with `--enable-debug`":
(https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1556705320)
Linux distros do detached debug symbols so that you can grab them after the fact and just use them with your regular production binaries. cf http://debug.mirrors.debian.org/debian-debug/pool/main/b/bitcoin/ Doing that for our guix builds might be worthwhile, in which case perhaps we could always build with debug symbols?

Thinking about it more, I think for me the levels are more something like:

* production -- should only abort on unrecoverable errors, should be as fast as possible
* de
...

🚀 fanquake merged a pull request: "ci, iwyu: Double maximum line length for includes"
(https://github.com/bitcoin/bitcoin/pull/27707)

💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1556804109)
Rebased on top of the merged https://github.com/bitcoin/bitcoin/pull/27707.

🚀 fanquake merged a pull request: "ci: remove `RUN_SECURITY_TESTS`"
(https://github.com/bitcoin/bitcoin/pull/27683)

💬 fanquake commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1556818802)
Ok. Closing for now. Discussion can continue, but it's not clear why this is an issue. General questions /discussion can also be asked/happen in IRC etc.

✅ fanquake closed an issue: "Compute 'short id' when transaction joins mempool"
(https://github.com/bitcoin/bitcoin/issues/27706)

🚀 fanquake merged a pull request: "build: Do not define `ENABLE_ZMQ` when ZMQ is not available"
(https://github.com/bitcoin/bitcoin/pull/27696)

💬 fanquake commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1556839009)
cc @theuni

💬 fanquake commented on issue "Do not crash if peers.dat is corrupted":
(https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1556844214)
> So if your node crashes every few days, it sounds like you have another, unrelated problem.

@beeduul do you want to follow up with a new issue, providing more info if possible? Assuming this isn't a hardware related problem.

💬 ajtowns commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200117089)
I don't understand this logic -- why is having an outbound attempt in flight so terrible that we'll query all subsequent hb peers, even to the point of exceeding the parallel download limit? If outbound attempts are terrible, why do we also have an exception to immediately query hb outbound peers, also to the point of potentially exceeding the parallel download limit?

I would have expected logic more like:
* is this the first attempt?
* otherwise, is this a hb peer, and `already_in_flight
...

💬 de-served commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1556850708)
Ping

💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1200236216)
> not sure what the convention is

Neither do I.

> but most other places seem to use `Untranslated("")` over this?

```
$ git grep 'Untranslated("")'
src/net_permissions.cpp: error = Untranslated("");
src/net_permissions.cpp: error = Untranslated("");
src/net_permissions.cpp: error = Untranslated("");
```

I think that additional semantics--being untranslated--is not required for an empty string. An empty `bilingual_str` instance looks OK here in my opinion.

Anyway, I'm
...

💬 MarcoFalke commented on pull request "fuzz: Print error message when FUZZ is missing":
(https://github.com/bitcoin/bitcoin/pull/27672#issuecomment-1556863344)
Added a commit to also improve fuzz logging in the CI system: https://cirrus-ci.com/task/4987695903014912?logs=ci#L222