Skip to content

Commit

Permalink
refactor: enforce passing bls scheme for each call of sign by BLS
Browse files Browse the repository at this point in the history
  • Loading branch information
knst committed Dec 26, 2024
1 parent 7f212db commit 59d13ab
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 53 deletions.
10 changes: 5 additions & 5 deletions src/bench/bls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ static void BuildTestVectors(size_t count, size_t invalidCount,
secKeys[i].MakeNewKey();
pubKeys[i] = secKeys[i].GetPublicKey();
msgHashes[i] = GetRandHash();
sigs[i] = secKeys[i].Sign(msgHashes[i]);
sigs[i] = secKeys[i].Sign(msgHashes[i], false);

if (invalid[i]) {
CBLSSecretKey s;
s.MakeNewKey();
sigs[i] = s.Sign(msgHashes[i]);
sigs[i] = s.Sign(msgHashes[i], false);
}
}
}
Expand Down Expand Up @@ -71,8 +71,8 @@ static void BLS_SignatureAggregate_Normal(benchmark::Bench& bench)
CBLSSecretKey secKey1, secKey2;
secKey1.MakeNewKey();
secKey2.MakeNewKey();
CBLSSignature sig1 = secKey1.Sign(hash);
CBLSSignature sig2 = secKey2.Sign(hash);
CBLSSignature sig1 = secKey1.Sign(hash, false);
CBLSSignature sig2 = secKey2.Sign(hash, false);

// Benchmark.
bench.run([&] {
Expand All @@ -89,7 +89,7 @@ static void BLS_Sign_Normal(benchmark::Bench& bench)
// Benchmark.
bench.minEpochIterations(100).run([&] {
uint256 hash = GetRandHash();
sig = secKey.Sign(hash);
sig = secKey.Sign(hash, false);
});
}

Expand Down
5 changes: 0 additions & 5 deletions src/bls/bls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,6 @@ CBLSPublicKey CBLSSecretKey::GetPublicKey() const
return pubKey;
}

CBLSSignature CBLSSecretKey::Sign(const uint256& hash) const
{
return Sign(hash, bls::bls_legacy_scheme.load());
}

