Skip to content

refactor: optimize BLS serialization with stack-allocated arrays #6917

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+326 −17
Open

refactor: optimize BLS serialization with stack-allocated arrays #6917

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b41290
refactor: optimize BLS serialization with stack-allocated arrays
PastaPastaPasta Oct 24, 2025
ee42c15
chore: run clang-format and add trivial headers
PastaPastaPasta Oct 24, 2025
990e239
fix: use MakeUcharSpan
PastaPastaPasta Oct 26, 2025
bbca2d1
fix: proper include
PastaPastaPasta Oct 26, 2025
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
82 changes: 82 additions & 0 deletions src/bench/bls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <bench/bench.h>
#include <bls/bls_worker.h>
#include <random.h>
#include <streams.h>
#include <util/time.h>

#include <atomic>
Expand Down Expand Up @@ -358,6 +359,87 @@ static void BLS_Verify_BatchedParallel(benchmark::Bench& bench)
blsWorker.Stop();
}

static void BLS_ToBytes_Signature(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

bench.run([&] {
auto bytes = sig.ToBytes(false);
ankerl::nanobench::doNotOptimizeAway(bytes);
});
}

static void BLS_ToByteVector_Signature(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

bench.run([&] {
auto bytes = sig.ToByteVector(false);
ankerl::nanobench::doNotOptimizeAway(bytes);
});
}

static void BLS_ToBytes_PubKey(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto pk = sk.GetPublicKey();

bench.run([&] {
auto bytes = pk.ToBytes(false);
ankerl::nanobench::doNotOptimizeAway(bytes);
});
}

static void BLS_ToByteVector_PubKey(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto pk = sk.GetPublicKey();

bench.run([&] {
auto bytes = pk.ToByteVector(false);
ankerl::nanobench::doNotOptimizeAway(bytes);
});
}

static void BLS_Serialize_Signature_Array(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

bench.run([&] {
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
ss << sig;
Comment on lines +417 to +418
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing include for PROTOCOL_VERSION (bench build may fail).

CDataStream uses PROTOCOL_VERSION here, but protocol.h isn’t included in this TU.

Apply near the other headers:

 #include <streams.h>
+#include <protocol.h>

Also applies to: 430-431

🤖 Prompt for AI Agents 
In src/bench/bls.cpp around lines 417-418 (and similarly at 430-431), the code
uses PROTOCOL_VERSION via CDataStream but does not include protocol.h; add
#include "protocol.h" among the other header includes at the top of this TU so
PROTOCOL_VERSION is defined and the bench build will compile.

ankerl::nanobench::doNotOptimizeAway(ss);
});
}

static void BLS_Serialize_Signature_Vector(benchmark::Bench& bench)
{
CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

bench.run([&] {
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
auto vec = sig.ToByteVector(false);
ss << vec;
ankerl::nanobench::doNotOptimizeAway(ss);
});
}

BENCHMARK(BLS_ToBytes_Signature)
BENCHMARK(BLS_ToByteVector_Signature)
BENCHMARK(BLS_ToBytes_PubKey)
BENCHMARK(BLS_ToByteVector_PubKey)
BENCHMARK(BLS_Serialize_Signature_Array)
BENCHMARK(BLS_Serialize_Signature_Vector)
BENCHMARK(BLS_PubKeyAggregate_Normal)
BENCHMARK(BLS_SecKeyAggregate_Normal)
BENCHMARK(BLS_SignatureAggregate_Normal)
Expand Down
5 changes: 2 additions & 3 deletions src/bls/bls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ class CBLSWrapper
template <typename Stream>
inline void Serialize(Stream& s, const bool specificLegacyScheme) const
{
const auto bytes{ToBytes(specificLegacyScheme)};
s.write(AsBytes(Span{bytes.data(), SerSize}));
s.write(MakeByteSpan(ToBytes(specificLegacyScheme)));
}

template <typename Stream>
Expand Down Expand Up @@ -245,7 +244,7 @@ struct CBLSIdImplicit : public uint256
{
return {begin(), end()};
}
[[nodiscard]] std::array<uint8_t, 32> SerializeToArray(const bool fLegacy) const { return m_data; }
[[nodiscard]] std::array<uint8_t, BLS_CURVE_ID_SIZE> SerializeToArray(const bool fLegacy) const { return m_data; }
};

class CBLSId : public CBLSWrapper<CBLSIdImplicit, BLS_CURVE_ID_SIZE, CBLSId>
Expand Down
11 changes: 4 additions & 7 deletions src/bls/bls_ies.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ bool CBLSIESEncryptedBlob::Decrypt(size_t idx, const CBLSSecretKey& secretKey, C
return false;
}

