The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

The long-term idea is to move from uint8_t to std::byte everywhere?

I believe that the rationale behind std::byte in the C++ standard is having a type that represents pure "memory units" for storage, without associated arithmetic. So I think it makes sense to start adopting those in our codebase whenever char/uchar are used in places that don't actually treat them as numbers or as characters of a string.

The long-term idea is to move from uint8_t to std::byte everywhere?

Yes, in places where raw bytes are handled. Though, this is mostly a style question, so there is no rush or need to switch existing code. For example, #23438 updates serialize to use Spans, and updating to std::byte at the same time was "free".

Concept ACK

in the C++ standard is having a type that represents pure "memory units" for storage, without associated arithmetic.

Ok. I see. It would be incredibly difficult to port bitcoin to a system that doesn't have 8-bit memory units. uint8_t just makes sense to me in that regard.
But no problem with making some code more general when it comes for free.

@laanwj I don't think the point is generality; we very much assume that a byte is 8 bits all over the place. It's more a type safety thing: std::byte indicates we're just talking about bytes, and not about numbers. They don't automatically get converted to ints or bools, for example.

It's more a type safety thing: std::byte indicates we're just talking about bytes, and not about numbers

Thanks for the explanation. Yes a "memory area without interpretation" abstraction makes sense. I still have a hard time grasping when to use it though. We might want to add something to developer-notes.md for guidance.
(e.g. if it requires explicit casts everywhere, we don't want to use it in cases where the memory is in fact interpreted and treated as numbers, it would just add more boilerplate)

I think a good rule of thumb would be to use it only for "raw memory" or "serialized memory". That is, any kind of memory that needs to be passed through a deserializer/parser before being useful.

For example: #23438 changes the serialize framework.

ACK facd1fb

Does it make sense to mention this specifically in the Developer notes when to use std::byte?
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures

Yeah, I am happy to review a pull request that changes the dev notes, though it seems unrelated to this change here, since we already use std::byte extensively in existing code, so likely I won't be changing the docs in this pull.

Code review ACK facd1fb

Post-merge ACK facd1fb