CBLSSignature CBLSSecretKey::Sign(const uint256& hash, const bool specificLegacyScheme) const
{
if (!IsValid()) {
Expand Down
1 change: 0 additions & 1 deletion src/bls/bls.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ class CBLSSecretKey : public CBLSWrapper<bls::PrivateKey, BLS_CURVE_SECKEY_SIZE,
bool SecretKeyShare(Span<CBLSSecretKey> msk, const CBLSId& id);

[[nodiscard]] CBLSPublicKey GetPublicKey() const;
[[nodiscard]] CBLSSignature Sign(const uint256& hash) const;
[[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool specificLegacyScheme) const;
};

Expand Down
2 changes: 1 addition & 1 deletion src/bls/bls_worker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ bool CBLSWorker::VerifyVerificationVectors(Span<BLSVerificationVectorPtr> vvecs)
void CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash, const CBLSWorker::SignDoneCallback& doneCallback)
{
workerPool.push([secKey, msgHash, doneCallback](int threadId) {
doneCallback(secKey.Sign(msgHash));
doneCallback(secKey.Sign(msgHash, bls::bls_legacy_scheme.load()));
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/evo/mnauth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CActiveMasternode

mnauth.proRegTxHash = mn_activeman.GetProTxHash();

mnauth.sig = mn_activeman.Sign(signHash);
mnauth.sig = mn_activeman.Sign(signHash, bls::bls_legacy_scheme.load());

LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
connman.PushMessage(&peer, CNetMsgMaker(peer.GetCommonVersion()).Make(NetMsgType::MNAUTH, mnauth));
Expand Down
24 changes: 12 additions & 12 deletions src/llmq/dkgsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ CDKGSession::CDKGSession(const CBlockIndex* pQuorumBaseBlockIndex, const Consens
m_mn_metaman(mn_metaman),
m_mn_activeman(mn_activeman),
m_sporkman(sporkman),
m_quorum_base_block_index{pQuorumBaseBlockIndex}
m_quorum_base_block_index{pQuorumBaseBlockIndex},
m_use_legacy_bls{!DeploymentActiveAfter(m_quorum_base_block_index, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}
{
}

Expand Down Expand Up @@ -215,7 +216,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages, PeerMa

logger.Batch("encrypted contributions. time=%d", t1.count());

qc.sig = m_mn_activeman->Sign(qc.GetSignHash());
qc.sig = m_mn_activeman->Sign(qc.GetSignHash(), m_use_legacy_bls);

logger.Flush();

Expand Down Expand Up @@ -527,7 +528,7 @@ void CDKGSession::SendComplaint(CDKGPendingMessages& pendingMessages, PeerManage

logger.Batch("sending complaint. badCount=%d, complaintCount=%d", badCount, complaintCount);

qc.sig = m_mn_activeman->Sign(qc.GetSignHash());
qc.sig = m_mn_activeman->Sign(qc.GetSignHash(), m_use_legacy_bls);

logger.Flush();

Expand Down Expand Up @@ -721,7 +722,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, PeerMa
return;
}

qj.sig = m_mn_activeman->Sign(qj.GetSignHash());
qj.sig = m_mn_activeman->Sign(qj.GetSignHash(), m_use_legacy_bls);

logger.Flush();

Expand Down Expand Up @@ -1011,19 +1012,18 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages, PeerManag
(*commitmentHash.begin())++;
}

qc.sig = m_mn_activeman->Sign(commitmentHash);
qc.quorumSig = skShare.Sign(commitmentHash);
const bool use_legacy_bls = m_use_legacy_bls;
qc.sig = m_mn_activeman->Sign(commitmentHash, use_legacy_bls);
qc.quorumSig = skShare.Sign(commitmentHash, bls::bls_legacy_scheme.load());

if (lieType == 3) {
const bool is_bls_legacy = bls::bls_legacy_scheme.load();
std::vector<uint8_t> buf = qc.sig.ToByteVector(is_bls_legacy);
std::vector<uint8_t> buf = qc.sig.ToByteVector(use_legacy_bls);
buf[5]++;
qc.sig.SetByteVector(buf, is_bls_legacy);
qc.sig.SetByteVector(buf, use_legacy_bls);
} 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);
std::vector<uint8_t> buf = qc.quorumSig.ToByteVector(bls::bls_legacy_scheme.load());
buf[5]++;
qc.quorumSig.SetByteVector(buf, is_bls_legacy);
qc.quorumSig.SetByteVector(buf, bls::bls_legacy_scheme.load());
}

t3.stop();
Expand Down
1 change: 1 addition & 0 deletions src/llmq/dkgsession.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ class CDKGSession
const CSporkManager& m_sporkman;

const CBlockIndex* const m_quorum_base_block_index;
bool m_use_legacy_bls;
int quorumIndex{0};

private:
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing_shares.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
uint256 signHash = sigShare.buildSignHash();

sigShare.sigShare.Set(skShare.Sign(signHash), bls::bls_legacy_scheme.load());
sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load());
if (!sigShare.sigShare.Get().IsValid()) {
LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", __func__,
signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), t.count());
Expand Down
6 changes: 0 additions & 6 deletions src/masternode/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,6 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESEncryptedObject<CBL
template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjects<CBLSSecretKey>& obj, size_t idx,
CBLSSecretKey& ret_obj, int version) const;

[[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash) const
{
AssertLockNotHeld(cs);
return WITH_READ_LOCK(cs, return m_info.blsKeyOperator.Sign(hash));
}

[[nodiscard]] CBLSSignature CActiveMasternodeManager::Sign(const uint256& hash, const bool is_legacy) const
{
AssertLockNotHeld(cs);
Expand Down
1 change: 0 additions & 1 deletion src/masternode/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class CActiveMasternodeManager final : public CValidationInterface
template <template <typename> class EncryptedObj, typename Obj>
[[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 EXCLUSIVE_LOCKS_REQUIRED(!cs);
[[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool is_legacy) const EXCLUSIVE_LOCKS_REQUIRED(!cs);

/* TODO: Reconsider external locking */
Expand Down
11 changes: 6 additions & 5 deletions src/rpc/evo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,14 @@ static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxP
}
}

template<typename SpecialTxPayload>
static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload, const CBLSSecretKey& key)
template <typename SpecialTxPayload>
static void SignSpecialTxPayloadByHash(const CMutableTransaction& tx, SpecialTxPayload& payload,
const CBLSSecretKey& key, bool use_legacy)
{
UpdateSpecialTxInputsHash(tx, payload);

uint256 hash = ::SerializeHash(payload);
payload.sig = key.Sign(hash);
payload.sig = key.Sign(hash, use_legacy);
}

static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, CChainstateHelper& chain_helper, const ChainstateManager& chainman, const CMutableTransaction& tx, bool fSubmit = true)
Expand Down Expand Up @@ -1054,7 +1055,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques

FundSpecialTx(*wallet, tx, ptx, feeSource);

SignSpecialTxPayloadByHash(tx, ptx, keyOperator);
SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
SetTxPayload(tx, ptx);

return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
Expand Down Expand Up @@ -1254,7 +1255,7 @@ static RPCHelpMan protx_revoke()
throw JSONRPCError(RPC_INTERNAL_ERROR, "No payout or fee source addresses found, can't revoke");
}