std::vector<unsigned char> symKey = pk.ToByteVector(false);
symKey.resize(32);
auto symKey = pk.ToBytes(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think better to write directly type of symKey; it's not something super complicated but make code easier to read.

also consider {} initialization instead =

Copy link
Member Author

@PastaPastaPasta PastaPastaPasta Oct 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it being std::array<uint8_t, BLS_PUBLIC_KEY_SIZE> really more readable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because it's easy to see, that there's no allocation, and it's raw bytes, no any wrappers or overhead. I guess it's still subjective and anyway it's an opinion


uint256 iv = GetIV(idx);
return DecryptBlob(data.data(), data.size(), decryptedDataRet, symKey.data(), iv.begin());
Expand Down Expand Up @@ -80,8 +79,7 @@ bool CBLSIESMultiRecipientBlobs::Encrypt(size_t idx, const CBLSPublicKey& recipi
return false;
}

std::vector<uint8_t> symKey = pk.ToByteVector(false);
symKey.resize(32);
auto symKey = pk.ToBytes(false);

return EncryptBlob(blob.data(), blob.size(), blobs[idx], symKey.data(), ivVector[idx].begin());
}
Expand All @@ -97,13 +95,12 @@ bool CBLSIESMultiRecipientBlobs::Decrypt(size_t idx, const CBLSSecretKey& sk, Bl
return false;
}

std::vector<uint8_t> symKey = pk.ToByteVector(false);
symKey.resize(32);

uint256 iv = ivSeed;
for (size_t i = 0; i < idx; i++) {
iv = ::SerializeHash(iv);
}

auto symKey = pk.ToBytes(false);

return DecryptBlob(blobs[idx].data(), blobs[idx].size(), blobRet, symKey.data(), iv.begin());
}
5 changes: 3 additions & 2 deletions src/coinjoin/coinjoin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <coinjoin/common.h>

#include <bls/bls.h>
#include <core_io.h>
#include <netaddress.h>
#include <primitives/block.h>
Expand Down Expand Up @@ -182,7 +183,7 @@ class CCoinJoinQueue
uint256 m_protxHash;
int64_t nTime{0};
bool fReady{false}; //ready for submit
std::vector<unsigned char> vchSig;
std::array<uint8_t, BLS_CURVE_SIG_SIZE> vchSig{};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mixing fails with CCoinJoinQueue::CheckSignature -- VerifyInsecure() failed - should change serialization accordingly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it fail?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because array and vector serializations do not match (no compact size for array).

// memory only
bool fTried{false};

Expand Down Expand Up @@ -235,7 +236,7 @@ class CCoinJoinBroadcastTx
CTransactionRef tx;
COutPoint masternodeOutpoint;
uint256 m_protxHash;
std::vector<unsigned char> vchSig;
std::array<uint8_t, BLS_CURVE_SIG_SIZE> vchSig{};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

int64_t sigTime{0};
CCoinJoinBroadcastTx() :
tx(MakeTransactionRef(CMutableTransaction{}))
Expand Down
2 changes: 1 addition & 1 deletion src/governance/vote.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class CGovernanceVote
UpdateHash();
}

void SetSignature(const std::vector<unsigned char>& vchSigIn) { vchSig = vchSigIn; }
void SetSignature(Span<const uint8_t> vchSigIn) { vchSig.assign(vchSigIn.begin(), vchSigIn.end()); }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so basically back to ToByteVector results


bool CheckSignature(const CKeyID& keyID) const;
bool CheckSignature(const CBLSPublicKey& pubKey) const;
Expand Down
4 changes: 2 additions & 2 deletions src/masternode/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,12 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec
return WITH_READ_LOCK(cs, return m_info.blsKeyOperator.Sign(hash, is_legacy));
}

[[nodiscard]] std::vector<uint8_t> CActiveMasternodeManager::SignBasic(const uint256& hash) const
[[nodiscard]] std::array<uint8_t, BLS_CURVE_SIG_SIZE> CActiveMasternodeManager::SignBasic(const uint256& hash) const
{
AssertLockNotHeld(cs);
auto sig = Sign(hash, /*is_legacy=*/false);
assert(sig.IsValid());
return sig.ToByteVector(/*specificLegacyScheme=*/false);
return sig.ToBytes(/*specificLegacyScheme=*/false);
}

// We need to pass a copy as opposed to a const ref because CBLSPublicKeyVersionWrapper
Expand Down
2 changes: 1 addition & 1 deletion src/masternode/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class CActiveMasternodeManager
[[nodiscard]] bool Decrypt(const EncryptedObj<Obj>& obj, size_t idx, Obj& ret_obj, int version) const
EXCLUSIVE_LOCKS_REQUIRED(!cs);
[[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool is_legacy) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
[[nodiscard]] std::vector<uint8_t> SignBasic(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
[[nodiscard]] std::array<uint8_t, BLS_CURVE_SIG_SIZE> SignBasic(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);

/* TODO: Reconsider external locking */
[[nodiscard]] COutPoint GetOutPoint() const { READ_LOCK(cs); return m_info.outpoint; }
Expand Down
51 changes: 51 additions & 0 deletions src/serialize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,12 @@ template<typename Stream, unsigned int N, typename T> inline void Unserialize(St
template<typename Stream, typename T, typename A> inline void Serialize(Stream& os, const std::vector<T, A>& v);
template<typename Stream, typename T, typename A> inline void Unserialize(Stream& is, std::vector<T, A>& v);

/**
* array
*/
template<typename Stream, typename T, size_t N> void Serialize(Stream& os, const std::array<T, N>& a);
template<typename Stream, typename T, size_t N> void Unserialize(Stream& is, std::array<T, N>& a);

/**
* pair
*/
Expand Down Expand Up @@ -1069,6 +1075,51 @@ void Unserialize(Stream& is, std::vector<T, A>& v)
}
}

/**
* array
*/
template<typename Stream, typename T, size_t N>
void Serialize(Stream& os, const std::array<T, N>& a)
{
if constexpr (std::is_same_v<T, unsigned char>) {
// Directly write the byte data without writing the size
if constexpr (N > 0) {
os.write(MakeByteSpan(a));
}
} else if constexpr (std::is_same_v<T, bool>) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need a case of std::array<bool, N> ? I don't see any usage in code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to just keep compatibility with the vector api.

// Serialize each bool individually
for (const bool& elem : a) {
::Serialize(os, elem);
}
} else {
// Serialize each element using the default Serialize function
for (const T& elem : a) {
::Serialize(os, elem);
}
}
}

