💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750421415)
Yes, that's what I tried at first, but I don't yet see how that would be possible without changing `prevector`, as explained [here](https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747621990)

💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750424687)
Related to https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1748936794, will see what I can do to document it better. I guess the explanation should rather go to the commit message instead of the code, since the code itself isn't doing anything special.

🤔 ryanofsky reviewed a pull request: "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`"
(https://github.com/bitcoin/bitcoin/pull/30765#pullrequestreview-2290235809)
I think this PR has been a good exploration of ways to let CScript support std::span and std::byte but current approach is a more complicated than it needs to be. I think it's be worth considering a more minimal, focused change:

```diff
--- a/src/script/script.h
+++ b/src/script/script.h
@@ -463,7 +463,7 @@ public:
return *this;
}

- CScript& operator<<(const std::vector<unsigned char>& b) LIFETIMEBOUND
+ CScript& operator<<(std::span<const std::byte> b) LIFETIMEB
...

💬 ryanofsky commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750460628)
> Yes, that's what I tried at first, but I don't yet see how that would be possible without changing `prevector`, as explained [here](https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1747621990)

Thanks that's helpful to know, and it's clear why changing prevector could fix this but not clear why it would be necessary to change prevector to fix that as opposed to just passing a start and end pointer of the type it is expecting

💬 fanquake commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2338464097)
Thought this might just be constrained to that job, but:
ASAN (currently running PR): [`Total Test time (real) = 1068.87 sec`](https://github.com/bitcoin/bitcoin/actions/runs/10775638367/job/29880507688?pr=30791#step:7:4709)
ASAN (master finished 23 minutes ago): [`Total Test time (real) = 498.22 sec`](https://github.com/bitcoin/bitcoin/actions/runs/10775288903/job/29879351290#step:7:4709).

💬 achow101 commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2338471211)
Is the cause for the speedup a difference in the actual hardware are simply due to dedicated resources?

I currently have some older server hardware that I am planning to dedicate to fuzzing. I could instead also set them up for our CI. These servers will live in a data center and should have 100% uptime.

🤔 mzumsande reviewed a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2290116946)
Concept ACK

I think the description of the first commit could be improved:
> AssumeUTXO nodes prioritize tip synchronization first, meaning they advertise
knowledge of all blocks posterior to the snapshot base block hash.

This is confusing to me - nodes advertise `NODE_NETWORK` (all historical blocks prior to tip) or `NODE_NETWORK_LIMITED` (last 288 from tip). They also communicate their current tip with peers, but it's not part of the advertisement. So this includes (amongst others
...

💬 mzumsande commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#discussion_r1750386961)
whitespace nit: `& ~services` makes more sense to me, and clang-format agrees.

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2338512812)
I haven't done the benchmarks here, so I can't say for sure, but the `AMD EPYC™ 9454P` all-core boost clock is lower than the base clock of the `AMD Ryzen™ 7 7700` that @willcl-ark did the benchmark on. So I guess it is a combination of faster clock + dedicated resources + more idle CPU waiting for work on the server?

If your older hardware has about 90 CPU threads, about 152 GB of RAM, and a 3 TB of SSD, and has comparable performance, it could also be used.

I think you can compare the pe
...

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2338518268)
Asan is running on GHA infra, so this highlights that the issue may not be infra related, but related to the code, config or build files in this repo?

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2338522744)
reACK 59028de422bbf8ccf53da27f1153e343952e585f

via `git range-diff master 100e1b9ecb29c76187112fda9fed1c01da192c99 59028de422bbf8ccf53da27f1153e343952e585f`

fwiw I ran the branch with the two dropped optimizations, and it lead to a slight improvement to most of the optimal benchmarks on my machine

💬 m3dwards commented on issue "Trying to run bitcoin qt on Windows and getting an AV":
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2338560259)
So far unable to recreate on Win 11. Installing the 23H2 update and will try again.

🚀 achow101 merged a pull request: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401)

💬 achow101 commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#issuecomment-2338581569)
ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef

🚀 achow101 merged a pull request: "init: fix init fatal error on invalid negated option value"
(https://github.com/bitcoin/bitcoin/pull/30684)

💬 achow101 commented on pull request "test: Add coverage for dumptxoutset failure robustness":
(https://github.com/bitcoin/bitcoin/pull/30817#issuecomment-2338601019)
ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7

🚀 achow101 merged a pull request: "test: Add coverage for dumptxoutset failure robustness"
(https://github.com/bitcoin/bitcoin/pull/30817)

📝 l0rinc converted_to_draft a pull request: "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors"
(https://github.com/bitcoin/bitcoin/pull/30765)
Split out of https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722326803.

Extracted existing serialization to append size & data in separate private methods to clarify that it does more than just a simple data insertion.

This enables using `CScript() << "..."_hex_u8` (followed by `CScript() << "..."_hex`), skipping vector conversion before serializing to the prevector in CScript.

Originally I've added separate `std::vector` and `std::array` overloads, but based on the comments
...

⚠️ rkrux opened an issue: "28.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/30854)
This issue is to discuss the [28.0 Release Candidate Testing Guide](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Candidate-Testing-Guide). If you have any feedback on the document, please leave a comment here.

Note: This is for feedback on the document, not on Bitcoin Core or on the 28.0 changes.

Thank you for taking a look at the guide and leaving your feedback.

💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750661750)
You're right, fixed. This also made me realize that I don't really need the loop. But I'm thinking to do a followup where it would build on the chain of 16 headers, as opposed to just sending 16 headers one time and resetting. So I think I'll keep the loop there for now, as it shouldn't make a difference as far as I'm aware.