refactor: Allow std::span construction from CKey #29133
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee 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. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
DrahtBot
changed the title
maflcko
force-pushed
the
2312-key-span-
branch
from
fa3f19f
to
fa9d23f
Compare
There was a problem hiding this comment.
Code Review ACK
Notes:
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.
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?
Yes, I think it makes sense to allow new code to use |
This is needed for consistency, and also to allow std::span construction from XOnlyPubKey.
maflcko
force-pushed
the
2312-key-span-
branch
2 times, most recently
from
fae4d89
to
84b2dfb
Compare
maflcko
force-pushed
the
2312-key-span-
branch
from
84b2dfb
to
fa96d93
Compare
|
I dropped the last commit, to not introduce new 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. */ |
There was a problem hiding this comment.
ACK fa96d93
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.
Is is possible to construct a
Spanfrom a reference to aCKey. However, the same is not possible withstd::span.Fix that.