💬 TheCharlatan commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2121073979)
I think it should be ok to scope the lock to just the necessary call to `LookupBlockIndex`, but I am also having a hard time saying for certain, that there might not be some edge case race condition here with the later call to `Contains` and `FindFork`.

💬 TheCharlatan commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2121131527)
I think you are just comparing pointers here, but actually want to compare their height. This block of code is also kind of hard to test against from a functional test. Maybe we can add a unit test for `StartIndexBackgroundSync` that manipulates some of the internal block index state?

💬 TheCharlatan commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r2121167855)
Nit: These include fixes seem a bit random. Why not apply all suggested fixes from the CI?

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2930801207)
> if I (perhaps accidentally) `sendrawtransaction` the same tx a second time, we open private connections, send our INV -- but those nodes already have the tx.

This PR will not queue new private broadcast connections if the same tx is submitted again. However, it does keep trying to broadcast with the 3 connections queued from the first time the tx is submitted. If those peers already have the tx, then the connection will wait for 3 minutes until disconnecting. See discussion [here](https://g
...

👍 TheCharlatan approved a pull request: "doc: Remove build instruction for running `clang-tidy`"
(https://github.com/bitcoin/bitcoin/pull/32662#pullrequestreview-2888488249)
ACK 4b1b36acb48fab133ca4a3241148fa9683915874

🤔 janb84 reviewed a pull request: "doc: Remove build instruction for running `clang-tidy`"
(https://github.com/bitcoin/bitcoin/pull/32662#pullrequestreview-2888493704)
ACK 4b1b36acb48fab133ca4a3241148fa9683915874

Checked both outcomes, the "old" way and the new way, both outcomes are the equal.(the diff was harder to check due to running it in multithreaded mode, therefor the files where out of order)

🚀 fanquake merged a pull request: "doc: Remove build instruction for running `clang-tidy`"
(https://github.com/bitcoin/bitcoin/pull/32662)

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2930824497)
It seems that if a (miniscript, not yet spendable) script path is present in the descriptor, then `walletprocesspsbt` won't provide a `pubnonce`.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2120994262)
Yes, seemed like the most fair way to do it.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2120989801)
It's a part of what boost multiindex requires for its "key extractor" concept: https://www.boost.org/doc/libs/1_88_0/libs/multi_index/doc/reference/key_extraction.html

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2121231438)
Would we be able to distinguish shorttxid collision from an intentionally mutated block without the check?

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121277977)
See `AddCoins`'s declaration in `src/coins.h`:
```cpp
void AddCoins(CCoinsViewCache& cache, const CTransaction& tx, int nHeight, bool check = false);
```

💬 l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2121282517)
I don't think there could/should be, you could name them as `const auto [x, y, z]{hss.ProcessNextHeaders(std::span{second_chain.begin() + 1, second_chain.end()}, false)};` as well

💬 hebasto commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#issuecomment-2930944523)
My Guix build:
```
aarch64
6ec55ec67ac33aeb89eae88bd55525e36e05d3c768eb98cf9c718cc8221de76e guix-build-18cf727429e9/output/aarch64-linux-gnu/SHA256SUMS.part
f3e260544a199b5b06f3f5de4352fcff61f6a3e36fd12a827317c2ba168ab3a8 guix-build-18cf727429e9/output/aarch64-linux-gnu/bitcoin-18cf727429e9-aarch64-linux-gnu-debug.tar.gz
259b8bd0260adc1517c11a8579983210501f6625083b780f3451c5bf6d531c52 guix-build-18cf727429e9/output/aarch64-linux-gnu/bitcoin-18cf727429e9-aarch64-linux-gnu.tar.gz
6772df34
...

💬 l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2121291449)
When we're only iterating inside, it makes sense to have a fine-grained, segregated interface and only define the narrowest functionality which supports the operations we actually need inside. It announces to the reader that we shouldn't expect any extra complexity, only the ones that the interface implements. But if you think vector does it better, I'm also fine with that. For me span is not about a subset of teh data but a subset of the functionality - just a narrowed view to rule out certain
...

💬 l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2121293497)
Thanks for checking!

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2930956026)
Rebased e604a57dccac1c4e6802a3ecc7f4cb4c3229c8ed -> 15ff9cb1789b7fe69c0ad24d737712aa6955e9cb ([spendblock_3](https://github.com/TheCharlatan/bitcoin/tree/spendblock_3) -> [spendblock_4](https://github.com/TheCharlatan/bitcoin/tree/spendblock_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_3..spendblock_4))

* Fixed silent merge conflict with #32304

💬 tnndbtc commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that …":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2121315218)
@maflcko Nope. I only used chatgpt to study the functions. However, every letter in the PR is typed by me. :)

💬 John-zhan commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2930992630)
Ok, now downgrade my windows cmake version from 4 to 3.31.7。And now when execute 'cmake -B build --preset vs2022-static' it say this:

`正在安装 55/93 个 openssl:x64-windows@3.3.2#1...
正在生成 openssl:x64-windows@3.3.2#1...
C:\Users\win11\AppData\Local\vcpkg\registries\git-trees\2c0920b57254210866a89aa705291b452a31df48: info: installing from git registry git+https://github.com/microsoft/vcpkg@2c0920b57254210866a89aa705291b452a31df48
-- Using cached openssl-openssl-210dc9a50dfd99caa1cf7c3d2fa42850124b1bb
...

💬 hebasto commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2931010856)
> Download failed, halting portfile.

means some network issues.

If it is not intermittent, please report the issue [upstream](https://github.com/microsoft/vcpkg).