💬 fanquake commented on pull request "doc: filter out merge-script from list of authors":
(https://github.com/bitcoin/bitcoin/pull/27703#issuecomment-1555883473)
Going to cherry-pick this into a larger set of changes, to cleanup the release process documentation.

✅ fanquake closed a pull request: "doc: filter out merge-script from list of authors"
(https://github.com/bitcoin/bitcoin/pull/27703)

💬 instagibbs commented on pull request "rpc: add `descriptorprocesspsbt` rpc":
(https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1555885480)
reACK https://github.com/bitcoin/bitcoin/pull/25796/commits/1bce12acd3e271a7c88d9400b4e3a5645bc8a911

👍 hebasto approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620)
re-ACK bbfbcd0360e486d47025a412f2c0f4e8535ab255

💬 josibake commented on pull request "ci: remove `RUN_SECURITY_TESTS`":
(https://github.com/bitcoin/bitcoin/pull/27683#issuecomment-1555887845)
code review ACK https://github.com/bitcoin/bitcoin/pull/27683/commits/6a936580d1c42576f627d5fac5423ec7af88e547

+1 on removing "off by default" stuff from the CI scripts. also agree that guix is the right place to be handling these sorts of checks

⚠️ 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[
...

💬 brunoerg commented on issue "Compute 'short id' when transaction joins mempool":
(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...
```

💬 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.

💬 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.

💬 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.

💬 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!

👍 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
...

💬 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
...

📝 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.

💬 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
...

💬 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
...

💬 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 |
```

💬 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?

🤔 hebasto reviewed a pull request: "random: drop syscall wrapper usage for getrandom()"
(https://github.com/bitcoin/bitcoin/pull/27699#pullrequestreview-1435419332)
Approach ACK f33211a4fe563e03542343ee033d7f5ec7887cbf.