💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2553777710)
Makes sense. Thanks.

💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#issuecomment-3567526785)
Thanks for the review @w0xlt
- Addressed the [suggestion](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2547248797) and added you as co-author

✅ achow101 closed an issue: "Fresh install version 30.0 bitcoin-cli kicks me out if adding rpc creds"
(https://github.com/bitcoin/bitcoin/issues/33928)

💬 achow101 commented on issue "Fresh install version 30.0 bitcoin-cli kicks me out if adding rpc creds":
(https://github.com/bitcoin/bitcoin/issues/33928#issuecomment-3567596771)
Without `rpcuser` and `rpcpassword`, the RPC server creates an ephemeral user and password saved in `.cookie` in the datadir. `bitcoin-cli` will parse your `bitcoin.conf` for `rpcuser` and `rpcpassword` first, and if it doesn't find them, then it looks at `.cookie`. Since you haven't restarted Bitcoin Core yet, those new credentials aren't being used by the RPC server, but `bitcoin-cli` is trying to use them, hence an unauthorized error.

When you use `rpcauth`, the password is not included dire
...

💬 hebasto commented on issue "callgrind_annotate broken after cmake migration?":
(https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-3567810672)
> Maybe the prefix-map can be disabled by default or an option could be added to disable it, or it could be disabled on `-DWITH_CCACHE=NO`, as there is no need for it then?

https://github.com/bitcoin/bitcoin/commit/788c1324f3d840f7a39b8bc3537dcff26ca0b552 could be considered for reverting. However, doing so would resurface https://github.com/bitcoin/bitcoin/issues/30799.

💬 hebasto commented on issue "callgrind_annotate broken after cmake migration?":
(https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-3567833333)
> > Maybe the prefix-map can be disabled by default or an option could be added to disable it, or it could be disabled on `-DWITH_CCACHE=NO`, as there is no need for it then?
>
> [788c132](https://github.com/bitcoin/bitcoin/commit/788c1324f3d840f7a39b8bc3537dcff26ca0b552) could be considered for reverting. However, doing so would resurface [#30799](https://github.com/bitcoin/bitcoin/issues/30799).

And the latter could be addressed using another approach:
```diff
--- a/src/logging.cpp
+++ b/src
...

💬 hebasto commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-3567837050)
@pinheadmz

Does the approach proposed in both https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-3567810672 and https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-3567833333 make any difference to this issue?

💬 hebasto commented on pull request "build: Set AUTHOR_WARNING on warnings":
(https://github.com/bitcoin/bitcoin/pull/33144#issuecomment-3567848817)
cc @purpleKarrot

✅ l0rinc closed an issue: "malloc: Failed to allocate segment from range group - out of space"
(https://github.com/bitcoin/bitcoin/issues/33806)

💬 l0rinc commented on issue "malloc: Failed to allocate segment from range group - out of space":
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3567870300)
No idea what changed since, but I cannot reproduce it with `master` or with `27.2`.
Has anyone else tried it? Closing for now, we can reopen if it reproduces later.

💬 hebasto commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3567921587)
> Where is it defined or documented why `CMAKE_POSITION_INDEPENDENT_CODE` is set to `ON` in the first place?
>
> A better approach would be to not override user preferences at all and set the `POSITION_INDEPENDENT_CODE` target property explicitly on targets that need it.

Using only PIC objects by default was introduced in https://github.com/bitcoin/bitcoin/commit/19df238a7ba7a6cdfbf48bc663c39ddbf9f6c7d0.

💬 TheCharlatan commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554084325)
We should include post-condition checks here. For the `TestBlockValidity` refactor, we added
```
if (state.IsValid()) NONFATAL_UNREACHABLE();
```
I think such a check should be added wherever a boolean was returned previously.

💬 TheCharlatan commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554092568)
I don't think this change is correct. Previously false was returned when the block failed to be accepted or on a fatal error condition, not when it failed to validate. If you look into `ActivateBestChainStep`, you'll see that we break on a validation failure, reset the state again and don't return false. Can you drop this change again?

💬 hebasto commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3568067474)
> This introduces two behaviour changes:
>
> 1. a user-defined `CMAKE_POSITION_INDEPENDENT_CODE=OFF` will now be respected by our build system, instead of silently overriding it like before

Concept ACK on that.

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3568091456)
Rebased on top of #33629.

👋 andrewtoth's pull request is ready for review: "validation: fetch block inputs on parallel threads >20% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132)

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3568164009)
I've updated the PR to make the `InputFetcher` a subclass of `CCoinsViewCache`. Instead of waiting on the MPSC queue to be finished before connecting the block, the queue can be processed during `CCoinsViewCache::FetchCoin` during `ConnectBlock`. This makes the fetching completely non-blocking, which is a significant performance improvement. It's been renamed to `CoinsViewCacheAsync`.

@l0rinc thank you for your very thorough benchmarks. I've updated this to use 4 worker threads, which yields
...

💬 151henry151 commented on pull request "Defer transaction signing until user clicks Send":
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3568201828)
It looks like the one failing check is a CI issue with disk space too low. I'm not sure how to address that. I think the code of the PR is correct.

💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568224698)
I updated the tests to match the new policy. The NONSTANDARD script in p2p_segwit.py now passes AreInputsStandard since it has zero sigops, so I kept it as-is. For test_segwit_versions, I switched the input from the NONSTANDARD UTXO to the standard UTXOs created earlier, so the transaction now passes all checks and gets accepted. For feature_taproot.py, I adjusted the is_standard flag for the compat/nocsa spender because bare NONSTANDARD scripts with low sigop counts now pass policy checks.


...

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3568292879)
reACK 17cf9ff7efdbab07644fc2f9017fcac1b0757c38

Just the `POST_CHANGE_WORK` change. I ran with the changes manually a bit and logged when things were non-optimal as a sanity check (it never happened). As a follow-up, may be nice to have non-optimality logged in some sensible way for diagnostics.

`git range-diff master 3fb3c230c93e39706b813f67006ae86c9e34e803 17cf9ff7efdbab07644fc2f9017fcac1b0757c38`