💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199939444)
> This is already the case now in case of -reindex-chainstate after https://github.com/bitcoin/bitcoin/pull/25193 (I didn't think of that), but it could be easily changed in master by moving g_indexes_ready_to_sync = true up one line in ThreadImport.
yeah, I don't see why that would be a downside of this changes neither. Same as with `g_indexes_ready_to_sync` flag, the `StartIndexes()` call could be executed before the mempool load too.
It wouldn't be bad to rename `ThreadImport` to `Impo
...
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199939444)
> This is already the case now in case of -reindex-chainstate after https://github.com/bitcoin/bitcoin/pull/25193 (I didn't think of that), but it could be easily changed in master by moving g_indexes_ready_to_sync = true up one line in ThreadImport.
yeah, I don't see why that would be a downside of this changes neither. Same as with `g_indexes_ready_to_sync` flag, the `StartIndexes()` call could be executed before the mempool load too.
It wouldn't be bad to rename `ThreadImport` to `Impo
...
💬 jarolrod commented on pull request "build: Do not define `ENABLE_ZMQ` when ZMQ is not available":
(https://github.com/bitcoin/bitcoin/pull/27696#issuecomment-1556549338)
hashes:
```
7091c64cada36ec2850f889978ae5fdc3cf4f28ec40bfb1fd16100c3f502fe6e guix-build-fa5831bd6f94/output/aarch64-linux-gnu/SHA256SUMS.part
e3b87e22e4cbd7a50aac4d5c982c12dd5b3f224ff4f9543dd3c331d424899287 guix-build-fa5831bd6f94/output/aarch64-linux-gnu/bitcoin-fa5831bd6f94-aarch64-linux-gnu-debug.tar.gz
96ffa1075a4b1d14c842bb8a5f5106cdb8f401eb173578c7376f6454b918e4eb guix-build-fa5831bd6f94/output/aarch64-linux-gnu/bitcoin-fa5831bd6f94-aarch64-linux-gnu.tar.gz
1bbe3a56784762b811eadc
...
(https://github.com/bitcoin/bitcoin/pull/27696#issuecomment-1556549338)
hashes:
```
7091c64cada36ec2850f889978ae5fdc3cf4f28ec40bfb1fd16100c3f502fe6e guix-build-fa5831bd6f94/output/aarch64-linux-gnu/SHA256SUMS.part
e3b87e22e4cbd7a50aac4d5c982c12dd5b3f224ff4f9543dd3c331d424899287 guix-build-fa5831bd6f94/output/aarch64-linux-gnu/bitcoin-fa5831bd6f94-aarch64-linux-gnu-debug.tar.gz
96ffa1075a4b1d14c842bb8a5f5106cdb8f401eb173578c7376f6454b918e4eb guix-build-fa5831bd6f94/output/aarch64-linux-gnu/bitcoin-fa5831bd6f94-aarch64-linux-gnu.tar.gz
1bbe3a56784762b811eadc
...
👍 jarolrod approved a pull request: "build: Do not define `ENABLE_ZMQ` when ZMQ is not available"
(https://github.com/bitcoin/bitcoin/pull/27696#pullrequestreview-1435784103)
ACK fa5831bd6f940c4afb43ff625ba4fa6c641e999a
(https://github.com/bitcoin/bitcoin/pull/27696#pullrequestreview-1435784103)
ACK fa5831bd6f940c4afb43ff625ba4fa6c641e999a
💬 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
...
(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
(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
(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
...
(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
...
(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?
(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?
(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
...
(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)
(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.
(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)
(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.
(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)
(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)
(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
(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.
(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
...
(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
...