From 544031db273eaabe33d80d317db8b0d9e60b47b1 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 13 Dec 2024 23:32:42 +0700 Subject: [PATCH 01/17] fix: do not pass param 'modOrder' for bls::PrivateKey --- src/rpc/evo.cpp | 13 ++++++------- src/test/bls_tests.cpp | 14 ++++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index a7d040a059baf..e6951efb936c0 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -197,11 +197,12 @@ static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string return pubKey; } -static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName, bool specific_legacy_bls_scheme) +static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName) { CBLSSecretKey secKey; - if (!secKey.SetHexStr(hexKey, specific_legacy_bls_scheme)) { + // Actually, bool flag for bls::PrivateKey has other meaning (modOrder) + if (!secKey.SetHexStr(hexKey, false)) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid BLS secret key", paramName)); } return secKey; @@ -964,7 +965,6 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques EnsureWalletIsUnlocked(*wallet); const bool isV19active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; - const bool is_bls_legacy = !isV19active; if (isEvoRequested && !isV19active) { throw JSONRPCError(RPC_INVALID_REQUEST, "EvoNodes aren't allowed yet"); } @@ -979,7 +979,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques throw std::runtime_error(strprintf("invalid network address %s", request.params[1].get_str())); } - CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[2].get_str(), "operatorKey", is_bls_legacy); + CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[2].get_str(), "operatorKey"); size_t paramIdx = 3; if (isEvoRequested) { @@ -1210,12 +1210,11 @@ static RPCHelpMan protx_revoke() EnsureWalletIsUnlocked(*pwallet); const bool isV19active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; - const bool is_bls_legacy = !isV19active; CProUpRevTx ptx; ptx.nVersion = CProUpRevTx::GetVersion(isV19active); ptx.proTxHash = ParseHashV(request.params[0], "proTxHash"); - CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey", is_bls_legacy); + CBLSSecretKey keyOperator = ParseBLSSecretKey(request.params[1].get_str(), "operatorKey"); if (!request.params[2].isNull()) { int32_t nReason = ParseInt32V(request.params[2], "reason"); @@ -1766,7 +1765,7 @@ static RPCHelpMan bls_fromsecret() if (!request.params[1].isNull()) { bls_legacy_scheme = ParseBoolV(request.params[1], "bls_legacy_scheme"); } - CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey", bls_legacy_scheme); + CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey"); UniValue ret(UniValue::VOBJ); ret.pushKV("secret", sk.ToString()); ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index 5afe60b26adab..e643d5b1206d3 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -63,19 +63,21 @@ void FuncSetHexStr(const bool legacy_scheme) { bls::bls_legacy_scheme.store(legacy_scheme); + // Note: 2nd bool argument for SetHexStr for bls::PrivateKey has a meaning modOrder, not is-legacy CBLSSecretKey sk; std::string strValidSecret = "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"; // Note: invalid string passed to SetHexStr() should cause it to fail and reset key internal data - BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1g", legacy_scheme)); // non-hex + BOOST_CHECK(sk.SetHexStr(strValidSecret, false)); + BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1g", false)); // non-hex BOOST_CHECK(!sk.IsValid()); BOOST_CHECK(sk == CBLSSecretKey()); // Try few more invalid strings - BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e", legacy_scheme)); // hex but too short + BOOST_CHECK(sk.SetHexStr(strValidSecret, false)); + BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e", false)); // hex but too short BOOST_CHECK(!sk.IsValid()); - BOOST_CHECK(sk.SetHexStr(strValidSecret, legacy_scheme)); - BOOST_CHECK(!sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20", legacy_scheme)); // hex but too long + BOOST_CHECK(sk.SetHexStr(strValidSecret, false)); + BOOST_CHECK( + !sk.SetHexStr("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20", false)); // hex but too long BOOST_CHECK(!sk.IsValid()); return; From b5cd09e58d596787a3d46133c5975876bfb9ffc2 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 14 Dec 2024 00:18:15 +0700 Subject: [PATCH 02/17] fix: rename argument name for IsTriviallyValid for protx Old name 'use legacy scheme' is incorrect, because this flag means 'is basic scheme' used --- src/evo/providertx.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/evo/providertx.h b/src/evo/providertx.h index 5734ead9449c3..73ec27ba61f68 100644 --- a/src/evo/providertx.h +++ b/src/evo/providertx.h @@ -112,7 +112,7 @@ class CProRegTx return obj; } - bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const; + bool IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const; }; class CProUpServTx @@ -192,7 +192,7 @@ class CProUpServTx return obj; } - bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const; + bool IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const; }; class CProUpRegTx @@ -257,7 +257,7 @@ class CProUpRegTx return obj; } - bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const; + bool IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const; }; class CProUpRevTx @@ -321,7 +321,7 @@ class CProUpRevTx return obj; } - bool IsTriviallyValid(bool is_bls_legacy_scheme, TxValidationState& state) const; + bool IsTriviallyValid(bool is_basic_scheme_active, TxValidationState& state) const; }; template From f1a7e95096cfdfee0f7613b41f1729dbcdc2c21a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 24 Dec 2024 16:44:38 +0700 Subject: [PATCH 03/17] fix: condition of bls activation to make work even from block 1 in specialtxman It used to be incorrectly processed of v19 is activated from block 1 --- src/evo/specialtxman.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 969efa2703055..1e1ebd87e48a7 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -206,7 +206,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB nTimeMnehf += nTime7 - nTime6; LogPrint(BCLog::BENCHMARK, " - m_mnhfman: %.2fms [%.2fs]\n", 0.001 * (nTime7 - nTime6), nTimeMnehf * 0.000001); - if (Params().GetConsensus().V19Height == pindex->nHeight + 1) { + if (DeploymentActiveAfter(pindex, m_consensus_params, Consensus::DEPLOYMENT_V19) && bls::bls_legacy_scheme.load()) { // NOTE: The block next to the activation is the one that is using new rules. // V19 activated just activated, so we must switch to the new rules here. bls::bls_legacy_scheme.store(false); @@ -227,7 +227,7 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const CBlock& block, const CBloc auto bls_legacy_scheme = bls::bls_legacy_scheme.load(); try { - if (Params().GetConsensus().V19Height == pindex->nHeight + 1) { + if (!DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_V19) && !bls_legacy_scheme) { // NOTE: The block next to the activation is the one that is using new rules. // Removing the activation block here, so we must switch back to the old rules. bls::bls_legacy_scheme.store(true); From 4acdd79cb455d3c04ceaa2e638d5d2adfb12cb3d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 23 Dec 2024 15:27:09 +0700 Subject: [PATCH 04/17] refactor: remove general constructor of BLS objects from bytes, add only to objects that uses it --- src/bls/bls.h | 13 +++++++++---- src/coinjoin/coinjoin.cpp | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index b1b0eb5f75764..45b7392d25a3c 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -57,10 +57,6 @@ class CBLSWrapper static constexpr size_t SerSize = _SerSize; explicit CBLSWrapper() = default; - explicit CBLSWrapper(Span vecBytes) : CBLSWrapper() - { - SetByteVector(vecBytes, bls::bls_legacy_scheme.load()); - } CBLSWrapper(const CBLSWrapper& ref) = default; CBLSWrapper& operator=(const CBLSWrapper& ref) = default; @@ -272,6 +268,11 @@ class CBLSSecretKey : public CBLSWrapper vecBytes) + { + // The second param here is not 'is_legacy', but `modOrder` + SetByteVector(vecBytes, false); + } CBLSSecretKey(const CBLSSecretKey&) = default; CBLSSecretKey& operator=(const CBLSSecretKey&) = default; @@ -338,6 +339,10 @@ class CBLSSignature : public CBLSWrapper bytes, bool is_serialized_legacy) + { + SetByteVector(bytes, is_serialized_legacy); + } CBLSSignature(const CBLSSignature&) = default; CBLSSignature& operator=(const CBLSSignature&) = default; diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 74fc644a70911..976633bb77175 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -62,7 +62,7 @@ bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman) bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const { - if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { + if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { LogPrint(BCLog::COINJOIN, "CCoinJoinQueue::CheckSignature -- VerifyInsecure() failed\n"); return false; } @@ -101,7 +101,7 @@ bool CCoinJoinBroadcastTx::Sign(const CActiveMasternodeManager& mn_activeman) bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const { - if (!CBLSSignature(Span{vchSig}).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { + if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { LogPrint(BCLog::COINJOIN, "CCoinJoinBroadcastTx::CheckSignature -- VerifyInsecure() failed\n"); return false; } From f728fc902558c2b26955ab236a76a331c97aaf85 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 23 Dec 2024 15:44:08 +0700 Subject: [PATCH 05/17] refactor: drop legacy flag from CActiveMasternodeInfo --- src/masternode/node.cpp | 1 - src/masternode/node.h | 2 -- src/rpc/governance.cpp | 9 ++++----- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 0a0b51deef7d8..76332e4ce749f 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -174,7 +174,6 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex) m_info.proTxHash = dmn->proTxHash; m_info.outpoint = dmn->collateralOutpoint; - m_info.legacy = dmn->pdmnState->nVersion == CProRegTx::LEGACY_BLS_VERSION; m_state = MasternodeState::READY; } diff --git a/src/masternode/node.h b/src/masternode/node.h index 713161ee065eb..9d10cc68f0cf8 100644 --- a/src/masternode/node.h +++ b/src/masternode/node.h @@ -22,7 +22,6 @@ struct CActiveMasternodeInfo { uint256 proTxHash; COutPoint outpoint; CService service; - bool legacy{true}; CActiveMasternodeInfo(const CBLSSecretKey& blsKeyOperator, const CBLSPublicKey& blsPubKeyOperator) : blsKeyOperator(blsKeyOperator), blsPubKeyOperator(blsPubKeyOperator) {}; @@ -74,7 +73,6 @@ class CActiveMasternodeManager final : public CValidationInterface [[nodiscard]] uint256 GetProTxHash() const { READ_LOCK(cs); return m_info.proTxHash; } [[nodiscard]] CService GetService() const { READ_LOCK(cs); return m_info.service; } [[nodiscard]] CBLSPublicKey GetPubKey() const; - [[nodiscard]] bool IsLegacy() const { READ_LOCK(cs); return m_info.legacy; } private: void InitInternal(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 5123945c3f323..2671149417f52 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -313,11 +313,10 @@ static RPCHelpMan gobject_submit() if (node.mn_activeman) { const bool fMnFound = mnList.HasValidMNByCollateral(node.mn_activeman->GetOutPoint()); - LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n", - node.mn_activeman->GetPubKey().ToString(node.mn_activeman->IsLegacy()), - node.mn_activeman->GetOutPoint().ToStringShort(), - request.params.size(), - fMnFound); + LogPrint(BCLog::GOBJECT, /* Continued */ + "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n", + node.mn_activeman->GetPubKey().ToString(false), node.mn_activeman->GetOutPoint().ToStringShort(), + request.params.size(), fMnFound); } else { LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = N/A, outpoint = N/A, params.size() = %lld, fMnFound = %d\n", request.params.size(), From caedaba80237021afeb493ab6b7b91c0ee44e6e9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 24 Dec 2024 19:41:56 +0700 Subject: [PATCH 06/17] refactor: remove CheckMalleable without bls scheme argument --- src/bls/bls.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index 45b7392d25a3c..c02dac47b2110 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -211,11 +211,6 @@ class CBLSWrapper return true; } - inline bool CheckMalleable(Span vecBytes) const - { - return CheckMalleable(vecBytes, bls::bls_legacy_scheme.load()); - } - inline std::string ToString(const bool specificLegacyScheme) const { std::vector buf = ToByteVector(specificLegacyScheme); From 394d6496166aa55a65f49d67038a66fce8068e26 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 23 Dec 2024 17:25:35 +0700 Subject: [PATCH 07/17] feat: use basic scheme for RPC `bls generate` by default Soft fork V19 is activated long time ago, so, basically no changes for mainnet/testnet --- src/rpc/evo.cpp | 14 ++++---------- src/test/rpc_tests.cpp | 14 +++++++------- test/functional/feature_dip3_deterministicmns.py | 4 ++-- test/functional/test_framework/test_framework.py | 4 ++-- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index e6951efb936c0..04e6841cd3ad5 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1705,7 +1705,7 @@ static RPCHelpMan bls_generate() return RPCHelpMan{"bls generate", "\nReturns a BLS secret/public key pair.\n", { - {"legacy", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true until the v19 fork is activated, otherwise false"}, "Use legacy BLS scheme"}, + {"legacy", RPCArg::Type::BOOL, RPCArg::Default{false}, "Set it true if need in legacy BLS scheme"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -1719,12 +1719,9 @@ static RPCHelpMan bls_generate() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - const NodeContext& node = EnsureAnyNodeContext(request.context); - const ChainstateManager& chainman = EnsureChainman(node); - CBLSSecretKey sk; sk.MakeNewKey(); - bool bls_legacy_scheme{!DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; + bool bls_legacy_scheme{false}; if (!request.params[0].isNull()) { bls_legacy_scheme = ParseBoolV(request.params[0], "bls_legacy_scheme"); } @@ -1744,7 +1741,7 @@ static RPCHelpMan bls_fromsecret() "\nParses a BLS secret key and returns the secret/public key pair.\n", { {"secret", RPCArg::Type::STR, RPCArg::Optional::NO, "The BLS secret key"}, - {"legacy", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true until the v19 fork is activated, otherwise false"}, "Use legacy BLS scheme"}, + {"legacy", RPCArg::Type::BOOL, RPCArg::Default{false}, "Pass true if you need in legacy scheme"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -1758,10 +1755,7 @@ static RPCHelpMan bls_fromsecret() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - const NodeContext& node = EnsureAnyNodeContext(request.context); - const ChainstateManager& chainman = EnsureChainman(node); - - bool bls_legacy_scheme{!DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; + bool bls_legacy_scheme{false}; if (!request.params[1].isNull()) { bls_legacy_scheme = ParseBoolV(request.params[1], "bls_legacy_scheme"); } diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 687530b49fb72..6f7eb898f2669 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -543,7 +543,7 @@ BOOST_AUTO_TEST_CASE(rpc_bls) UniValue r; BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls generate"))); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "legacy"); + BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "basic"); BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls generate 1"))); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "legacy"); @@ -555,15 +555,15 @@ BOOST_AUTO_TEST_CASE(rpc_bls) std::string secret_basic = find_value(r.get_obj(), "secret").get_str(); std::string public_basic = find_value(r.get_obj(), "public").get_str(); - BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret_legacy)); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "legacy"); - BOOST_CHECK_EQUAL(find_value(r.get_obj(), "public").get_str(), public_legacy); + BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret_basic)); + BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "basic"); + BOOST_CHECK_EQUAL(find_value(r.get_obj(), "public").get_str(), public_basic); BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret_legacy + std::string(" 1"))); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "legacy"); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "public").get_str(), public_legacy); - BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret_legacy + std::string(" 0"))); + BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret_basic + std::string(" 0"))); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "basic"); BOOST_CHECK(find_value(r.get_obj(), "public").get_str() != public_legacy); @@ -576,10 +576,10 @@ BOOST_AUTO_TEST_CASE(rpc_bls) BOOST_CHECK(find_value(r.get_obj(), "public").get_str() != public_basic); std::string secret = "0b072b1b8b28335b0460aa695ee8ce1f60dc01e6eb12655ece2a877379dfdb51"; - BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret)); + BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret + " 1")); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "legacy"); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "public").get_str(), "9379c28e0f50546906fe733f1222c8f7e39574d513790034f1fec1476286eb652a350c8c0e630cd2cc60d10c26d6f6ee"); - BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret + " 0")); + BOOST_CHECK_NO_THROW(r = CallRPC(std::string("bls fromsecret ") + secret)); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "scheme").get_str(), "basic"); BOOST_CHECK_EQUAL(find_value(r.get_obj(), "public").get_str(), "b379c28e0f50546906fe733f1222c8f7e39574d513790034f1fec1476286eb652a350c8c0e630cd2cc60d10c26d6f6ee"); } diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 1cc6e17e2db0b..6a06dcdef57b7 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -12,7 +12,7 @@ from test_framework.blocktools import create_block_with_mnpayments from test_framework.messages import tx_from_hex from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, force_finish_mnsync, p2p_port +from test_framework.util import assert_equal, force_finish_mnsync, p2p_port, softfork_active class Masternode(object): pass @@ -226,7 +226,7 @@ def prepare_mn(self, node, idx, alias): mn.p2p_port = p2p_port(mn.idx) mn.operator_reward = (mn.idx % self.num_initial_mn) - blsKey = node.bls('generate') + blsKey = node.bls('generate') if softfork_active(node, 'v19') else node.bls('generate', True) mn.fundsAddr = node.getnewaddress() mn.ownerAddr = node.getnewaddress() mn.operatorAddr = blsKey['public'] diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index e8ddfff205610..ed042d0393245 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1303,7 +1303,7 @@ def dynamically_add_masternode(self, evo=False, rnd=None, should_be_rejected=Fal return created_mn_info def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None): - bls = self.nodes[0].bls('generate') + bls = self.nodes[0].bls('generate') if softfork_active(self.nodes[0], 'v19') else self.nodes[0].bls('generate', True) collateral_address = self.nodes[0].getnewaddress() funds_address = self.nodes[0].getnewaddress() owner_address = self.nodes[0].getnewaddress() @@ -1387,7 +1387,7 @@ def prepare_masternode(self, idx): register_fund = (idx % 2) == 0 - bls = self.nodes[0].bls('generate') + bls = self.nodes[0].bls('generate') if softfork_active(self.nodes[0], 'v19') else self.nodes[0].bls('generate', True) address = self.nodes[0].getnewaddress() collateral_amount = MASTERNODE_COLLATERAL From 7cbb26b39c85b5580ca3a7a705146b9b155a1d91 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 25 Dec 2024 15:59:33 +0700 Subject: [PATCH 08/17] style: clang suggestion for bls rpcs --- src/rpc/evo.cpp | 95 ++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 49 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 04e6841cd3ad5..faf2e41b7fc61 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1702,71 +1702,68 @@ static RPCHelpMan protx_help() static RPCHelpMan bls_generate() { - return RPCHelpMan{"bls generate", + return RPCHelpMan{ + "bls generate", "\nReturns a BLS secret/public key pair.\n", { {"legacy", RPCArg::Type::BOOL, RPCArg::Default{false}, "Set it true if need in legacy BLS scheme"}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "secret", "BLS secret key"}, - {RPCResult::Type::STR_HEX, "public", "BLS public key"}, - {RPCResult::Type::STR_HEX, "scheme", "BLS scheme (valid schemes: legacy, basic)"} - }}, - RPCExamples{ - HelpExampleCli("bls generate", "") }, - [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - CBLSSecretKey sk; - sk.MakeNewKey(); - bool bls_legacy_scheme{false}; - if (!request.params[0].isNull()) { - bls_legacy_scheme = ParseBoolV(request.params[0], "bls_legacy_scheme"); - } - UniValue ret(UniValue::VOBJ); - ret.pushKV("secret", sk.ToString()); - ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); - std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic"; - ret.pushKV("scheme", bls_scheme_str); - return ret; -}, + RPCResult{RPCResult::Type::OBJ, + "", + "", + {{RPCResult::Type::STR_HEX, "secret", "BLS secret key"}, + {RPCResult::Type::STR_HEX, "public", "BLS public key"}, + {RPCResult::Type::STR_HEX, "scheme", "BLS scheme (valid schemes: legacy, basic)"}}}, + RPCExamples{HelpExampleCli("bls generate", "")}, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + CBLSSecretKey sk; + sk.MakeNewKey(); + bool bls_legacy_scheme{false}; + if (!request.params[0].isNull()) { + bls_legacy_scheme = ParseBoolV(request.params[0], "bls_legacy_scheme"); + } + UniValue ret(UniValue::VOBJ); + ret.pushKV("secret", sk.ToString()); + ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); + std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic"; + ret.pushKV("scheme", bls_scheme_str); + return ret; + }, }; } static RPCHelpMan bls_fromsecret() { - return RPCHelpMan{"bls fromsecret", + return RPCHelpMan{ + "bls fromsecret", "\nParses a BLS secret key and returns the secret/public key pair.\n", { {"secret", RPCArg::Type::STR, RPCArg::Optional::NO, "The BLS secret key"}, {"legacy", RPCArg::Type::BOOL, RPCArg::Default{false}, "Pass true if you need in legacy scheme"}, }, - RPCResult{ - RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "secret", "BLS secret key"}, - {RPCResult::Type::STR_HEX, "public", "BLS public key"}, - {RPCResult::Type::STR_HEX, "scheme", "BLS scheme (valid schemes: legacy, basic)"}, - }}, + RPCResult{RPCResult::Type::OBJ, + "", + "", + { + {RPCResult::Type::STR_HEX, "secret", "BLS secret key"}, + {RPCResult::Type::STR_HEX, "public", "BLS public key"}, + {RPCResult::Type::STR_HEX, "scheme", "BLS scheme (valid schemes: legacy, basic)"}, + }}, RPCExamples{ - HelpExampleCli("bls fromsecret", "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f") + HelpExampleCli("bls fromsecret", "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f")}, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + bool bls_legacy_scheme{false}; + if (!request.params[1].isNull()) { + bls_legacy_scheme = ParseBoolV(request.params[1], "bls_legacy_scheme"); + } + CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey"); + UniValue ret(UniValue::VOBJ); + ret.pushKV("secret", sk.ToString()); + ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); + std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic"; + ret.pushKV("scheme", bls_scheme_str); + return ret; }, - [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - bool bls_legacy_scheme{false}; - if (!request.params[1].isNull()) { - bls_legacy_scheme = ParseBoolV(request.params[1], "bls_legacy_scheme"); - } - CBLSSecretKey sk = ParseBLSSecretKey(request.params[0].get_str(), "secretKey"); - UniValue ret(UniValue::VOBJ); - ret.pushKV("secret", sk.ToString()); - ret.pushKV("public", sk.GetPublicKey().ToString(bls_legacy_scheme)); - std::string bls_scheme_str = bls_legacy_scheme ? "legacy" : "basic"; - ret.pushKV("scheme", bls_scheme_str); - return ret; -}, }; } From 26c058a28c347adbe2c689a4778df88dbca74f6d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 23 Dec 2024 21:09:36 +0700 Subject: [PATCH 09/17] refactor: optimize usages bls_ies.h and bls.h --- src/evo/assetlocktx.h | 2 +- src/llmq/dkgsessionmgr.cpp | 1 + src/llmq/dkgsessionmgr.h | 6 +++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/evo/assetlocktx.h b/src/evo/assetlocktx.h index 531183fd0a075..d63ef4092e32c 100644 --- a/src/evo/assetlocktx.h +++ b/src/evo/assetlocktx.h @@ -5,7 +5,7 @@ #ifndef BITCOIN_EVO_ASSETLOCKTX_H #define BITCOIN_EVO_ASSETLOCKTX_H -#include +#include #include #include #include diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index d6fb0715c1014..d0842c0bcd7ae 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index eaa5dc79deae3..fd984247023b9 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -6,7 +6,6 @@ #define BITCOIN_LLMQ_DKGSESSIONMGR_H #include -#include #include #include #include @@ -14,6 +13,11 @@ #include #include +template +class CBLSIESMultiRecipientObjects; +template +class CBLSIESEncryptedObject; + class CActiveMasternodeManager; class CBlockIndex; class CChainState; From 91f10b16ef1f87ee33ae07dd6508c88abe51aba0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 24 Dec 2024 20:37:02 +0700 Subject: [PATCH 10/17] refactor: removed unused functions from bls_ies --- src/bls/bls_ies.cpp | 36 ------------------------------------ src/bls/bls_ies.h | 36 ------------------------------------ 2 files changed, 72 deletions(-) diff --git a/src/bls/bls_ies.cpp b/src/bls/bls_ies.cpp index 1068e078fc9b2..728a4fb8bf052 100644 --- a/src/bls/bls_ies.cpp +++ b/src/bls/bls_ies.cpp @@ -38,24 +38,6 @@ uint256 CBLSIESEncryptedBlob::GetIV(size_t idx) const return iv; } -bool CBLSIESEncryptedBlob::Encrypt(size_t idx, const CBLSPublicKey& peerPubKey, const void* plainTextData, size_t dataSize) -{ - CBLSSecretKey ephemeralSecretKey; - ephemeralSecretKey.MakeNewKey(); - ephemeralPubKey = ephemeralSecretKey.GetPublicKey(); - - CBLSPublicKey pk; - if (!pk.DHKeyExchange(ephemeralSecretKey, peerPubKey)) { - return false; - } - - std::vector symKey = pk.ToByteVector(); - symKey.resize(32); - - uint256 iv = GetIV(idx); - return EncryptBlob(plainTextData, dataSize, data, symKey.data(), iv.begin()); -} - bool CBLSIESEncryptedBlob::Decrypt(size_t idx, const CBLSSecretKey& secretKey, CDataStream& decryptedDataRet) const { CBLSPublicKey pk; @@ -75,24 +57,6 @@ bool CBLSIESEncryptedBlob::IsValid() const return ephemeralPubKey.IsValid() && !data.empty() && !ivSeed.IsNull(); } - -bool CBLSIESMultiRecipientBlobs::Encrypt(const std::vector& recipients, const BlobVector& _blobs) -{ - if (recipients.size() != _blobs.size()) { - return false; - } - - InitEncrypt(_blobs.size()); - - for (size_t i = 0; i < _blobs.size(); i++) { - if (!Encrypt(i, recipients[i], _blobs[i])) { - return false; - } - } - - return true; -} - void CBLSIESMultiRecipientBlobs::InitEncrypt(size_t count) { ephemeralSecretKey.MakeNewKey(); diff --git a/src/bls/bls_ies.h b/src/bls/bls_ies.h index 002d41a86bdf8..d2a58cc370aed 100644 --- a/src/bls/bls_ies.h +++ b/src/bls/bls_ies.h @@ -22,7 +22,6 @@ class CBLSIESEncryptedBlob READWRITE(obj.ephemeralPubKey, obj.ivSeed, obj.data); } - bool Encrypt(size_t idx, const CBLSPublicKey& peerPubKey, const void* data, size_t dataSize); bool Decrypt(size_t idx, const CBLSSecretKey& secretKey, CDataStream& decryptedDataRet) const; bool IsValid() const; }; @@ -40,17 +39,6 @@ class CBLSIESEncryptedObject : public CBLSIESEncryptedBlob data = dataIn; } - bool Encrypt(size_t idx, const CBLSPublicKey& peerPubKey, const Object& obj, int nVersion) - { - try { - CDataStream ds(SER_NETWORK, nVersion); - ds << obj; - return CBLSIESEncryptedBlob::Encrypt(idx, peerPubKey, ds.data(), ds.size()); - } catch (const std::exception&) { - return false; - } - } - bool Decrypt(size_t idx, const CBLSSecretKey& secretKey, Object& objRet, int nVersion) const { CDataStream ds(SER_NETWORK, nVersion); @@ -80,8 +68,6 @@ class CBLSIESMultiRecipientBlobs CBLSSecretKey ephemeralSecretKey; std::vector ivVector; - bool Encrypt(const std::vector& recipients, const BlobVector& _blobs); - void InitEncrypt(size_t count); bool Encrypt(size_t idx, const CBLSPublicKey& recipient, const Blob& blob); bool Decrypt(size_t idx, const CBLSSecretKey& sk, Blob& blobRet) const; @@ -96,28 +82,6 @@ template class CBLSIESMultiRecipientObjects : public CBLSIESMultiRecipientBlobs { public: - using ObjectVector = std::vector; - - bool Encrypt(const std::vector& recipients, const ObjectVector& _objects, int nVersion) - { - BlobVector blobs; - blobs.resize(_objects.size()); - - try { - CDataStream ds(SER_NETWORK, nVersion); - for (size_t i = 0; i < _objects.size(); i++) { - ds.clear(); - - ds << _objects[i]; - blobs[i].assign(UCharCast(ds.data()), UCharCast(ds.data() + ds.size())); - } - } catch (const std::exception&) { - return false; - } - - return CBLSIESMultiRecipientBlobs::Encrypt(recipients, blobs); - } - bool Encrypt(size_t idx, const CBLSPublicKey& recipient, const Object& obj, int nVersion) { CDataStream ds(SER_NETWORK, nVersion); From 99e9c65484e935d56132da99a549d0ccd26dd90a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 25 Dec 2024 00:05:13 +0700 Subject: [PATCH 11/17] feat: use Basic scheme only for public key serialization in bls_ies It also removes unused ToByteVector from CBLSWrapper --- src/bls/bls.h | 5 ----- src/bls/bls_ies.cpp | 9 ++++----- src/bls/bls_ies.h | 5 +++++ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index c02dac47b2110..f026b73de51d1 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -127,11 +127,6 @@ class CBLSWrapper return impl.Serialize(specificLegacyScheme); } - std::vector ToByteVector() const - { - return ToByteVector(bls::bls_legacy_scheme.load()); - } - const uint256& GetHash() const { if (cachedHash.IsNull()) { diff --git a/src/bls/bls_ies.cpp b/src/bls/bls_ies.cpp index 728a4fb8bf052..e6282c02853a5 100644 --- a/src/bls/bls_ies.cpp +++ b/src/bls/bls_ies.cpp @@ -9,8 +9,7 @@ #include -template -static bool EncryptBlob(const void* in, size_t inSize, Out& out, const void* symKey, const void* iv) +static bool EncryptBlob(const void* in, size_t inSize, std::vector& out, const void* symKey, const void* iv) { out.resize(inSize); @@ -45,7 +44,7 @@ bool CBLSIESEncryptedBlob::Decrypt(size_t idx, const CBLSSecretKey& secretKey, C return false; } - std::vector symKey = pk.ToByteVector(); + std::vector symKey = pk.ToByteVector(false); symKey.resize(32); uint256 iv = GetIV(idx); @@ -81,7 +80,7 @@ bool CBLSIESMultiRecipientBlobs::Encrypt(size_t idx, const CBLSPublicKey& recipi return false; } - std::vector symKey = pk.ToByteVector(); + std::vector symKey = pk.ToByteVector(false); symKey.resize(32); return EncryptBlob(blob.data(), blob.size(), blobs[idx], symKey.data(), ivVector[idx].begin()); @@ -98,7 +97,7 @@ bool CBLSIESMultiRecipientBlobs::Decrypt(size_t idx, const CBLSSecretKey& sk, Bl return false; } - std::vector symKey = pk.ToByteVector(); + std::vector symKey = pk.ToByteVector(false); symKey.resize(32); uint256 iv = ivSeed; diff --git a/src/bls/bls_ies.h b/src/bls/bls_ies.h index d2a58cc370aed..72633e1aa5086 100644 --- a/src/bls/bls_ies.h +++ b/src/bls/bls_ies.h @@ -8,6 +8,11 @@ #include #include +/** + * All objects in this module working from assumption that basic scheme is + * available on all masternodes. Serialization of public key for Encrypt and + * Decrypt by bls_ies.h done using Basic Scheme. + */ class CBLSIESEncryptedBlob { public: From 5ea9ff9e7150f3594823f6091767f6578002dfaa Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 24 Dec 2024 02:17:00 +0700 Subject: [PATCH 12/17] feat: enforce using _legacy suffix for protx commands; no more guessing based if v19 is activated Soft fork V19 is activated long time ago, so, basically no changes for mainnet/testnet --- src/rpc/evo.cpp | 5 ++--- test/functional/feature_dip3_deterministicmns.py | 8 ++++---- test/functional/test_framework/test_framework.py | 13 ++++++++----- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index faf2e41b7fc61..5932bc22f9729 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -653,7 +653,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, tx.nVersion = 3; tx.nType = TRANSACTION_PROVIDER_REGISTER; - const bool use_legacy = isV19active ? specific_legacy_bls_scheme : true; + const bool use_legacy = specific_legacy_bls_scheme; CProRegTx ptx; ptx.nType = mnType; @@ -1107,8 +1107,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme ptx.keyIDVoting = dmn->pdmnState->keyIDVoting; ptx.scriptPayout = dmn->pdmnState->scriptPayout; - const bool isV19Active{DeploymentActiveAfter(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip();), Params().GetConsensus(), Consensus::DEPLOYMENT_V19)}; - const bool use_legacy = isV19Active ? specific_legacy_bls_scheme : true; + const bool use_legacy = specific_legacy_bls_scheme; if (request.params[1].get_str() != "") { // new pubkey diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 6a06dcdef57b7..3be51eba75b53 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -211,7 +211,7 @@ def run_test(self): assert old_voting_address != new_voting_address # also check if funds from payout address are used when no fee source address is specified node.sendtoaddress(mn.rewards_address, 0.001) - node.protx('update_registrar', mn.protx_hash, "", new_voting_address, "") + node.protx('update_registrar' if softfork_active(node, 'v19') else 'update_registrar_legacy', mn.protx_hash, "", new_voting_address, "") self.generate(node, 1) new_dmnState = mn.node.masternode("status")["dmnState"] new_voting_address_from_rpc = new_dmnState["votingAddress"] @@ -254,7 +254,7 @@ def register_fund_mn(self, node, mn): mn.collateral_address = node.getnewaddress() mn.rewards_address = node.getnewaddress() - mn.protx_hash = node.protx('register_fund', mn.collateral_address, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) + mn.protx_hash = node.protx('register_fund' if softfork_active(node, 'v19') else 'register_fund_legacy', mn.collateral_address, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) mn.collateral_txid = mn.protx_hash mn.collateral_vout = None @@ -270,7 +270,7 @@ def register_mn(self, node, mn): node.sendtoaddress(mn.fundsAddr, 0.001) mn.rewards_address = node.getnewaddress() - mn.protx_hash = node.protx('register', mn.collateral_txid, mn.collateral_vout, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) + mn.protx_hash = node.protx('register' if softfork_active(node, 'v19') else 'register_legacy', mn.collateral_txid, mn.collateral_vout, '127.0.0.1:%d' % mn.p2p_port, mn.ownerAddr, mn.operatorAddr, mn.votingAddr, mn.operator_reward, mn.rewards_address, mn.fundsAddr) self.generate(node, 1, sync_fun=self.no_op) def start_mn(self, mn): @@ -288,7 +288,7 @@ def spend_mn_collateral(self, mn, with_dummy_input_output=False): def update_mn_payee(self, mn, payee): self.nodes[0].sendtoaddress(mn.fundsAddr, 0.001) - self.nodes[0].protx('update_registrar', mn.protx_hash, '', '', payee, mn.fundsAddr) + self.nodes[0].protx('update_registrar' if softfork_active(self.nodes[0], 'v19') else 'update_registrar_legacy', mn.protx_hash, '', '', payee, mn.fundsAddr) self.generate(self.nodes[0], 1) info = self.nodes[0].protx('info', mn.protx_hash) assert info['state']['payoutAddress'] == payee diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ed042d0393245..886d12672973a 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1303,7 +1303,8 @@ def dynamically_add_masternode(self, evo=False, rnd=None, should_be_rejected=Fal return created_mn_info def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None): - bls = self.nodes[0].bls('generate') if softfork_active(self.nodes[0], 'v19') else self.nodes[0].bls('generate', True) + v19_active = softfork_active(self.nodes[0], 'v19') + bls = self.nodes[0].bls('generate') if v19_active else self.nodes[0].bls('generate', True) collateral_address = self.nodes[0].getnewaddress() funds_address = self.nodes[0].getnewaddress() owner_address = self.nodes[0].getnewaddress() @@ -1336,7 +1337,7 @@ def dynamically_prepare_masternode(self, idx, node_p2p_port, evo=False, rnd=None if evo: protx_result = self.nodes[0].protx("register_evo", collateral_txid, collateral_vout, ipAndPort, owner_address, bls['public'], voting_address, operatorReward, reward_address, platform_node_id, platform_p2p_port, platform_http_port, funds_address, True) else: - protx_result = self.nodes[0].protx("register", collateral_txid, collateral_vout, ipAndPort, owner_address, bls['public'], voting_address, operatorReward, reward_address, funds_address, True) + protx_result = self.nodes[0].protx("register" if v19_active else "register_legacy", collateral_txid, collateral_vout, ipAndPort, owner_address, bls['public'], voting_address, operatorReward, reward_address, funds_address, True) self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block tip = self.generate(self.nodes[0], 1)[0] @@ -1387,7 +1388,9 @@ def prepare_masternode(self, idx): register_fund = (idx % 2) == 0 - bls = self.nodes[0].bls('generate') if softfork_active(self.nodes[0], 'v19') else self.nodes[0].bls('generate', True) + v19_active = softfork_active(self.nodes[0], 'v19') + + bls = self.nodes[0].bls('generate') if v19_active else self.nodes[0].bls('generate', True) address = self.nodes[0].getnewaddress() collateral_amount = MASTERNODE_COLLATERAL @@ -1416,10 +1419,10 @@ def prepare_masternode(self, idx): submit = (idx % 4) < 2 if register_fund: - protx_result = self.nodes[0].protx('register_fund', address, ipAndPort, ownerAddr, bls['public'], votingAddr, operatorReward, rewardsAddr, address, submit) + protx_result = self.nodes[0].protx('register_fund' if v19_active else 'register_fund_legacy', address, ipAndPort, ownerAddr, bls['public'], votingAddr, operatorReward, rewardsAddr, address, submit) else: self.generate(self.nodes[0], 1, sync_fun=self.no_op) - protx_result = self.nodes[0].protx('register', txid, collateral_vout, ipAndPort, ownerAddr, bls['public'], votingAddr, operatorReward, rewardsAddr, address, submit) + protx_result = self.nodes[0].protx('register' if v19_active else 'register_legacy', txid, collateral_vout, ipAndPort, ownerAddr, bls['public'], votingAddr, operatorReward, rewardsAddr, address, submit) if submit: proTxHash = protx_result From 954e5ef9f073487e91458b8fa16019ac2735d4b5 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Dec 2024 22:56:28 +0700 Subject: [PATCH 13/17] fix: undefined behaviour in evo RPC calls of RPCHelpMan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The implementation of RPC has been refactored in bitcoin significantly with using RPCHelpMan, see multiple PR, such as bitcoin#1853, bitcoin#19528, etc Backporting them and appliying same refactoring to Dash Core caused undefined behaviour, particularly #6072 For example, in this code the local variable `use_legacy` will be used after `protx_update_registrar_wrapper`, when executor will be called. static RPCHelpMan protx_update_registrar_wrapper(const bool use_legacy) { std::string rpc_name = use_legacy ? "update_registrar_legacy" : "update_registrar"; std::string rpc_full_name = std::string("protx ").append(rpc_name); std::string pubkey_operator = use_legacy ? "\"0532646990082f4fd639f90387b1551f2c7c39d37392cb9055a06a7e85c1d23692db8f87f827886310bccc1e29db9aee\"" : "\"8532646990082f4fd639f90387b1551f2c7c39d37392cb9055a06a7e85c1d23692db8f87f827886310bccc1e29db9aee\""; std::string rpc_example = rpc_name.append(" \"0123456701234567012345670123456701234567012345670123456701234567\" ").append(pubkey_operator).append(" \"" + EXAMPLE_ADDRESS[1] + "\""); return RPCHelpMan{rpc_full_name, <...> RPCResult{ RPCResult::Type::STR_HEX, "txid", "The transaction id" }, RPCExamples{ HelpExampleCli("protx", rpc_example) }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { <...> ptx.nVersion = specific_legacy_bls_scheme ? CProUpRegTx::LEGACY_BLS_VERSION : CProUpRegTx::BASIC_BLS_VERSION; <<<<---- THERE IS UB It can be easy tested by adding debug logs to log string, for example rpc_full_name at the moment of call of executor has been during run: 2024-12-26T16:10:15Z rpc full name: 'r,ߧzu\x00\x00����m���istrar_legacy' This PR fixes multiple unexplainable random failures which had happen only for `tsan` build or for only `ubsan` build during debuggin #6508 and depends only on code revision. --- src/rpc/evo.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 5932bc22f9729..cd76099069498 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -398,7 +398,7 @@ static RPCHelpMan protx_register_fund_wrapper(const bool legacy) }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::Fund, MnType::Regular); + return protx_register_common_wrapper(request, self.m_name == "protx register_fund_legacy", ProTxRegisterAction::Fund, MnType::Regular); }, }; } @@ -445,7 +445,7 @@ static RPCHelpMan protx_register_wrapper(bool legacy) }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::External, MnType::Regular); + return protx_register_common_wrapper(request, self.m_name == "protx register_legacy", ProTxRegisterAction::External, MnType::Regular); }, }; } @@ -493,7 +493,7 @@ static RPCHelpMan protx_register_prepare_wrapper(const bool legacy) }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - return protx_register_common_wrapper(request, legacy, ProTxRegisterAction::Prepare, MnType::Regular); + return protx_register_common_wrapper(request, self.m_name == "protx register_prepare_legacy", ProTxRegisterAction::Prepare, MnType::Regular); }, }; } @@ -1060,7 +1060,7 @@ static UniValue protx_update_service_common_wrapper(const JSONRPCRequest& reques return SignAndSendSpecialTx(request, chain_helper, chainman, tx); } -static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme) +static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_scheme) { std::string rpc_name = specific_legacy_bls_scheme ? "update_registrar_legacy" : "update_registrar"; std::string rpc_full_name = std::string("protx ").append(rpc_name); @@ -1073,7 +1073,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme + HELP_REQUIRING_PASSPHRASE, { GetRpcArg("proTxHash"), - specific_legacy_bls_scheme ? GetRpcArg("operatorPubKey_update_legacy") : GetRpcArg("operatorPubKey_update"), + specific_legacy_bls_scheme ? GetRpcArg("operatorPubKey_update_legacy") : GetRpcArg("operatorPubKey_update"), GetRpcArg("votingAddress_update"), GetRpcArg("payoutAddress_update"), GetRpcArg("feeSourceAddress"), @@ -1086,6 +1086,7 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + const bool use_legacy{self.m_name == "protx update_registrar_legacy"}; const NodeContext& node = EnsureAnyNodeContext(request.context); const ChainstateManager& chainman = EnsureChainman(node); @@ -1107,8 +1108,6 @@ static RPCHelpMan protx_update_registrar_wrapper(bool specific_legacy_bls_scheme ptx.keyIDVoting = dmn->pdmnState->keyIDVoting; ptx.scriptPayout = dmn->pdmnState->scriptPayout; - const bool use_legacy = specific_legacy_bls_scheme; - if (request.params[1].get_str() != "") { // new pubkey ptx.pubKeyOperator.Set(ParseBLSPubKey(request.params[1].get_str(), "operator BLS address", use_legacy), use_legacy); From ce18dcd2ac64e4b11d640253e2f0b3ff655885a2 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 24 Dec 2024 19:22:39 +0700 Subject: [PATCH 14/17] refactor: enforce passing bls scheme for each call of sign by BLS --- src/bench/bls.cpp | 10 +++++----- src/bls/bls.cpp | 5 ----- src/bls/bls.h | 1 - src/bls/bls_worker.cpp | 2 +- src/evo/mnauth.cpp | 2 +- src/llmq/dkgsession.cpp | 23 +++++++++++------------ src/llmq/dkgsession.h | 1 + src/llmq/signing_shares.cpp | 2 +- src/masternode/node.cpp | 6 ------ src/masternode/node.h | 1 - src/rpc/evo.cpp | 11 ++++++----- src/test/bls_tests.cpp | 23 ++++++++++++----------- src/test/evo_deterministicmns_tests.cpp | 4 ++-- src/test/evo_mnhf_tests.cpp | 5 +++-- 14 files changed, 43 insertions(+), 53 deletions(-) diff --git a/src/bench/bls.cpp b/src/bench/bls.cpp index b9fd3ac163f17..c22321da40d10 100644 --- a/src/bench/bls.cpp +++ b/src/bench/bls.cpp @@ -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); } } } @@ -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([&] { @@ -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); }); } diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index 85a96c3e3e219..09f192cd67b52 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -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()) { diff --git a/src/bls/bls.h b/src/bls/bls.h index f026b73de51d1..1c08240227467 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -275,7 +275,6 @@ class CBLSSecretKey : public CBLSWrapper 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; }; diff --git a/src/bls/bls_worker.cpp b/src/bls/bls_worker.cpp index f5197e8fa53af..4767d02ed394a 100644 --- a/src/bls/bls_worker.cpp +++ b/src/bls/bls_worker.cpp @@ -762,7 +762,7 @@ bool CBLSWorker::VerifyVerificationVectors(Span 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())); }); } diff --git a/src/evo/mnauth.cpp b/src/evo/mnauth.cpp index 36c6c17524c40..650cfedfbace6 100644 --- a/src/evo/mnauth.cpp +++ b/src/evo/mnauth.cpp @@ -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)); diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index b63ebf45a4e0f..6f0b2a903dae1 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -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)} { } @@ -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(); @@ -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(); @@ -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(); @@ -1011,19 +1012,17 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages, PeerManag (*commitmentHash.begin())++; } - qc.sig = m_mn_activeman->Sign(commitmentHash); - qc.quorumSig = skShare.Sign(commitmentHash); + qc.sig = m_mn_activeman->Sign(commitmentHash, m_use_legacy_bls); + qc.quorumSig = skShare.Sign(commitmentHash, m_use_legacy_bls); if (lieType == 3) { - const bool is_bls_legacy = bls::bls_legacy_scheme.load(); - std::vector buf = qc.sig.ToByteVector(is_bls_legacy); + std::vector buf = qc.sig.ToByteVector(m_use_legacy_bls); buf[5]++; - qc.sig.SetByteVector(buf, is_bls_legacy); + qc.sig.SetByteVector(buf, m_use_legacy_bls); } else if (lieType == 4) { - const bool is_bls_legacy = bls::bls_legacy_scheme.load(); - std::vector buf = qc.quorumSig.ToByteVector(is_bls_legacy); + std::vector buf = qc.quorumSig.ToByteVector(m_use_legacy_bls); buf[5]++; - qc.quorumSig.SetByteVector(buf, is_bls_legacy); + qc.quorumSig.SetByteVector(buf, m_use_legacy_bls); } t3.stop(); diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 82a1263b923e6..1482e35e08033 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -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: diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 6bcc0e8996524..489ca42d28861 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1541,7 +1541,7 @@ std::optional 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()); diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 76332e4ce749f..ecf1d8f75a0fa 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -275,12 +275,6 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESEncryptedObject& 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); diff --git a/src/masternode/node.h b/src/masternode/node.h index 9d10cc68f0cf8..f1a54ed2cc705 100644 --- a/src/masternode/node.h +++ b/src/masternode/node.h @@ -65,7 +65,6 @@ class CActiveMasternodeManager final : public CValidationInterface template