SignSpecialTxPayloadByHash(tx, ptx, keyOperator);
SignSpecialTxPayloadByHash(tx, ptx, keyOperator, !isV19active);
SetTxPayload(tx, ptx);

return SignAndSendSpecialTx(request, chain_helper, chainman, tx);
Expand Down
23 changes: 12 additions & 11 deletions src/test/bls_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ void FuncSign(const bool legacy_scheme)
uint256 msgHash1 = uint256::ONE;
uint256 msgHash2 = uint256::TWO;

auto sig1 = sk1.Sign(msgHash1);
auto sig2 = sk2.Sign(msgHash1);
auto sig1 = sk1.Sign(msgHash1, legacy_scheme);
auto sig2 = sk2.Sign(msgHash1, legacy_scheme);
BOOST_CHECK(sig1.VerifyInsecure(sk1.GetPublicKey(), msgHash1));
BOOST_CHECK(!sig1.VerifyInsecure(sk1.GetPublicKey(), msgHash2));
BOOST_CHECK(!sig2.VerifyInsecure(sk1.GetPublicKey(), msgHash1));
Expand All @@ -44,7 +44,7 @@ void FuncSerialize(const bool legacy_scheme)
uint256 msgHash = uint256::ONE;

sk.MakeNewKey();
CBLSSignature sig1 = sk.Sign(msgHash);
CBLSSignature sig1 = sk.Sign(msgHash, legacy_scheme);
ds2 << sig1;
ds3 << CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(sig1), !legacy_scheme);

Expand Down Expand Up @@ -102,7 +102,7 @@ void FuncKeyAgg(const bool legacy_scheme)
uint256 msgHash1 = uint256::ONE;
uint256 msgHash2 = uint256::TWO;

auto sig = ag_sk.Sign(msgHash1);
auto sig = ag_sk.Sign(msgHash1, legacy_scheme);
BOOST_CHECK(sig.VerifyInsecure(ag_pk, msgHash1));
BOOST_CHECK(!sig.VerifyInsecure(ag_pk, msgHash2));
}
Expand Down Expand Up @@ -144,7 +144,7 @@ void FuncKeyAggVec(const bool legacy_scheme)
uint256 msgHash1 = uint256::ONE;
uint256 msgHash2 = uint256::TWO;

