👍 pinheadmz approved a pull request: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177#pullrequestreview-1435414414)
re-ACK 272eb5561667482f8226bcf98eea00689dccefb8
confirmed trivial rebase and modifictaion to one test to pass CI, since last review
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 272eb5561667482f8226bcf98eea00689dccefb8
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRo3i4ACgkQ5+KYS2KJ
yTq+bw//bLFMWw6VL7OnEzumxPTLIFRHW6ol5Wsk8nIkgvwDmviLXRjU0Gh2K0G5
fMWUq0t2lrRqWzZgobcWfTG9+HNRn2J3Y/WuR+PeUfsOk7lE
...
(https://github.com/bitcoin/bitcoin/pull/27177#pullrequestreview-1435414414)
re-ACK 272eb5561667482f8226bcf98eea00689dccefb8
confirmed trivial rebase and modifictaion to one test to pass CI, since last review
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 272eb5561667482f8226bcf98eea00689dccefb8
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRo3i4ACgkQ5+KYS2KJ
yTq+bw//bLFMWw6VL7OnEzumxPTLIFRHW6ol5Wsk8nIkgvwDmviLXRjU0Gh2K0G5
fMWUq0t2lrRqWzZgobcWfTG9+HNRn2J3Y/WuR+PeUfsOk7lE
...
💬 Davidson-Souza commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555930857)
@pavel-vasin Yes, you're right. The compact block relay design, as it currently exists, does not permit computations.
> Salting the hash computation using the block header is needed to prevent intentional collisions, and is the price for using short ids.
Does it really need to be the block header? If we kept a random, per connection byte-array used for salting, how could an attacker cause intentional collisions?
(Thinking outside BIP152 context) If we hold a per connection salt, and a r
...
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555930857)
@pavel-vasin Yes, you're right. The compact block relay design, as it currently exists, does not permit computations.
> Salting the hash computation using the block header is needed to prevent intentional collisions, and is the price for using short ids.
Does it really need to be the block header? If we kept a random, per connection byte-array used for salting, how could an attacker cause intentional collisions?
(Thinking outside BIP152 context) If we hold a per connection salt, and a r
...
📝 furszy opened a pull request: "Return EXIT_FAILURE on post-init fatal errors"
(https://github.com/bitcoin/bitcoin/pull/27708)
It seems odd to return `EXIT_SUCCESS` when the node aborted execution due a fatal internal error
or any post-init problem that triggers an unrequested shutdown.
e.g. blocks or coins db I/O errors, disconnect block failure, failure during thread import (external
blocks loading process error), among others.
(https://github.com/bitcoin/bitcoin/pull/27708)
It seems odd to return `EXIT_SUCCESS` when the node aborted execution due a fatal internal error
or any post-init problem that triggers an unrequested shutdown.
e.g. blocks or coins db I/O errors, disconnect block failure, failure during thread import (external
blocks loading process error), among others.
💬 hebasto commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1555932774)
Guix builds:
```
15e52e32e8ad2574fa5a1bd5e7699f33b7084a63d4ed3951a9b9f22632dd57fb guix-build-f33211a4fe56/output/aarch64-linux-gnu/SHA256SUMS.part
1c981a6d7cce696f4b562443ef53f401dfa6edc4704154f453ee4f66b8fc72d6 guix-build-f33211a4fe56/output/aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu-debug.tar.gz
c8d0b81e5ad5f504650690c9b0192338b22a09aeead9cc1a48054e228704f1a8 guix-build-f33211a4fe56/output/aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu.tar.gz
4e59bc9cbe2499fd415
...
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1555932774)
Guix builds:
```
15e52e32e8ad2574fa5a1bd5e7699f33b7084a63d4ed3951a9b9f22632dd57fb guix-build-f33211a4fe56/output/aarch64-linux-gnu/SHA256SUMS.part
1c981a6d7cce696f4b562443ef53f401dfa6edc4704154f453ee4f66b8fc72d6 guix-build-f33211a4fe56/output/aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu-debug.tar.gz
c8d0b81e5ad5f504650690c9b0192338b22a09aeead9cc1a48054e228704f1a8 guix-build-f33211a4fe56/output/aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu.tar.gz
4e59bc9cbe2499fd415
...
💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199624849)
> Said that, talking about the init returned error, maybe we could cache the node shutdown reason and return it even if the error was post-init?. Will give it a look.
Following-up this, pushed #27708. Which will let us keep the same behavior as we have now:
The user, same as the functional tests, shouldn't notice any difference between running the pruning violation checks in the load-blk thread vs init thread with it. We will continue erroring out after a pruning violation with an `EXIT_FA
...
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199624849)
> Said that, talking about the init returned error, maybe we could cache the node shutdown reason and return it even if the error was post-init?. Will give it a look.
Following-up this, pushed #27708. Which will let us keep the same behavior as we have now:
The user, same as the functional tests, shouldn't notice any difference between running the pruning violation checks in the load-blk thread vs init thread with it. We will continue erroring out after a pruning violation with an `EXIT_FA
...
💬 hebasto commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199626358)
```suggestion
| Linux Kernel | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes |
```
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199626358)
```suggestion
| Linux Kernel | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes |
```
💬 hebasto commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199627459)
What Linux systems do require `#include <unistd.h>` for the `getrandom` call?
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199627459)
What Linux systems do require `#include <unistd.h>` for the `getrandom` call?
🤔 hebasto reviewed a pull request: "random: drop syscall wrapper usage for getrandom()"
(https://github.com/bitcoin/bitcoin/pull/27699#pullrequestreview-1435419332)
Approach ACK f33211a4fe563e03542343ee033d7f5ec7887cbf.
(https://github.com/bitcoin/bitcoin/pull/27699#pullrequestreview-1435419332)
Approach ACK f33211a4fe563e03542343ee033d7f5ec7887cbf.
💬 fanquake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199627846)
None?
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199627846)
None?
💬 fanquake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199630126)
Done.
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199630126)
Done.
💬 hebasto commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199630721)
Then may be drop it? (as it done c13c97dbf846cf0e6a5581ac414ef96a215b0dc6)
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199630721)
Then may be drop it? (as it done c13c97dbf846cf0e6a5581ac414ef96a215b0dc6)
💬 fanquake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199631142)
Dropped.
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1199631142)
Dropped.
👍 hebasto approved a pull request: "random: drop syscall wrapper usage for getrandom()"
(https://github.com/bitcoin/bitcoin/pull/27699#pullrequestreview-1435422548)
ACK 5228223e1ff2af29e6e77668ce3288005c2adbbc, I've tested build system changes on Ubuntu 22.04 and macOS Monterey 12.6.6 (x86_64).
(https://github.com/bitcoin/bitcoin/pull/27699#pullrequestreview-1435422548)
ACK 5228223e1ff2af29e6e77668ce3288005c2adbbc, I've tested build system changes on Ubuntu 22.04 and macOS Monterey 12.6.6 (x86_64).
💬 hebasto commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1555955582)
Guix builds:
```
9dd5852b303462b94703c06d30f432a19769379026c4f61001f452893dbe8121 guix-build-5228223e1ff2/output/aarch64-linux-gnu/SHA256SUMS.part
19f9e24d91522aac6c349dffd4ae5b0386bc15c92767a65eb393e6a42ab5b256 guix-build-5228223e1ff2/output/aarch64-linux-gnu/bitcoin-5228223e1ff2-aarch64-linux-gnu-debug.tar.gz
935f337b5679825aadb004ab0848ba6bae5f7c287f17198faec361eae83e7f1e guix-build-5228223e1ff2/output/aarch64-linux-gnu/bitcoin-5228223e1ff2-aarch64-linux-gnu.tar.gz
badb4ce4f57e8b27d61
...
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1555955582)
Guix builds:
```
9dd5852b303462b94703c06d30f432a19769379026c4f61001f452893dbe8121 guix-build-5228223e1ff2/output/aarch64-linux-gnu/SHA256SUMS.part
19f9e24d91522aac6c349dffd4ae5b0386bc15c92767a65eb393e6a42ab5b256 guix-build-5228223e1ff2/output/aarch64-linux-gnu/bitcoin-5228223e1ff2-aarch64-linux-gnu-debug.tar.gz
935f337b5679825aadb004ab0848ba6bae5f7c287f17198faec361eae83e7f1e guix-build-5228223e1ff2/output/aarch64-linux-gnu/bitcoin-5228223e1ff2-aarch64-linux-gnu.tar.gz
badb4ce4f57e8b27d61
...
💬 beeduul commented on issue "Do not crash if peers.dat is corrupted":
(https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1555962035)
Although this issue has been marked as fixed for the next release, I'll leave this additional note here for posterity.
This issue happens me every few days on my 4gb pi umbrel. It appears that immediately before each crash, the log contains `Socks5() connect to xxx.xxx.xxx.xxx:8333 failed: InterruptibleRecv() timeout or other failure`.
(https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1555962035)
Although this issue has been marked as fixed for the next release, I'll leave this additional note here for posterity.
This issue happens me every few days on my 4gb pi umbrel. It appears that immediately before each crash, the log contains `Socks5() connect to xxx.xxx.xxx.xxx:8333 failed: InterruptibleRecv() timeout or other failure`.
👍 TheCharlatan approved a pull request: "ci, iwyu: Double maximum line length for includes"
(https://github.com/bitcoin/bitcoin/pull/27707#pullrequestreview-1435439213)
ACK 98ea79841177cf7492ec1de1ba1979f75390c63b
(https://github.com/bitcoin/bitcoin/pull/27707#pullrequestreview-1435439213)
ACK 98ea79841177cf7492ec1de1ba1979f75390c63b
💬 ArmchairCryptologist commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1555986348)
For what it's worth, I ran the node with debug logging for a bit, and while it seems to be mostly (but not exclusively) the same peer causing the stall, it also doesn't necessarily seem malicious, just slow, since it does successfully transfer *some* blocks. The client seems to really like requesting blocks from the peer in question, so I'm guessing it get blocks earlier than most other peers, and possibly gets overloaded because of it. In some cases it even looks to be requesting another block
...
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1555986348)
For what it's worth, I ran the node with debug logging for a bit, and while it seems to be mostly (but not exclusively) the same peer causing the stall, it also doesn't necessarily seem malicious, just slow, since it does successfully transfer *some* blocks. The client seems to really like requesting blocks from the peer in question, so I'm guessing it get blocks earlier than most other peers, and possibly gets overloaded because of it. In some cases it even looks to be requesting another block
...
👍 TheCharlatan approved a pull request: "ci: remove `RUN_SECURITY_TESTS`"
(https://github.com/bitcoin/bitcoin/pull/27683#pullrequestreview-1435441971)
ACK 6a936580d1c42576f627d5fac5423ec7af88e547
Doesn't make sense to maintain this.
(https://github.com/bitcoin/bitcoin/pull/27683#pullrequestreview-1435441971)
ACK 6a936580d1c42576f627d5fac5423ec7af88e547
Doesn't make sense to maintain this.
💬 mzumsande commented on issue "Do not crash if peers.dat is corrupted":
(https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1556008722)
> Although this issue has been marked as fixed for the next release, I'll leave this additional note here for posterity.
>
> This issue happens me every few days on my 4gb pi umbrel. It appears that immediately before each crash, the log contains `Socks5() connect to xxx.xxx.xxx.xxx:8333 failed: InterruptibleRecv() timeout or other failure`.
To be clear: the fix doesn't prevent any crashes from happening - what it fixes is that if the node crashes for some unrelated reason, `peers.dat` sho
...
(https://github.com/bitcoin/bitcoin/issues/26599#issuecomment-1556008722)
> Although this issue has been marked as fixed for the next release, I'll leave this additional note here for posterity.
>
> This issue happens me every few days on my 4gb pi umbrel. It appears that immediately before each crash, the log contains `Socks5() connect to xxx.xxx.xxx.xxx:8333 failed: InterruptibleRecv() timeout or other failure`.
To be clear: the fix doesn't prevent any crashes from happening - what it fixes is that if the node crashes for some unrelated reason, `peers.dat` sho
...
💬 ajtowns commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1199687717)
Should be `!pfrom.IsInboundConn()` presumably, to include manual connections at least.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1199687717)
Should be `!pfrom.IsInboundConn()` presumably, to include manual connections at least.