Put height into mined commitments and use it instead of the special handling of quorumVvecHash (#2501)

* Allow to skip sig verification for CFinalCommitment::Verify

* Add CFinalCommitmentTxPayload and CheckLLMQCommitment and use it

As described in https://github.com/dashpay/dips/pull/31 (see discussion).

* Properly ban nodes for invalid commitments
This commit is contained in:
Alexander Block 2018-11-27 08:04:08 +01:00 committed by GitHub
parent ed53fce470
commit 812834dc5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 111 additions and 41 deletions

View File

@ -674,12 +674,12 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C
LogPrintf("CDeterministicMNManager::%s -- MN %s revoked operator key at height %d: %s\n",
__func__, proTx.proTxHash.ToString(), nHeight, proTx.ToString());
} else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) {
llmq::CFinalCommitment qc;
llmq::CFinalCommitmentTxPayload qc;
if (!GetTxPayload(tx, qc)) {
assert(false); // this should have been handled already
}
if (!qc.IsNull()) {
HandleQuorumCommitment(qc, newList);
if (!qc.commitment.IsNull()) {
HandleQuorumCommitment(qc.commitment, newList);
}
}
}

View File

@ -38,7 +38,7 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVali
case TRANSACTION_COINBASE:
return CheckCbTx(tx, pindexPrev, state);
case TRANSACTION_QUORUM_COMMITMENT:
return true; // can't really check much here. checks are done in ProcessBlock
return llmq::CheckLLMQCommitment(tx, pindexPrev, state);
}
return state.DoS(10, false, REJECT_INVALID, "bad-tx-type-check");

View File

