Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: introduce ToBytes in bls.h and use it in all spots trivially possible, convert vecBytes to std::array #6515

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions src/bls/bls.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CBLSWrapper
explicit CBLSWrapper() = default;
explicit CBLSWrapper(Span<const unsigned char> vecBytes) : CBLSWrapper<ImplType, _SerSize, C>()
{
SetByteVector(vecBytes, bls::bls_legacy_scheme.load());
SetBytes(vecBytes, bls::bls_legacy_scheme.load());
}

CBLSWrapper(const CBLSWrapper& ref) = default;
Expand Down Expand Up @@ -103,7 +103,7 @@ class CBLSWrapper
*(static_cast<C*>(this)) = C();
}

void SetByteVector(Span<const uint8_t> vecBytes, const bool specificLegacyScheme)
void SetBytes(Span<const uint8_t> vecBytes, const bool specificLegacyScheme)
{
if (vecBytes.size() != SerSize) {
Reset();
Expand Down Expand Up @@ -136,6 +136,15 @@ class CBLSWrapper
return ToByteVector(bls::bls_legacy_scheme.load());
}

std::array<uint8_t, SerSize> ToBytes(const bool specificLegacyScheme) const
{
return impl.SerializeToArray(specificLegacyScheme);
}
std::array<uint8_t, SerSize> ToBytes() const
{
return ToBytes(bls::bls_legacy_scheme.load());
}

const uint256& GetHash() const
{
if (cachedHash.IsNull()) {
Expand All @@ -155,7 +164,7 @@ class CBLSWrapper
Reset();
return false;
}
SetByteVector(b, specificLegacyScheme);
SetBytes(b, specificLegacyScheme);
return IsValid();
}

Expand All @@ -167,7 +176,7 @@ class CBLSWrapper
template <typename Stream>
inline void Serialize(Stream& s, const bool specificLegacyScheme) const
{
s.write(AsBytes(Span{ToByteVector(specificLegacyScheme).data(), SerSize}));
s.write(AsBytes(Span{ToBytes(specificLegacyScheme).data(), SerSize}));
}

template <typename Stream>
Expand All @@ -181,12 +190,12 @@ class CBLSWrapper
{
std::array<uint8_t, SerSize> vecBytes{};
s.read(AsWritableBytes(Span{vecBytes.data(), SerSize}));
SetByteVector(vecBytes, specificLegacyScheme);
SetBytes(vecBytes, specificLegacyScheme);

if (!CheckMalleable(vecBytes, specificLegacyScheme)) {
// If CheckMalleable failed with specificLegacyScheme, we need to try again with the opposite scheme.
// Probably we received the BLS object sent with legacy scheme, but in the meanwhile the fork activated.
SetByteVector(vecBytes, !specificLegacyScheme);
SetBytes(vecBytes, !specificLegacyScheme);
if (!CheckMalleable(vecBytes, !specificLegacyScheme)) {
// Both attempts failed
throw std::ios_base::failure("malleable BLS object");
Expand All @@ -206,7 +215,7 @@ class CBLSWrapper

inline bool CheckMalleable(Span<uint8_t> vecBytes, const bool specificLegacyScheme) const
{
if (memcmp(vecBytes.data(), ToByteVector(specificLegacyScheme).data(), SerSize)) {
if (memcmp(vecBytes.data(), ToBytes(specificLegacyScheme).data(), SerSize)) {
// TODO not sure if this is actually possible with the BLS libs. I'm assuming here that somewhere deep inside
// these libs masking might happen, so that 2 different binary representations could result in the same object
// representation
Expand All @@ -222,7 +231,7 @@ class CBLSWrapper

inline std::string ToString(const bool specificLegacyScheme) const
{
std::vector<uint8_t> buf = ToByteVector(specificLegacyScheme);
auto buf = ToBytes(specificLegacyScheme);
return HexStr(buf);
}

Expand All @@ -249,6 +258,10 @@ struct CBLSIdImplicit : public uint256
{
return {begin(), end()};
}
[[nodiscard]] std::array<uint8_t, 32> SerializeToArray(const bool fLegacy) const
{
return m_data;
}
};

class CBLSId : public CBLSWrapper<CBLSIdImplicit, BLS_CURVE_ID_SIZE, CBLSId>
Expand Down Expand Up @@ -381,7 +394,7 @@ class CBLSLazyWrapper
private:
mutable std::mutex mutex;

mutable std::vector<uint8_t> vecBytes;
mutable std::array<uint8_t, BLSObject::SerSize> vecBytes;
mutable bool bufValid{false};
mutable bool bufLegacyScheme{true};

Expand All @@ -392,7 +405,7 @@ class CBLSLazyWrapper

public:
CBLSLazyWrapper() :
vecBytes(BLSObject::SerSize, 0),
vecBytes{0},
bufLegacyScheme(bls::bls_legacy_scheme.load())
{}

Expand All @@ -410,7 +423,6 @@ class CBLSLazyWrapper
if (r.bufValid) {
vecBytes = r.vecBytes;
} else {
vecBytes.resize(BLSObject::SerSize);
std::fill(vecBytes.begin(), vecBytes.end(), 0);
}
objInitialized = r.objInitialized;
Expand All @@ -433,10 +445,9 @@ class CBLSLazyWrapper
{
std::unique_lock<std::mutex> l(mutex);
if (!objInitialized && !bufValid) {
vecBytes.resize(BLSObject::SerSize);
std::fill(vecBytes.begin(), vecBytes.end(), 0);
} else if (!bufValid || (bufLegacyScheme != specificLegacyScheme)) {
vecBytes = obj.ToByteVector(specificLegacyScheme);
vecBytes = obj.ToBytes(specificLegacyScheme);
bufValid = true;
bufLegacyScheme = specificLegacyScheme;
hash.SetNull();
Expand Down Expand Up @@ -484,7 +495,7 @@ class CBLSLazyWrapper
return invalidObj;
}
if (!objInitialized) {
obj.SetByteVector(vecBytes, bufLegacyScheme);
obj.SetBytes(vecBytes, bufLegacyScheme);
if (!obj.IsValid()) {
bufValid = false;
return invalidObj;
Expand Down Expand Up @@ -518,11 +529,10 @@ class CBLSLazyWrapper
{
std::unique_lock<std::mutex> l(mutex);
if (!objInitialized && !bufValid) {
vecBytes.resize(BLSObject::SerSize);
std::fill(vecBytes.begin(), vecBytes.end(), 0);
hash.SetNull();
} else if (!bufValid) {
vecBytes = obj.ToByteVector(bufLegacyScheme);
vecBytes = obj.ToBytes(bufLegacyScheme);
bufValid = true;
hash.SetNull();
}
Expand Down
2 changes: 1 addition & 1 deletion src/governance/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ bool CGovernanceObject::Sign(const CActiveMasternodeManager& mn_activeman)
bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const
{
CBLSSignature sig;
sig.SetByteVector(m_obj.vchSig, false);
sig.SetBytes(m_obj.vchSig, false);
if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) {
LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n");
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/governance/vote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ bool CGovernanceVote::Sign(const CActiveMasternodeManager& mn_activeman)
bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const
{
CBLSSignature sig;
sig.SetByteVector(vchSig, false);
sig.SetBytes(vchSig, false);
if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), false)) {
LogPrintf("CGovernanceVote::CheckSignature -- VerifyInsecure() failed\n");
return false;
Expand Down
8 changes: 4 additions & 4 deletions src/llmq/dkgsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1016,14 +1016,14 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages, PeerManag

if (lieType == 3) {
const bool is_bls_legacy = bls::bls_legacy_scheme.load();
std::vector<uint8_t> buf = qc.sig.ToByteVector(is_bls_legacy);
auto buf = qc.sig.ToBytes(is_bls_legacy);
buf[5]++;
qc.sig.SetByteVector(buf, is_bls_legacy);
qc.sig.SetBytes(buf, is_bls_legacy);
} else if (lieType == 4) {
const bool is_bls_legacy = bls::bls_legacy_scheme.load();
std::vector<uint8_t> buf = qc.quorumSig.ToByteVector(is_bls_legacy);
auto buf = qc.quorumSig.ToBytes(is_bls_legacy);
buf[5]++;
qc.quorumSig.SetByteVector(buf, is_bls_legacy);
qc.quorumSig.SetBytes(buf, is_bls_legacy);
}

t3.stop();
Expand Down
Loading