⚠️ brunoerg opened an issue: "Compute 'short id' when transaction joins mempool"
(https://github.com/bitcoin/bitcoin/issues/27706)
When a node receives a `cmpctblock` it has to verify to its check mempool in order to know whether it has all the required transactions to construct that block. If it doesn't, it will send `getblocktxn` to fetch the missing tx{s}.
`PartiallyDownloadedBlock::InitData` shows we have to iterate the whole mempool in order to get the short id and do the verifications, see:
```cpp
for (size_t i = 0; i < pool->vTxHashes.size(); i++) {
uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[
...
(https://github.com/bitcoin/bitcoin/issues/27706)
When a node receives a `cmpctblock` it has to verify to its check mempool in order to know whether it has all the required transactions to construct that block. If it doesn't, it will send `getblocktxn` to fetch the missing tx{s}.
`PartiallyDownloadedBlock::InitData` shows we have to iterate the whole mempool in order to get the short id and do the verifications, see:
```cpp
for (size_t i = 0; i < pool->vTxHashes.size(); i++) {
uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[
...
💬 brunoerg commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555893035)
cc: @Davidson-Souza
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555893035)
cc: @Davidson-Souza
📝 hebasto opened a pull request: "ci, iwyu: Double maximum line length for includes"
(https://github.com/bitcoin/bitcoin/pull/27707)
This PR makes the IWYU output in the CI 'tidy' task more useful by avoiding most cases where a comment ends with an ellipsis like that:
```
#include "primitives/transaction.h" // for CTxIn, CMutableTransaction, CTra...
```
(https://github.com/bitcoin/bitcoin/pull/27707)
This PR makes the IWYU output in the CI 'tidy' task more useful by avoiding most cases where a comment ends with an ellipsis like that:
```
#include "primitives/transaction.h" // for CTxIn, CMutableTransaction, CTra...
```
💬 pavel-vasin commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555904229)
Short IDs, as they are specified in BIP 152, cannot be computed in advance because their formula includes a block header (and a random nonce) in addition to a transaction content.
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555904229)
Short IDs, as they are specified in BIP 152, cannot be computed in advance because their formula includes a block header (and a random nonce) in addition to a transaction content.
💬 sipa commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555906319)
Yeah, what @pavel-vasin says.
Salting the hash computation using the block header is needed to prevent intentional collisions, and is the price for using short ids.
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555906319)
Yeah, what @pavel-vasin says.
Salting the hash computation using the block header is needed to prevent intentional collisions, and is the price for using short ids.
💬 fanquake commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1555907175)
@ArmchairCryptologist Thanks for reporting. This is an issue we are aware of. To avoid any confusion, and make it clear you're seeing the issue with `v24.1`, I've edited your post slightly.
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1555907175)
@ArmchairCryptologist Thanks for reporting. This is an issue we are aware of. To avoid any confusion, and make it clear you're seeing the issue with `v24.1`, I've edited your post slightly.
💬 brunoerg commented on issue "Compute 'short id' when transaction joins mempool":
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555917084)
Makes sense, thanks for the clarification!
(https://github.com/bitcoin/bitcoin/issues/27706#issuecomment-1555917084)
Makes sense, thanks for the clarification!
👍 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).