@ -90,7 +90,7 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC
auto members = CLLMQUtils::GetAllQuorumMembers(type, qc.quorumHash);
if (!qc.Verify(members)) {
if (!qc.Verify(members, true)) {
LOCK(cs_main);
LogPrintf("CQuorumBlockProcessor::%s -- commitment for quorum %s:%d is not valid, peer=%d\n", __func__,
qc.quorumHash.ToString(), qc.llmqType, pfrom->id);
@ -167,7 +167,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(const CBlockIndex* pindexPrev, con
}
if (qc.IsNull()) {
if (!qc.VerifyNull(pindexPrev->nHeight + 1)) {
if (!qc.VerifyNull()) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid-null");
}
return true;
@ -185,7 +185,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(const CBlockIndex* pindexPrev, con
auto members = CLLMQUtils::GetAllQuorumMembers(params.type, quorumHash);
if (!qc.Verify(members)) {
if (!qc.Verify(members, true)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid");
}
@ -236,21 +236,18 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const C
for (const auto& tx : block.vtx) {
if (tx->nType == TRANSACTION_QUORUM_COMMITMENT) {
CFinalCommitment qc;
CFinalCommitmentTxPayload qc;
if (!GetTxPayload(*tx, qc)) {
// should not happen as it was verified before processing the block
return state.DoS(100, false, REJECT_INVALID, "bad-tx-payload");
}
if (!consensus.llmqs.count((Consensus::LLMQType)qc.llmqType)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-type");
}
// only allow one commitment per type and per block
if (ret.count((Consensus::LLMQType)qc.llmqType)) {
if (ret.count((Consensus::LLMQType)qc.commitment.llmqType)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-dup");
}
ret.emplace((Consensus::LLMQType)qc.llmqType, std::move(qc));
ret.emplace((Consensus::LLMQType)qc.commitment.llmqType, std::move(qc.commitment));
}
}
@ -406,7 +403,6 @@ bool CQuorumBlockProcessor::GetMinableCommitment(Consensus::LLMQType llmqType, c
if (it == minableCommitmentsByQuorum.end()) {
// null commitment required
ret = CFinalCommitment(Params().GetConsensus().llmqs.at(llmqType), quorumHash);
ret.quorumVvecHash = ::SerializeHash(std::make_pair(quorumHash, nHeight));
return true;
}
@ -419,11 +415,13 @@ bool CQuorumBlockProcessor::GetMinableCommitmentTx(Consensus::LLMQType llmqType,
{
AssertLockHeld(cs_main);
CFinalCommitment qc;
if (!GetMinableCommitment(llmqType, pindexPrev, qc)) {
CFinalCommitmentTxPayload qc;
if (!GetMinableCommitment(llmqType, pindexPrev, qc.commitment)) {
return false;
}
qc.nHeight = pindexPrev->nHeight + 1;
CMutableTransaction tx;
tx.nVersion = 3;
tx.nType = TRANSACTION_QUORUM_COMMITMENT;

View File

@ -6,6 +6,9 @@
#include "quorums_utils.h"
#include "chainparams.h"
#include "validation.h"
#include "evo/specialtx.h"
#include <univalue.h>
@ -24,7 +27,7 @@ CFinalCommitment::CFinalCommitment(const Consensus::LLMQParams& params, const ui
LogPrintStr(strprintf("CFinalCommitment::%s -- %s", __func__, tinyformat::format(__VA_ARGS__))); \
} while(0)
bool CFinalCommitment::Verify(const std::vector<CDeterministicMNCPtr>& members) const
bool CFinalCommitment::Verify(const std::vector<CDeterministicMNCPtr>& members, bool checkSigs) const
{
if (nVersion == 0 || nVersion > CURRENT_VERSION) {
return false;
@ -76,30 +79,33 @@ bool CFinalCommitment::Verify(const std::vector<CDeterministicMNCPtr>& members)
}
}
uint256 commitmentHash = CLLMQUtils::BuildCommitmentHash((uint8_t)params.type, quorumHash, validMembers, quorumPublicKey, quorumVvecHash);
// sigs are only checked when the block is processed
if (checkSigs) {
uint256 commitmentHash = CLLMQUtils::BuildCommitmentHash((uint8_t)params.type, quorumHash, validMembers, quorumPublicKey, quorumVvecHash);
std::vector<CBLSPublicKey> memberPubKeys;
for (size_t i = 0; i < members.size(); i++) {
if (!signers[i]) {
continue;
std::vector<CBLSPublicKey> memberPubKeys;
for (size_t i = 0; i < members.size(); i++) {
if (!signers[i]) {
continue;
}
memberPubKeys.emplace_back(members[i]->pdmnState->pubKeyOperator);
}
memberPubKeys.emplace_back(members[i]->pdmnState->pubKeyOperator);
}
if (!membersSig.VerifySecureAggregated(memberPubKeys, commitmentHash)) {
LogPrintfFinalCommitment("invalid aggregated members signature\n");
return false;
}
if (!membersSig.VerifySecureAggregated(memberPubKeys, commitmentHash)) {
LogPrintfFinalCommitment("invalid aggregated members signature\n");
return false;
}
if (!quorumSig.VerifyInsecure(quorumPublicKey, commitmentHash)) {
LogPrintfFinalCommitment("invalid quorum signature\n");
return false;
if (!quorumSig.VerifyInsecure(quorumPublicKey, commitmentHash)) {
LogPrintfFinalCommitment("invalid quorum signature\n");
return false;
}
}
return true;
}
bool CFinalCommitment::VerifyNull(int nHeight) const
bool CFinalCommitment::VerifyNull() const
{
if (!Params().GetConsensus().llmqs.count((Consensus::LLMQType)llmqType)) {
LogPrintfFinalCommitment("invalid llmqType=%d\n", llmqType);
@ -111,11 +117,6 @@ bool CFinalCommitment::VerifyNull(int nHeight) const
return false;
}
uint256 expectedQuorumVvecHash = ::SerializeHash(std::make_pair(quorumHash, nHeight));
if (quorumVvecHash != expectedQuorumVvecHash) {
return false;
}
return true;
}
@ -143,4 +144,50 @@ void CFinalCommitment::ToJson(UniValue& obj) const
obj.push_back(Pair("quorumPublicKey", quorumPublicKey.ToString()));
}
bool CheckLLMQCommitment(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state)
{
CFinalCommitmentTxPayload qcTx;
if (!GetTxPayload(tx, qcTx)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-payload");
}
if (qcTx.nVersion == 0 || qcTx.nVersion > CFinalCommitmentTxPayload::CURRENT_VERSION) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-version");
}
if (qcTx.nHeight != pindexPrev->nHeight + 1) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-height");
}
if (!mapBlockIndex.count(qcTx.commitment.quorumHash)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-quorum-hash");
}
const CBlockIndex* pindexQuorum = mapBlockIndex[qcTx.commitment.quorumHash];
if (pindexQuorum != pindexPrev->GetAncestor(pindexQuorum->nHeight)) {
// not part of active chain
return state.DoS(100, false, REJECT_INVALID, "bad-qc-quorum-hash");
}
if (!Params().GetConsensus().llmqs.count((Consensus::LLMQType)qcTx.commitment.llmqType)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-type");
}
const auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)qcTx.commitment.llmqType);
if (qcTx.commitment.IsNull()) {
if (!qcTx.commitment.VerifyNull()) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid-null");
}
return true;
}
auto members = CLLMQUtils::GetAllQuorumMembers(params.type, qcTx.commitment.quorumHash);
if (!qcTx.commitment.Verify(members, false)) {
return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid");
}
return true;
}
}

View File

@ -48,8 +48,8 @@ public:
return (int)std::count(validMembers.begin(), validMembers.end(), true);
}
bool Verify(const std::vector<CDeterministicMNCPtr>& members) const;
bool VerifyNull(int nHeight) const;
bool Verify(const std::vector<CDeterministicMNCPtr>& members, bool checkSigs) const;
bool VerifyNull() const;
bool VerifySizes(const Consensus::LLMQParams& params) const;
void ToJson(UniValue& obj) const;
@ -79,6 +79,7 @@ public:
return false;
}
if (quorumPublicKey.IsValid() ||
!quorumVvecHash.IsNull() ||
membersSig.IsValid() ||
quorumSig.IsValid()) {
return false;
@ -87,6 +88,30 @@ public:
}
};
class CFinalCommitmentTxPayload
{
public:
static const uint16_t CURRENT_VERSION = 1;
public:
uint16_t nVersion{CURRENT_VERSION};
uint32_t nHeight{(uint32_t)-1};
CFinalCommitment commitment;
public:
ADD_SERIALIZE_METHODS
template<typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action)
{
READWRITE(nVersion);
READWRITE(nHeight);
READWRITE(commitment);
}
};
bool CheckLLMQCommitment(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state);
}
#endif //DASH_QUORUMS_COMMITMENT_H

View File

@ -345,7 +345,7 @@ void CDummyDKG::CreateFinalCommitment(Consensus::LLMQType llmqType, const CBlock
}
fqc.membersSig = CBLSSignature::AggregateSecure(memberSigs, memberPubKeys, commitmentHash);
if (!fqc.Verify(members)) {
if (!fqc.Verify(members, true)) {
LogPrintf("CDummyDKG::%s -- self created final commitment is invalid for quorum %s:%d, validMembers=%d, signers=%d\n", __func__,
fqc.quorumHash.ToString(), fqc.llmqType, fqc.CountValidMembers(), fqc.CountSigners());
continue;