From ec42d861262c3c99dfc36f81c73efb5b9167c79a Mon Sep 17 00:00:00 2001 From: xdustinface Date: Sat, 12 Dec 2020 16:23:05 +0100 Subject: [PATCH 1/3] llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only Current `develop` tests fail. This was basically introduced by dashpay#3844 but it didn't come up before dashpay#3853 because the `v17` fork wasn't activated in `feature_llmq_dkgerrors.py`. After dashpay#3853 `dip0008` activation takes [200 blocks](https://github.com/dashpay/dash/commit/b95cf017c34f2075637e1aa33521e1d99af441d7#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R539) which was normally activated after [10 blocks](https://github.com/dashpay/dash/commit/b95cf017c34f2075637e1aa33521e1d99af441d7#diff-b92fa0fafafa27172736ebc88f9f9b658b1160caca512a318eefb7d93d22bf3cL18) in `feature_llmq_dkgerrors.py`. Now with the 200 blocks `v17` gets activated during test which then leads to MN1, MN2 banning MN0 because it lies in DKG of `LLMQ_TEST` and `LLMQ_TEST_V17`. There are other ways to solve it, like enabling `dip0008` earlier or enable `v17` later but IMO its anyway better to restrict `ShouldSimulateError` to only trigger for `LLMQ_TEST`. --- src/llmq/quorums_dkgsession.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/llmq/quorums_dkgsession.cpp b/src/llmq/quorums_dkgsession.cpp index afd2af55f2d3..cfdaffad73ae 100644 --- a/src/llmq/quorums_dkgsession.cpp +++ b/src/llmq/quorums_dkgsession.cpp @@ -51,8 +51,11 @@ static double GetSimulatedErrorRate(const std::string& type) return 0; } -static bool ShouldSimulateError(const std::string& type) +static bool ShouldSimulateError(const Consensus::LLMQType llmqType, const std::string& type) { + if (llmqType != Consensus::LLMQType::LLMQ_TEST) { + return false; + } double rate = GetSimulatedErrorRate(type); return GetRandBool(rate); } @@ -162,7 +165,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages) logger.Batch("sending contributions"); - if (ShouldSimulateError("contribution-omit")) { + if (ShouldSimulateError(params.type, "contribution-omit")) { logger.Batch("omitting"); return; } @@ -181,7 +184,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages) auto& m = members[i]; CBLSSecretKey skContrib = skContributions[i]; - if (i != myIdx && ShouldSimulateError("contribution-lie")) { + if (i != myIdx && ShouldSimulateError(params.type, "contribution-lie")) { logger.Batch("lying for %s", m->dmn->proTxHash.ToString()); skContrib.MakeNewKey(); } @@ -323,7 +326,7 @@ void CDKGSession::ReceiveMessage(const uint256& hash, const CDKGContribution& qc if (!qc.contributions->Decrypt(myIdx, *activeMasternodeInfo.blsKeyOperator, skContribution, PROTOCOL_VERSION)) { logger.Batch("contribution from %s could not be decrypted", member->dmn->proTxHash.ToString()); complain = true; - } else if (member->idx != myIdx && ShouldSimulateError("complain-lie")) { + } else if (member->idx != myIdx && ShouldSimulateError(params.type, "complain-lie")) { logger.Batch("lying/complaining for %s", member->dmn->proTxHash.ToString()); complain = true; } @@ -700,7 +703,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const CBLSSecretKey skContribution = skContributions[i]; - if (i != myIdx && ShouldSimulateError("justify-lie")) { + if (i != myIdx && ShouldSimulateError(params.type, "justify-lie")) { logger.Batch("lying for %s", m->dmn->proTxHash.ToString()); skContribution.MakeNewKey(); } @@ -708,7 +711,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const qj.contributions.emplace_back(i, skContribution); } - if (ShouldSimulateError("justify-omit")) { + if (ShouldSimulateError(params.type, "justify-omit")) { logger.Batch("omitting"); return; } @@ -957,7 +960,7 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) return; } - if (ShouldSimulateError("commit-omit")) { + if (ShouldSimulateError(params.type, "commit-omit")) { logger.Batch("omitting"); return; } @@ -995,7 +998,7 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) qc.quorumVvecHash = ::SerializeHash(*vvec); int lieType = -1; - if (ShouldSimulateError("commit-lie")) { + if (ShouldSimulateError(params.type, "commit-lie")) { lieType = GetRandInt(5); logger.Batch("lying on commitment. lieType=%d", lieType); } From bf9ee0f1f52e88ee12501595e6abf8a79533437f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 13 Dec 2020 01:34:58 +0300 Subject: [PATCH 2/3] Revert "llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only" This reverts commit ec42d861262c3c99dfc36f81c73efb5b9167c79a. --- src/llmq/quorums_dkgsession.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/llmq/quorums_dkgsession.cpp b/src/llmq/quorums_dkgsession.cpp index cfdaffad73ae..afd2af55f2d3 100644 --- a/src/llmq/quorums_dkgsession.cpp +++ b/src/llmq/quorums_dkgsession.cpp @@ -51,11 +51,8 @@ static double GetSimulatedErrorRate(const std::string& type) return 0; } -static bool ShouldSimulateError(const Consensus::LLMQType llmqType, const std::string& type) +static bool ShouldSimulateError(const std::string& type) { - if (llmqType != Consensus::LLMQType::LLMQ_TEST) { - return false; - } double rate = GetSimulatedErrorRate(type); return GetRandBool(rate); } @@ -165,7 +162,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages) logger.Batch("sending contributions"); - if (ShouldSimulateError(params.type, "contribution-omit")) { + if (ShouldSimulateError("contribution-omit")) { logger.Batch("omitting"); return; } @@ -184,7 +181,7 @@ void CDKGSession::SendContributions(CDKGPendingMessages& pendingMessages) auto& m = members[i]; CBLSSecretKey skContrib = skContributions[i]; - if (i != myIdx && ShouldSimulateError(params.type, "contribution-lie")) { + if (i != myIdx && ShouldSimulateError("contribution-lie")) { logger.Batch("lying for %s", m->dmn->proTxHash.ToString()); skContrib.MakeNewKey(); } @@ -326,7 +323,7 @@ void CDKGSession::ReceiveMessage(const uint256& hash, const CDKGContribution& qc if (!qc.contributions->Decrypt(myIdx, *activeMasternodeInfo.blsKeyOperator, skContribution, PROTOCOL_VERSION)) { logger.Batch("contribution from %s could not be decrypted", member->dmn->proTxHash.ToString()); complain = true; - } else if (member->idx != myIdx && ShouldSimulateError(params.type, "complain-lie")) { + } else if (member->idx != myIdx && ShouldSimulateError("complain-lie")) { logger.Batch("lying/complaining for %s", member->dmn->proTxHash.ToString()); complain = true; } @@ -703,7 +700,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const CBLSSecretKey skContribution = skContributions[i]; - if (i != myIdx && ShouldSimulateError(params.type, "justify-lie")) { + if (i != myIdx && ShouldSimulateError("justify-lie")) { logger.Batch("lying for %s", m->dmn->proTxHash.ToString()); skContribution.MakeNewKey(); } @@ -711,7 +708,7 @@ void CDKGSession::SendJustification(CDKGPendingMessages& pendingMessages, const qj.contributions.emplace_back(i, skContribution); } - if (ShouldSimulateError(params.type, "justify-omit")) { + if (ShouldSimulateError("justify-omit")) { logger.Batch("omitting"); return; } @@ -960,7 +957,7 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) return; } - if (ShouldSimulateError(params.type, "commit-omit")) { + if (ShouldSimulateError("commit-omit")) { logger.Batch("omitting"); return; } @@ -998,7 +995,7 @@ void CDKGSession::SendCommitment(CDKGPendingMessages& pendingMessages) qc.quorumVvecHash = ::SerializeHash(*vvec); int lieType = -1; - if (ShouldSimulateError(params.type, "commit-lie")) { + if (ShouldSimulateError("commit-lie")) { lieType = GetRandInt(5); logger.Batch("lying on commitment. lieType=%d", lieType); } From 78927767b9c0998615dd24f4f908049934bcd339 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 13 Dec 2020 04:41:30 +0300 Subject: [PATCH 3/3] llmq: Restrict `ShouldSimulateError` to trigger for LLMQ_TEST only (alternative) Move ShouldSimulateError into CDKGSession --- src/llmq/quorums_dkgsession.cpp | 5 ++++- src/llmq/quorums_dkgsession.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/llmq/quorums_dkgsession.cpp b/src/llmq/quorums_dkgsession.cpp index afd2af55f2d3..56c6861415cc 100644 --- a/src/llmq/quorums_dkgsession.cpp +++ b/src/llmq/quorums_dkgsession.cpp @@ -51,8 +51,11 @@ static double GetSimulatedErrorRate(const std::string& type) return 0; } -static bool ShouldSimulateError(const std::string& type) +bool CDKGSession::ShouldSimulateError(const std::string& type) { + if (params.type != Consensus::LLMQType::LLMQ_TEST) { + return false; + } double rate = GetSimulatedErrorRate(type); return GetRandBool(rate); } diff --git a/src/llmq/quorums_dkgsession.h b/src/llmq/quorums_dkgsession.h index e0a647f07584..7c2863ff70ec 100644 --- a/src/llmq/quorums_dkgsession.h +++ b/src/llmq/quorums_dkgsession.h @@ -340,6 +340,9 @@ class CDKGSession public: CDKGMember* GetMember(const uint256& proTxHash) const; + +private: + bool ShouldSimulateError(const std::string& type); }; void SetSimulatedDKGErrorRate(const std::string& type, double rate);