💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1199572071)
> but is there a bug?

No, there is not. I've double-checked the code, it looks OK for me too.

💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1555785546)
Reviewing the code:

- [x] 974b4293a5f2fe01302c608bf0063e3b52721d0b "kernel: Add notification interface"
- [x] ec5043d8c120332c0bf916b5b576aff8374af1ed "kernel: Add headerTip method to notifications"
- [ ] 1be1c16b70edc5def98eb1903cee08b5d546ba88 "kernel: Add progress method to notifications"
- [ ] 483fe64956179872e5821483474c52d955949e40 "kernel: Add warning method to notifications"
- [ ] 3785d38417618e921c5233cf14aa3c802f37387b "refactor: Move ScheduleBatchPriority to its own file"
- [
...

💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1555793887)
CI is happy now

💬 Sjors commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1555806652)
> > My understanding was that doing an optimal sort for a given cluster, requires it to have a few dozen transactions.
>
> It's the opposite really. [...] Up to 15-20 transactions we may be able to find it using an exponential-time algorithm; above that [...]

I meant to say "no more than a few dozen transactions".

So I guess we're ok with falling back to less than optimal sorting _within_ each cluster?

⚠️ ArmchairCryptologist opened an issue: "Frequent "Timeout downloading block" with 24.1.0"
(https://github.com/bitcoin/bitcoin/issues/27705)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

After updating to 24.1.0 yesterday I'm seeing frequent "Timeout downloading block" on two of my four public nodes, which is pretty annoying since the timeout is 10 minutes, which means every single timeout causes a 10-minute stall on new block downloads. The other two public nodes both have a couple of timeouts, but nothing frequent. A private listen=0 tor-only node also running 24.1.0 doe
...

🤔 hebasto reviewed a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435386964)
ACK d96c82a76775b1a41c098e6af60130fbdbba9975, I have reviewed the code and it looks OK.

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1555876922)
Updated d96c82a76775b1a41c098e6af60130fbdbba9975 -> a30cd548f1bc0ed73a4e157c6e81dbcf2542a916 ([chainstateRmKernelUiInterface_10](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_10) -> [chainstateRmKernelUiInterface_11](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_11), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_10..chainstateRmKernelUiInterface_11)).
* Addressed @hebasto's [comment](https:
...

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1555879559)
Rebased a30cd548f1bc0ed73a4e157c6e81dbcf2542a916 -> bbfbcd0360e486d47025a412f2c0f4e8535ab255 ([chainstateRmKernelUiInterface_11](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_11) -> [chainstateRmKernelUiInterface_12](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_11..chainstateRmKernelUiInterface_12)).
* Fixed broken include introduced by ht
...

💬 josibake commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1555881591)
ACK https://github.com/bitcoin/bitcoin/pull/27699/commits/f33211a4fe563e03542343ee033d7f5ec7887cbf

Nice to be able to drop the work-around :rocket:

I got the same as you for the guix build:

```
1c981a6d7cce696f4b562443ef53f401dfa6edc4704154f453ee4f66b8fc72d6 aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu-debug.tar.gz
c8d0b81e5ad5f504650690c9b0192338b22a09aeead9cc1a48054e228704f1a8 aarch64-linux-gnu/bitcoin-f33211a4fe56-aarch64-linux-gnu.tar.gz
4e655cf22f003455a14edd9408e
...

🚀 fanquake merged a pull request: "doc: remove Security section from build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27688)

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