refactor: use Span in random.* #24213

eeae84c5eb missing span headers here in src/dbwrapper.cpp and in src/key.cpp, src/random.h, src/rpc/request.cpp, maybe reverify if didn't miss any others

Should it really be included in files like dbwrapper.cpp and key.cpp and request.cpp? It seems to be that it makes more sense to just explicitly include it in src/random.h.

If this is in developer notes or a strong opinion of yours, I'll happily do it, but seems better to just do it in random.h (applied and squashed random.h include)

Yes, the developer notes specify the guideline and reason:

- Every `.cpp` and `.h` file should `#include` every header file it directly uses classes, functions or other
  definitions from, even if those headers are already included indirectly through other headers.

  - *Rationale*: Excluding headers because they are already indirectly included results in compilation
    failures when those indirect dependencies change. Furthermore, it obscures what the real code
    dependencies are.

I agree with the rule, but I'm not sure I agree with the application. I'm not really "using" span in these cpp files, I'm simply passing something to a function. If we change random.h to not use span, then we need to change the calling files anyway.

My local CLion for instance sees these includes are completely unused when I add them, so I see no reason to add them right now

Oh, you're right, now that you've added the header to random.h. Though key.cpp uses Span (not in your changes), so could add the header in 32b6173e30b514437f6b16aed261d118e416ae6e if you're so inclined. Otherwise LGTM.

d35019566a

    const uint16_t randv{GetRand<uint16_t>()};

applied and squashed

d35019566a what is the reason for the do/while style change?

It avoids a redundant assignment, and I think it makes more sense, although I would be happy to revert this change if you would strongly want that

I wouldn't needlessly change it unless other reviewers prefer the change, but no strong opinion.

I think the old code is slightly more elegant. There's no need to distinguish the first iteration here, so let's not.

This can probably use the exiting native chrono random stuff.

Further, I am wondering if it makes sense to add a FastRandomContext member to connman and use that for all noise generation instead of generating a new one each time. A commit like fa73e9d781d4898dbfd960aab997d99c62bc01ed can then be used to provide the chrono random stuff.

Maybe the fast context could then also be used to remove those args?

I wonder if it makes sense to use a single FastRandomContext member for all entropy drawn inside PeerManagerImpl, instead of creating a new context for each call.