💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3279713173)
I did trial freeing up space on the runner as suggested by @maflcko in #33293, and it is possible to free up 18GB realtively easily, using e.g. https://github.com/willcl-ark/bitcoin/blob/f5885d05160fe0ac167976f2b36b6d5bec373e08/.github/actions/clear-files/action.yml

But a one-line disable feels much cleaner, and also removes us from maintaing code in this repo, which isn't for our own use.

👍 willcl-ark approved a pull request: "ci: always use tag for LLVM checkout"
(https://github.com/bitcoin/bitcoin/pull/33364#pullrequestreview-3210692825)
ACK b736052e39f1f466f63f261ace3dd2deba171e8a

I did wonder about defining in the *.env files, something like:

```patch
diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh
index c71772b8e2c..47497ed4c81 100755
--- a/ci/test/00_setup_env_native_fuzz_with_msan.sh
+++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh
@@ -26,6 +26,7 @@ export BITCOIN_CONFIG="\
-DAPPEND_CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE' \
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3280041954)
`4327327dd0...3f6a61e3c5`: rebase due to conflicts and take https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2328909174

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2340272853)
Done.

💬 willcl-ark commented on pull request "Run feature_bind_port_(discover|externalip).py in CI":
(https://github.com/bitcoin/bitcoin/pull/33362#discussion_r2340540732)
What is the effect on a developer running CI locally on their machine, of adding NET_ADMIN capability? Is is possible for the CI to then modify/break the developer machine networking?

CAP_NET_ADMIN
Perform various network-related operations:
• interface configuration;
• administration of IP firewall, masquerading, and
accounting;
• modify routing tables;
• bind to any address for transparen
...

💬 vasild commented on pull request "Run feature_bind_port_(discover|externalip).py in CI":
(https://github.com/bitcoin/bitcoin/pull/33362#discussion_r2340774782)
The privileges only apply to the guest, not to host. The guest ("container" in docker language) has no way to manipulate the host's networking.

https://docs.docker.com/engine/containers/run/#container-networking
https://docs.docker.com/engine/network/

> We use our own `--network` in CI ...

Hmm. Maybe that could be an easier way to add the dummy routable addresses. Then they will be available for the whole duration of the entire CI run, not only during the functional tests as in this PR
...

💬 instagibbs commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2340792959)
double checking this will happen IFF there's as siphash collision, i.e., on average every ~2^32 sequence comparisons?

💬 instagibbs commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2340812332)
eyeballing this, I don't see this being initialized?

💬 instagibbs commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#issuecomment-3280624138)
concept ACK, I also think it removes the kinda weird behavior where same CFR clusters get mined/evicted based on the underlying cluster's original sequence value

⚠️ fanquake opened an issue: "oss-fuzz: build failing"
(https://github.com/bitcoin/bitcoin/issues/33366)
https://oss-fuzz-build-logs.storage.googleapis.com/log-08166704-3ba3-406e-9e55-637c906d1fd2.txt:
```bash
C++ compiler .......................... Clang 22.0.0, /src/aflplusplus/afl-clang-fast++
<snip>
Step #3 - "compile-afl-address-x86_64": [100%] [32m [1mLinking CXX executable ../../../bin/fuzz [0m
Step #3 - "compile-afl-address-x86_64": `.text.asan.module_ctor.45' referenced in section `.init_array.1[asan.module_ctor.45]' of /tmp/lto-llvm-b064c1.o: defined in discarded section `.text.asan.modu
...

✅ darosior closed a pull request: "script: return verification flag responsible for error upon validation failure"
(https://github.com/bitcoin/bitcoin/pull/33012)

💬 darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3280825306)
There may be use cases for better reporting independently of reducing unconfirmed transactions validation time, but i'm not going to make a priority of working on those. Closing for now.

💬 darosior commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#issuecomment-3280838227)
I would be a nice cleanup to do #33066 and then this PR, but i won't make a priority of working on it. Closing for now.

✅ darosior closed a pull request: "p2p: stop special-casing witness-stripped error for unconfirmed transactions"
(https://github.com/bitcoin/bitcoin/pull/32379)

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3280976957)
My goal with this PR was making the bulk of `ConnectBlock` logic work without using a `CCoinsViewCache` by moving the call to `UpdateCoins` out of the function. The side effect of that is that the `CBlockUndo` is populated outside of `ConnectBlock` too and has to be passed in. Since it already holds all the required coins, I rather took those coins vectors, than additionally requiring a view providing the same data (next to the reasons I provided in my comment above).

The change was motivated
...

🤔 glozow reviewed a pull request: "txgraph: use enum Level instead of bool main_only"
(https://github.com/bitcoin/bitcoin/pull/33354#pullrequestreview-3212180028)
code review ACK d45f3717d2c65d1a6012a4bc2f47ff75004fd171

💬 hebasto commented on issue "oss-fuzz: build failing":
(https://github.com/bitcoin/bitcoin/issues/33366#issuecomment-3281275004)
It seems caused by switching from Clang 18.1.8 to Clang 22.0.0.

💬 fjahr commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#discussion_r2341359835)
IMO it is not surprising that tests didn't need to be updated because this is a refactor. For pure refactors that don't change behavior it is normal to not add/edit tests.

Furthermore, trivially copyable is a compile-time structural property that affects ABI/optimization, not behavior. So we aren't changing something that could change behavior in this PR. FWIW, these lines were covered by unit tests, see the coverage reports: https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/arith_u
...

💬 fjahr commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#discussion_r2341360204)
It is implicitly now could not be in the future if there are changes made to the class. So it might be better to let the compiler choose `noexcept` conditionally based on what the class looks like in the future.

I would be fine with removing the default declarations like in the original version of the PR but it seems to me so far more reviewers were in favor of them, so keeping them for now.

If others reviewers are convinced by @l0rinc 's arguments and now in favor of removing, please comm
...

💬 fjahr commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3281328737)
Addressed the comments, thanks for running the benchmarks @l0rinc , it's good to see that the performance improvement can be demonstrated.