💬 fjahr commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#discussion_r1744492602)
What does the `detail_` prefix mean?

👋 hebasto's pull request is ready for review: "Reintroduce external signer support for Windows"
(https://github.com/bitcoin/bitcoin/pull/29868)

🤔 furszy reviewed a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2281468112)
Updated and rebased due to conflicts with a recently merged PR. The CI shouldn't complain now.

On a side note:
It took me longer to place my test code inside `feature_assumeutxo.py` than to fix the issue. I think it would be good to reorganize this file a bit and make the assumeUTXO parameters configurable at startup or runtime for tests. This would allow us to split tests into different files for each of the products we provide: network/p2p, local node, and wallet.

💬 benthecarman commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2330211051)
I am on 28.0rc1 and seem to be on a different chain. Block is `00000000000044a7284978db5161b2f4ca012b3fb981a9405a4ce2473f3e4a5f` for height `42545`, unsure how to get to the main chain

📝 marcofleon opened a pull request: "doc: fix assumeutxo design doc link"
(https://github.com/bitcoin/bitcoin/pull/30819)
A correction to a link as I was exploring Assumeutxo stuff.

💬 hebasto commented on issue "build: reproducibility issue with macOS Guix builds":
(https://github.com/bitcoin/bitcoin/issues/30815#issuecomment-2330215121)
This issue boils down to two following calls:

- https://github.com/bitcoin/bitcoin/blob/93e48240bfdc25c2760d33da69e739ba1f92da9b/cmake/module/Maintenance.cmake#L95
- https://github.com/bitcoin/bitcoin/blob/93e48240bfdc25c2760d33da69e739ba1f92da9b/cmake/module/Maintenance.cmake#L99

💬 fjahr commented on pull request "doc: fix assumeutxo design doc link":
(https://github.com/bitcoin/bitcoin/pull/30819#issuecomment-2330220764)
ACK e5f7272ad322aa4eb7a7d6f531419a4d7aee4802

Thanks @marcofleon for spotting this!

💬 hodlinator commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1744564915)
What happened to **wallet_tool** in the latest push?

2 dependencies exist according to **/doc/design/libraries.md**:
```
libbitcoin_wallet_tool-->libbitcoin_wallet;
libbitcoin_wallet_tool-->libbitcoin_util;
```

💬 hebasto commented on issue "build: reproducibility issue with macOS Guix builds":
(https://github.com/bitcoin/bitcoin/issues/30815#issuecomment-2330244811)
@fanquake @TheCharlatan

I wonder if binaries are reproducible on the _same_ machine during repeated builds?

💬 ryanofsky commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1744326718)
re: https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1740531844

> Looks like this was merged

Thanks! Updated

💬 ryanofsky commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1744573708)
I didn't dig into it but it appeared wallet_tool library disappeared in the conversion to cmake, so I removed references to it here. Probably libraries.md needs to be updated too though. There may be an existing cmake documentation PR where that change could be / has been made

💬 ryanofsky commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2330254084)
Updated 76ef8113f70070ab1deeeb142977d46d8132c36e -> 3e4312eef78f233eb7ae1d7d85e497de34144f2e ([`pr/weakcheck.8`](https://github.com/ryanofsky/bitcoin/commits/pr/weakcheck.8) -> [`pr/weakcheck.9`](https://github.com/ryanofsky/bitcoin/commits/pr/weakcheck.9), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/weakcheck.8..pr/weakcheck.9)) to fix failing CI job due to changed default build directory (https://cirrus-ci.com/task/5141392714891264?logs=ci#L5325)

💬 hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1744584134)
> I didn't dig into it but it appeared wallet_tool library disappeared in the conversion to cmake...

I couldn’t bring myself to create a library for just a single object file. :)

For the reference, the library was defined as follows: https://github.com/bitcoin/bitcoin/blob/80f00cafdeef0600fa1a5e906686720786c2336c/src/Makefile.am#L547-L549

🤔 furszy reviewed a pull request: "[WIP] p2p: send not_found msgs for unknown, pruned or unwilling to share blocks"
(https://github.com/bitcoin/bitcoin/pull/30385#pullrequestreview-2281528955)
> The assumeutxo node would just be a limited node, it'll offer the most recent blocks and none of the history. (it might have more history then it offers, as is the case of any other limited node.)

Yeah, this was the piece I was missing. Thanks!
Funny, I thought about a dynamically adjustable network signal but didn't consider reusing the limited service flag by enabling/disabling full-node service support based on the assumeUTXO background chain-sync status..

Created #30807 implementing
...

💬 luke-jr commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1744622700)
Seems like "echo err 1>&2` should be portable?

💬 luke-jr commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1744623636)
Why is an extra cmd.exe needed?

💬 luke-jr commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1744623828)
Seems like `echo err 1>&2` should be portable?

👍 tdb3 approved a pull request: "doc: fix assumeutxo design doc link"
(https://github.com/bitcoin/bitcoin/pull/30819#pullrequestreview-2281620339)
ACK e5f7272ad322aa4eb7a7d6f531419a4d7aee4802
Good eyes

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1744646596)
I'm sorry, I just don't see how those hardcoded tests are any different from the ones I linked. `ZeroL` is the same as `arith_uint256{0}`, and `uint256::ZERO` is the same as `uint256{0}`. Besides these aliases being irrelevant for a conversion test, their equivalence is tested in other unit tests already. What am I missing?

👍 tdb3 approved a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2281653280)
ACK e9ad9e01409eaa2009f75deb1cf834983c5a6137

Good find, thanks.
Tested similarly to @ismaelsadeeq (ran bitcoind on regtest independently of `p2p_permissions`) and observed the PR branch preventing collision (while the master branch encountered it).

Left a very minor nit. Feel free to ignore.