auto sig = ag_sk.Sign(msgHash1);
auto sig = ag_sk.Sign(msgHash1, legacy_scheme);
BOOST_CHECK(sig.VerifyInsecure(ag_pk, msgHash1));
BOOST_CHECK(!sig.VerifyInsecure(ag_pk, msgHash2));
}
Expand All @@ -170,7 +170,7 @@ void FuncSigAggSub(const bool legacy_scheme)
vec_pks.push_back(sk.GetPublicKey());
hash = GetRandHash();
vec_hashes.push_back(hash);
CBLSSignature sig_i = sk.Sign(hash);
CBLSSignature sig_i = sk.Sign(hash, legacy_scheme);
vec_sigs.push_back(sig_i);
if (i == 0) {
// first sig is assigned directly
Expand Down Expand Up @@ -221,7 +221,7 @@ void FuncSigAggSecure(const bool legacy_scheme)
for (int i = 0; i < count; i++) {
sk.MakeNewKey();
vec_pks.push_back(sk.GetPublicKey());
vec_sigs.push_back(sk.Sign(hash));
vec_sigs.push_back(sk.Sign(hash, legacy_scheme));
}

auto sec_agg_sig = CBLSSignature::AggregateSecure(vec_sigs, vec_pks, hash);
Expand Down Expand Up @@ -264,19 +264,20 @@ struct Message

static void AddMessage(std::vector<Message>& vec, uint32_t sourceId, uint32_t msgId, uint8_t msgHash, bool valid)
{
bool legacy_scheme = bls::bls_legacy_scheme.load();
Message m;
m.sourceId = sourceId;
m.msgId = msgId;
m.msgHash = uint256(msgHash);
m.sk.MakeNewKey();
m.pk = m.sk.GetPublicKey();
m.sig = m.sk.Sign(m.msgHash);
m.sig = m.sk.Sign(m.msgHash, legacy_scheme);
m.valid = valid;

if (!valid) {
CBLSSecretKey tmp;
tmp.MakeNewKey();
m.sig = tmp.Sign(m.msgHash);
m.sig = tmp.Sign(m.msgHash, legacy_scheme);
}

vec.emplace_back(m);
Expand Down Expand Up @@ -381,7 +382,7 @@ void FuncThresholdSignature(const bool legacy_scheme)

CBLSSecretKey thr_sk = v_threshold_sks[0];
CBLSPublicKey thr_pk = v_threshold_pks[0];
CBLSSignature thr_sig = thr_sk.Sign(hash);
CBLSSignature thr_sig = thr_sk.Sign(hash, legacy_scheme);

std::vector<CBLSId> v_size_ids;
std::vector<CBLSSecretKey> v_size_sk_shares;
Expand All @@ -398,7 +399,7 @@ void FuncThresholdSignature(const bool legacy_scheme)
std::vector<CBLSSignature> v_share_sigs;
std::vector<CBLSId> v_share_ids;
for ([[maybe_unused]] const auto j : irange::range(m_shares)) {
v_share_sigs.emplace_back(v_size_sk_shares[j].Sign(hash));
v_share_sigs.emplace_back(v_size_sk_shares[j].Sign(hash, legacy_scheme));
BOOST_CHECK(v_share_sigs.back().VerifyInsecure(v_size_pk_shares[j], hash));
v_share_ids.push_back(v_size_ids[j]);
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/evo_deterministicmns_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ static CMutableTransaction CreateProUpServTx(const CChain& active_chain, const C
tx.nType = TRANSACTION_PROVIDER_UPDATE_SERVICE;
FundTransaction(active_chain, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN, coinbaseKey);
proTx.inputsHash = CalcTxInputsHash(CTransaction(tx));
proTx.sig = operatorKey.Sign(::SerializeHash(proTx));
proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme);
SetTxPayload(tx, proTx);
SignTransaction(mempool, tx, coinbaseKey);

Expand Down Expand Up @@ -171,7 +171,7 @@ static CMutableTransaction CreateProUpRevTx(const CChain& active_chain, const CT
tx.nType = TRANSACTION_PROVIDER_UPDATE_REVOKE;
FundTransaction(active_chain, tx, utxos, GetScriptForDestination(PKHash(coinbaseKey.GetPubKey())), 1 * COIN, coinbaseKey);
proTx.inputsHash = CalcTxInputsHash(CTransaction(tx));
proTx.sig = operatorKey.Sign(::SerializeHash(proTx));
proTx.sig = operatorKey.Sign(::SerializeHash(proTx), bls::bls_legacy_scheme);
SetTxPayload(tx, proTx);
SignTransaction(mempool, tx, coinbaseKey);

Expand Down
5 changes: 3 additions & 2 deletions src/test/evo_mnhf_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ BOOST_AUTO_TEST_CASE(verify_mnhf_specialtx_tests)
BOOST_CHECK(ag_sk.IsValid());
BOOST_CHECK(ag_pk.IsValid());

const bool use_legacy{false};
uint256 verHash = uint256S(ToString(bit));
auto sig = ag_sk.Sign(verHash);
BOOST_CHECK(sig.VerifyInsecure(ag_pk, verHash));
auto sig = ag_sk.Sign(verHash, use_legacy);
BOOST_CHECK(sig.VerifyInsecure(ag_pk, verHash, use_legacy));

auto& chainman = Assert(m_node.chainman);
auto& qman = *Assert(m_node.llmq_ctx)->qman;
Expand Down

0 comments on commit 59d13ab

Please sign in to comment.