template<typename Stream, typename T, size_t N>
void Unserialize(Stream& is, std::array<T, N>& a)
{
if constexpr (std::is_same_v<T, unsigned char>) {
// Directly read the byte data without reading the size
if constexpr (N > 0) {
is.read(AsWritableBytes(Span{a}));
}
} else if constexpr (std::is_same_v<T, bool>) {
// Unserialize each bool individually
for (bool& elem : a) {
::Unserialize(is, elem);
}
} else {
// Unserialize each element using the default Unserialize function
for (T& elem : a) {
::Unserialize(is, elem);
}
}
}


/**
* pair
Expand Down
82 changes: 82 additions & 0 deletions src/test/bls_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,4 +593,86 @@ BOOST_AUTO_TEST_CASE(test_get_hash_consistency)
BOOST_CHECK(hash1 == hash2);
}

BOOST_AUTO_TEST_CASE(bls_tobytes_vs_tobytevector_signature)
{
// Test that ToBytes() and ToByteVector() produce identical results for signatures
bls::bls_legacy_scheme.store(false);

CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

auto bytes_array = sig.ToBytes(false);
auto bytes_vector = sig.ToByteVector(false);

BOOST_CHECK_EQUAL(bytes_array.size(), BLS_CURVE_SIG_SIZE);
BOOST_CHECK_EQUAL(bytes_vector.size(), BLS_CURVE_SIG_SIZE);
BOOST_CHECK(std::equal(bytes_array.begin(), bytes_array.end(), bytes_vector.begin()));
}

BOOST_AUTO_TEST_CASE(bls_tobytes_vs_tobytevector_pubkey)
{
// Test that ToBytes() and ToByteVector() produce identical results for public keys
bls::bls_legacy_scheme.store(false);

CBLSSecretKey sk;
sk.MakeNewKey();
auto pk = sk.GetPublicKey();

auto bytes_array = pk.ToBytes(false);
auto bytes_vector = pk.ToByteVector(false);

BOOST_CHECK_EQUAL(bytes_array.size(), BLS_CURVE_PUBKEY_SIZE);
BOOST_CHECK_EQUAL(bytes_vector.size(), BLS_CURVE_PUBKEY_SIZE);
BOOST_CHECK(std::equal(bytes_array.begin(), bytes_array.end(), bytes_vector.begin()));
}

BOOST_AUTO_TEST_CASE(bls_tobytes_vs_tobytevector_seckey)
{
// Test that ToBytes() and ToByteVector() produce identical results for secret keys
bls::bls_legacy_scheme.store(false);

CBLSSecretKey sk;
sk.MakeNewKey();

auto bytes_array = sk.ToBytes(false);
auto bytes_vector = sk.ToByteVector(false);

BOOST_CHECK_EQUAL(bytes_array.size(), BLS_CURVE_SECKEY_SIZE);
BOOST_CHECK_EQUAL(bytes_vector.size(), BLS_CURVE_SECKEY_SIZE);
BOOST_CHECK(std::equal(bytes_array.begin(), bytes_array.end(), bytes_vector.begin()));
}

BOOST_AUTO_TEST_CASE(bls_signature_array_serialization)
{
// Test that BLS signatures serialize correctly with std::array
bls::bls_legacy_scheme.store(false);

CBLSSecretKey sk;
sk.MakeNewKey();
auto sig = sk.Sign(uint256::ONE, false);

// Serialize using standard serialization
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
ss << sig;

// Deserialize
CBLSSignature sig2;
ss >> sig2;

BOOST_CHECK(sig == sig2);

// Verify the serialized size is exactly BLS_CURVE_SIG_SIZE (no size prefix)
CDataStream ss2(SER_NETWORK, PROTOCOL_VERSION);
ss2 << sig;
BOOST_CHECK_EQUAL(ss2.size(), BLS_CURVE_SIG_SIZE);

// Test array assignment from ToBytes
std::array<uint8_t, BLS_CURVE_SIG_SIZE> sig_array = sig.ToBytes(false);

// Construct signature from array via Span
CBLSSignature sig3(Span{sig_array}, false);
BOOST_CHECK(sig == sig3);
}

BOOST_AUTO_TEST_SUITE_END()
Loading
Loading