refactor: Allow std::span construction from CKey #29133

Is is possible to construct a Span from a reference to a CKey. However, the same is not possible with std::span.

Fix that.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage

For detailed information about the code coverage, see the test coverage report.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #28774 (Avoid returning references to mutex guarded members by vasild)
• #28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

<!--9cd9c72976c961c55c7acef8f6ba82cd-->

Guix builds (on x86_64)

Code Review ACK

<details> <summary> <b>Notes:</b> </summary>

Commit-1: Since std::span requires a mutable pointer to the data for certain operations, this change makes XOnlyPubKey compatible with std::span.

Commit-2: Since the return type of begin() and end() much match the data type of the std::span of the container, this change makes CKey compatible with std::span, and allows std::span<std::byte> construction for it.

Commit-3: Tests that the above changes made CKey compatible with std::span.

</details>

I am curious about one thing, though.

If we now have std::span since C++20, are we thinking of making every container in the codebase std::span compatible and moving away from needing to use our ### custom Span class?

If we now have std::span since C++20, are we thinking of making every container in the codebase std::span compatible and moving away from needing to use our ### custom Span class?

Yes, I think it makes sense to allow new code to use std::span directly. Also, I think it makes sense to (longer term) remove Span (or make it an alias of std::span) to avoid confusion which to use in new code, and to get the additional compile-time checks from std::span.

I dropped the last commit, to not introduce new std::span usage for now. To verify the changes in this pull, the commit's diff can be used. (Fails to compile before, compiles after):

diff --git a/src/key.cpp b/src/key.cpp
index 2bd6396298..300c95e35a 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -385,7 +385,7 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const {
     return key.Derive(out.key, out.chaincode, _nChild, chaincode);
 }
 
-void CExtKey::SetSeed(Span<const std::byte> seed)
+void CExtKey::SetSeed(std::span<const std::byte> seed)
 {
     static const unsigned char hashkey[] = {'B','i','t','c','o','i','n',' ','s','e','e','d'};
     std::vector<unsigned char, secure_allocator<unsigned char>> vout(64);
diff --git a/src/key.h b/src/key.h
index d6b26f891d..d63ec6c1cc 100644
--- a/src/key.h
+++ b/src/key.h
@@ -12,10 +12,10 @@
 #include <support/allocators/secure.h>
 #include <uint256.h>
 
+#include <span>
 #include <stdexcept>
 #include <vector>
 
-
 /**
  * CPrivKey is a serialized private key, with all parameters included
  * (SIZE bytes)
@@ -227,7 +227,7 @@ struct CExtKey {
     void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]);
     [[nodiscard]] bool Derive(CExtKey& out, unsigned int nChild) const;
     CExtPubKey Neuter() const;
-    void SetSeed(Span<const std::byte> seed);
+    void SetSeed(std::span<const std::byte> seed);
 };
 
 /** Initialize the elliptic curve support. May not be called twice without calling ECC_Stop first. */

ReACK fa96d937116682f32613d31a3ae7d6f652e8146d

Changes since my last review

• Removed the last commit, which was temporarily introduced to test the workings of changes in this commit.
• Tested that the above patch leads to failure in master, while successful compilation with this PR.

ACK fa96d937116682f32613d31a3ae7d6f652e8146d

A nice simple change to enable using std::span with CKey.

The PR description could be updated now that the verification commit has been removed.