diff --git a/contrib/flatpak/org.dash.dash-core.metainfo.xml b/contrib/flatpak/org.dash.dash-core.metainfo.xml index deb15c56e221..6cbc10d952fe 100644 --- a/contrib/flatpak/org.dash.dash-core.metainfo.xml +++ b/contrib/flatpak/org.dash.dash-core.metainfo.xml @@ -22,6 +22,7 @@ + diff --git a/doc/release-notes.md b/doc/release-notes.md index 2f1c11f9b3bf..4b89c8da4d63 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -1,9 +1,8 @@ -# Dash Core version v23.1.9 +# Dash Core version v23.1.10 -This is a new patch version release. It fixes a consensus-affecting -transaction-encoding bug, several remotely reachable crashes and -denial-of-service issues in the masternode, DKG, signing and governance paths, -and backports a number of hardening and robustness fixes from later releases. +This is a new patch version release. It fixes a governance issue that could +stop a superblock from paying any proposals, and hardens DKG and BLS signature +verification against misbehaving quorum members and peers. Upgrading is **strongly recommended** for all nodes, and required for masternodes. @@ -30,91 +29,54 @@ require a reindex. # Release Notes -## Consensus - -- Transactions whose version field is negative are now rejected once DIP3 is - active. The version and type share one 32-bit field; a negative version was - sign-extended when a transaction was re-serialized, so a block containing such - a transaction could be accepted by a node that received the original bytes and - rejected by every node that received the re-serialized form, leading to a - persistent chain split. The new rule rejects these transactions uniformly. No - valid or historical transaction is affected and transaction ids are unchanged. - -## Crash fixes - -These issues do not affect consensus rules or put funds at risk, but each one -can take a node offline. - -- Fixed an abort during the DKG when a quorum member sent a contribution whose - decrypted secret-key share was invalid: the share reached an aggregation - assertion and terminated the process on every honest member of the quorum. - Invalid shares are now rejected before aggregation. -- Fixed block-template failures caused by provider transactions: a - `ProUpServTx` with an invalid type, a `ProRegTx` reusing collateral already - referenced by an in-mempool update, and a pending owner key colliding with a - Platform node id are now rejected at mempool acceptance. -- Bounded the masternode-list caches by size so a peer cannot drive unbounded - memory growth between blocks. -- Bounded `DomainPort` deserialization so a small provider transaction can no - longer make the node allocate and zero very large buffers while holding - `cs_main`. -- Decoding an unknown-version provider transaction through the verbose decode - RPCs no longer dereferences a missing network-info object. -- `masternode payments` no longer aborts at the genesis block, and the - masternode-list RPCs no longer cache a list for a block outside the active - chain. -- Evonodes no longer abort at credit-pool snapshot boundaries when the pool is - first constructed outside a block connection (for example by - `getassetunlockstatuses`). -- Flat-file (block and undo) positions are now stored and seeked as 64-bit - values. Block-undo data can be several times the size of its block, so a busy - chain could push a data file past the point a 32-bit offset can address; the - write and seek paths no longer narrow the position to 32 bits, which avoided a - silent wrap (and, on Windows, a failure past 2 GiB). - -## InstantSend - -- Provider (masternode) transactions are no longer given InstantSend locks. - Locking them served no purpose and interacted badly with lock eviction on - mempool removal. Software that waited for an InstantSend lock on a provider - transaction should wait for a ChainLock or for confirmations instead. +## Governance + +- Masternodes no longer vote NO-FUNDING on a superblock trigger before that + superblock's maturity window opens (about 3 days before the superblock on + mainnet), when they have no candidate of their own to compare it against. + These early votes could keep a trigger with the correct payments from + reaching the funding threshold, so the superblock paid no proposals. + Triggers that don't match a masternode's candidate still receive a NO vote + once the window opens. +- Masternodes no longer vote on triggers until they are fully synced. A + masternode restarting inside the maturity window could otherwise vote NO on + the correct trigger before it was able to build its own candidate. + +Masternodes running older versions still cast the early NO votes, so this +protection becomes effective as masternodes upgrade. Masternode operators +should upgrade before the next superblock. ## Security and denial-of-service hardening These issues do not affect consensus and do not put funds at risk. -- DKG contribution verification: a single verification-vector coefficient - shared between two members no longer fails the whole verification batch. - Previously that marked unrelated honest members' contributions invalid and - caused them to be complained about; contributions are now verified - individually, so only the offending member is affected. -- Signature-share batch verification is now scoped per sender, so one peer can - no longer get an honest peer penalised or have a forged share stored in place - of an honest one. Malformed detached quorum share batches are rejected, and - signing intake is gated against the active-quorum set before any quorum is - retrieved. -- Governance: an orphan vote now requires a valid masternode signature before - it is cached, the number of orphan parent objects re-requested per tick is - bounded, governance vote signature checks are memoised to bound the cost of a - sync request, a vote is no longer recorded before it is validated, and a - failed BLS vote-signature check is logged under the governance category - instead of unconditionally, so a peer cannot fill `debug.log` with it. -- Socket exception monitoring is now consistent between the `poll()` and - `select()` backends, and a peer whose send buffer stalls is now subject to the - send timeout, so it can no longer hold a connection slot indefinitely. -- Verified inbound connections are bounded per operator identity; `CHECKDATASIG` - uses the signature cache; repeated authenticated Platform bans are skipped; - `DYNBITSET` allocation is bounded against the remaining stream size; a bogus - InstantSend lock no longer drops a batch of unrelated locks; and - `-allowprivatenet` no longer influences masternode address validation. - -## Wallet - -- Invalid-mnemonic errors no longer echo the recovery phrase, an encrypted - wallet is relocked if an HD upgrade fails after it was unlocked, and a wallet - unlock deadline can no longer be extended without re-authenticating. - -# v23.1.9 Change log +- BLS batch verification (signature shares, recovered signatures, InstantSend + locks and DKG messages) now weights each signature with a secret random + factor before aggregating. Previously two invalid signatures whose errors + cancelled out could pass as a valid batch. A peer could use this to disrupt + InstantSend or ChainLock signing, get honest peers penalised, or get honest + DKG members marked bad. +- Batched verification of DKG contributions is weighted the same way, so + colluding members can no longer give another member invalid secret-key + shares that pass verification, which could leave that member unable to sign + for the quorum. This did not expose the quorum key. +- A DKG member copying another member's entire contribution could prevent the + whole quorum from forming. Members whose verification vectors share a + coefficient are now excluded from the commitment instead of the round + failing. This excludes both the copying member and the member it copied, + and both receive the usual PoSe penalty for failed DKG participation. +- Colluding DKG members could stop chosen honest members from building their + secret-key share, which could prevent a quorum from forming without any + member being blamed. The same condition made a node penalise an honest peer + serving quorum data during share recovery. Equal secret-key shares from + different members are now accepted; the resulting key share is still + verified against the quorum's verification vector. + +Like the governance fix, the DKG fixes protect a quorum once enough of its +members run this version. Weighted batch verification does more work per +message, so masternodes may see higher CPU use during DKG and signing. + +# v23.1.10 Change log See detailed [set of changes][set-of-changes]. @@ -122,8 +84,6 @@ See detailed [set of changes][set-of-changes]. Thanks to everyone who directly contributed to this release: -- Konstantin Akimov -- PastaClaw - PastaPastaPasta - UdjinM6 @@ -134,6 +94,7 @@ debug the release candidates. These releases are considered obsolete. Old release notes can be found here: +- [v23.1.9](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.9.md) released Oct/29/2026 - [v23.1.8](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.8.md) released Jul/29/2026 - [v23.1.7](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.7.md) released Jun/30/2026 - [v23.1.5](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.5.md) released Jun/19/2026 @@ -154,4 +115,4 @@ These releases are considered obsolete. Old release notes can be found here: - [v21.0.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-21.0.0.md) released Jul/25/2024 - [v20.1.1](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-20.1.1.md) released April/3/2024 -[set-of-changes]: https://github.com/dashpay/dash/compare/v23.1.8...dashpay:v23.1.9 +[set-of-changes]: https://github.com/dashpay/dash/compare/v23.1.9...dashpay:v23.1.10 diff --git a/doc/release-notes/dash/release-notes-23.1.9.md b/doc/release-notes/dash/release-notes-23.1.9.md new file mode 100644 index 000000000000..2f1c11f9b3bf --- /dev/null +++ b/doc/release-notes/dash/release-notes-23.1.9.md @@ -0,0 +1,157 @@ +# Dash Core version v23.1.9 + +This is a new patch version release. It fixes a consensus-affecting +transaction-encoding bug, several remotely reachable crashes and +denial-of-service issues in the masternode, DKG, signing and governance paths, +and backports a number of hardening and robustness fixes from later releases. + +Upgrading is **strongly recommended** for all nodes, and required for +masternodes. + +Please report bugs using the issue tracker at GitHub: + + + +# Upgrading and downgrading + +## How to Upgrade + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes for older versions), then run the +installer (on Windows) or just copy over /Applications/Dash-Qt (on Mac) or +dashd/dash-qt (on Linux). + +## Downgrade warning + +### Downgrade to a version < v23.0.0 + +Downgrading to a version older than v23.0.0 is not supported, and will +require a reindex. + +# Release Notes + +## Consensus + +- Transactions whose version field is negative are now rejected once DIP3 is + active. The version and type share one 32-bit field; a negative version was + sign-extended when a transaction was re-serialized, so a block containing such + a transaction could be accepted by a node that received the original bytes and + rejected by every node that received the re-serialized form, leading to a + persistent chain split. The new rule rejects these transactions uniformly. No + valid or historical transaction is affected and transaction ids are unchanged. + +## Crash fixes + +These issues do not affect consensus rules or put funds at risk, but each one +can take a node offline. + +- Fixed an abort during the DKG when a quorum member sent a contribution whose + decrypted secret-key share was invalid: the share reached an aggregation + assertion and terminated the process on every honest member of the quorum. + Invalid shares are now rejected before aggregation. +- Fixed block-template failures caused by provider transactions: a + `ProUpServTx` with an invalid type, a `ProRegTx` reusing collateral already + referenced by an in-mempool update, and a pending owner key colliding with a + Platform node id are now rejected at mempool acceptance. +- Bounded the masternode-list caches by size so a peer cannot drive unbounded + memory growth between blocks. +- Bounded `DomainPort` deserialization so a small provider transaction can no + longer make the node allocate and zero very large buffers while holding + `cs_main`. +- Decoding an unknown-version provider transaction through the verbose decode + RPCs no longer dereferences a missing network-info object. +- `masternode payments` no longer aborts at the genesis block, and the + masternode-list RPCs no longer cache a list for a block outside the active + chain. +- Evonodes no longer abort at credit-pool snapshot boundaries when the pool is + first constructed outside a block connection (for example by + `getassetunlockstatuses`). +- Flat-file (block and undo) positions are now stored and seeked as 64-bit + values. Block-undo data can be several times the size of its block, so a busy + chain could push a data file past the point a 32-bit offset can address; the + write and seek paths no longer narrow the position to 32 bits, which avoided a + silent wrap (and, on Windows, a failure past 2 GiB). + +## InstantSend + +- Provider (masternode) transactions are no longer given InstantSend locks. + Locking them served no purpose and interacted badly with lock eviction on + mempool removal. Software that waited for an InstantSend lock on a provider + transaction should wait for a ChainLock or for confirmations instead. + +## Security and denial-of-service hardening + +These issues do not affect consensus and do not put funds at risk. + +- DKG contribution verification: a single verification-vector coefficient + shared between two members no longer fails the whole verification batch. + Previously that marked unrelated honest members' contributions invalid and + caused them to be complained about; contributions are now verified + individually, so only the offending member is affected. +- Signature-share batch verification is now scoped per sender, so one peer can + no longer get an honest peer penalised or have a forged share stored in place + of an honest one. Malformed detached quorum share batches are rejected, and + signing intake is gated against the active-quorum set before any quorum is + retrieved. +- Governance: an orphan vote now requires a valid masternode signature before + it is cached, the number of orphan parent objects re-requested per tick is + bounded, governance vote signature checks are memoised to bound the cost of a + sync request, a vote is no longer recorded before it is validated, and a + failed BLS vote-signature check is logged under the governance category + instead of unconditionally, so a peer cannot fill `debug.log` with it. +- Socket exception monitoring is now consistent between the `poll()` and + `select()` backends, and a peer whose send buffer stalls is now subject to the + send timeout, so it can no longer hold a connection slot indefinitely. +- Verified inbound connections are bounded per operator identity; `CHECKDATASIG` + uses the signature cache; repeated authenticated Platform bans are skipped; + `DYNBITSET` allocation is bounded against the remaining stream size; a bogus + InstantSend lock no longer drops a batch of unrelated locks; and + `-allowprivatenet` no longer influences masternode address validation. + +## Wallet + +- Invalid-mnemonic errors no longer echo the recovery phrase, an encrypted + wallet is relocked if an HD upgrade fails after it was unlocked, and a wallet + unlock deadline can no longer be extended without re-authenticating. + +# v23.1.9 Change log + +See detailed [set of changes][set-of-changes]. + +# Credits + +Thanks to everyone who directly contributed to this release: + +- Konstantin Akimov +- PastaClaw +- PastaPastaPasta +- UdjinM6 + +As well as everyone that submitted issues, reviewed pull requests and helped +debug the release candidates. + +# Older releases + +These releases are considered obsolete. Old release notes can be found here: + +- [v23.1.8](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.8.md) released Jul/29/2026 +- [v23.1.7](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.7.md) released Jun/30/2026 +- [v23.1.5](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.5.md) released Jun/19/2026 +- [v23.1.4](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.4.md) released Jun/18/2026 +- [v23.1.3](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.3.md) released May/28/2026 +- [v23.1.2](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.2.md) released Mar/12/2026 +- [v23.1.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.0.md) released Feb/15/2026 +- [v23.0.2](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.0.2.md) released Dec/4/2025 +- [v23.0.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.0.0.md) released Nov/10/2025 +- [v22.1.3](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-22.1.3.md) released Jul/15/2025 +- [v22.1.2](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-22.1.2.md) released Apr/15/2025 +- [v22.1.1](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-22.1.1.md) released Feb/17/2025 +- [v22.1.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-22.1.0.md) released Feb/10/2025 +- [v22.0.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-22.0.0.md) released Dec/12/2024 +- [v21.1.1](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-21.1.1.md) released Oct/22/2024 +- [v21.1.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-21.1.0.md) released Aug/8/2024 +- [v21.0.2](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-21.0.2.md) released Aug/1/2024 +- [v21.0.0](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-21.0.0.md) released Jul/25/2024 +- [v20.1.1](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-20.1.1.md) released April/3/2024 + +[set-of-changes]: https://github.com/dashpay/dash/compare/v23.1.8...dashpay:v23.1.9 diff --git a/src/active/dkgsession.cpp b/src/active/dkgsession.cpp index ecf83d6890e7..d3a7753c984b 100644 --- a/src/active/dkgsession.cpp +++ b/src/active/dkgsession.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -50,7 +51,23 @@ std::optional ActiveDKGSession::Contribute() cxxtimer::Timer t1(true); logger.Batch("generating contributions"); - if (!blsWorker.GenerateContributions(params.threshold, memberIds, vvecContribution, m_sk_contributions)) { + if (ShouldSimulateError(DKGError::type::CONTRIBUTION_CLONE)) { + // Derive the contribution from the quorum alone, so every member simulating this deals the same + // contribution, as if it had copied another member's + logger.Batch("dealing a cloned contribution"); + std::vector svec(params.threshold); + vvecContribution = std::make_shared>(); + for (const auto i : util::irange(svec.size())) { + uint256 seed{::SerializeHash(std::make_pair(m_quorum_base_block_index->GetBlockHash(), uint32_t(i)))}; + *seed.begin() = 0; // keep it below the group order + svec[i] = CBLSSecretKey{Span{seed}}; + vvecContribution->emplace_back(svec[i].GetPublicKey()); + } + m_sk_contributions.resize(memberIds.size()); + for (const auto i : util::irange(memberIds.size())) { + m_sk_contributions[i].SecretKeyShare(svec, memberIds[i]); + } + } else if (!blsWorker.GenerateContributions(params.threshold, memberIds, vvecContribution, m_sk_contributions)) { // this should never happen actually logger.Batch("GenerateContributions failed"); return std::nullopt; @@ -122,9 +139,9 @@ std::optional ActiveDKGSession::SendContributions() } // Verifies all pending secret key contributions in one batch -// This is done by aggregating the verification vectors belonging to the secret key contributions +// This is done by aggregating randomly weighted verification vectors belonging to the secret key contributions // The resulting aggregated vvec is then used to recover a public key share -// The public key share must match the public key belonging to the aggregated secret key contributions +// The public key share must match the equally weighted public keys of the secret key contributions // See CBLSWorker::VerifyContributionShares for more details. void ActiveDKGSession::VerifyPendingContributions() { @@ -141,6 +158,7 @@ void ActiveDKGSession::VerifyPendingContributions() std::vector memberIndexes; std::vector vvecs; std::vector skContributions; + bool anyLegacyVvec{false}; for (const auto& idx : pendingContributionVerifications) { const auto& m = members[idx]; @@ -149,6 +167,7 @@ void ActiveDKGSession::VerifyPendingContributions() } memberIndexes.emplace_back(idx); vvecs.emplace_back(receivedVvecs[idx]); + anyLegacyVvec |= receivedVvecsLegacyScheme[idx]; skContributions.emplace_back(receivedSkContributions[idx]); // Write here to definitely store one contribution for each member no matter if // our share is valid or not, could be that others are still correct @@ -166,7 +185,9 @@ void ActiveDKGSession::VerifyPendingContributions() return; } - auto result = blsWorker.VerifyContributionShares(myId, vvecs, skContributions); + // Weighted aggregation needs points that passed subgroup checks, which legacy-scheme deserialization skips + auto result = blsWorker.VerifyContributionShares(myId, vvecs, skContributions, /*parallel=*/true, + /*aggregated=*/!anyLegacyVvec); if (result.size() != memberIndexes.size()) { logger.Batch("VerifyContributionShares returned result of size %d but size %d was expected, something is wrong", result.size(), memberIndexes.size()); return; @@ -454,7 +475,25 @@ std::optional ActiveDKGSession::VerifyAndCommit() } } - if (!badMembers.empty() || !openComplaintMembers.empty()) { + // A member can copy another member's whole contribution: every share verifies, but the duplicated + // coefficients make the quorum verification vector impossible to build, so no commitment could be made. + // The copy can't be told from the original, so drop both; members that agree on the remaining members + // and hold the same contributions drop the same ones and still agree on the commitment. + std::vector remainingMembers; + std::vector remainingVvecs; + for (const auto& m : members) { + if (!m->bad) { + remainingMembers.emplace_back(m->idx); + remainingVvecs.emplace_back(receivedVvecs[m->idx]); + } + } + std::vector sharedCoefficientMembers; + for (const auto i : CBLSWorker::FindVerificationVectorsWithSharedCoefficients(remainingVvecs)) { + MarkBadMember(remainingMembers[i]); + sharedCoefficientMembers.emplace_back(remainingMembers[i]); + } + + if (!badMembers.empty() || !openComplaintMembers.empty() || !sharedCoefficientMembers.empty()) { logger.Batch("verification result:"); } if (!badMembers.empty()) { @@ -469,6 +508,12 @@ std::optional ActiveDKGSession::VerifyAndCommit() logger.Batch(" %s", members[idx]->dmn->proTxHash.ToString()); } } + if (!sharedCoefficientMembers.empty()) { + logger.Batch(" members sharing verification vector coefficients and now marked as bad:"); + for (const auto& idx : sharedCoefficientMembers) { + logger.Batch(" %s", members[idx]->dmn->proTxHash.ToString()); + } + } logger.Flush(); diff --git a/src/bench/bls_dkg.cpp b/src/bench/bls_dkg.cpp index 7b60a5fe4f03..bdf9015be8b7 100644 --- a/src/bench/bls_dkg.cpp +++ b/src/bench/bls_dkg.cpp @@ -47,7 +47,8 @@ class DKG void VerifyContributionShares(size_t whoAmI, const std::set& invalidIndexes, bool aggregated) { - auto result = blsWorker.VerifyContributionShares(members[whoAmI].id, receivedVvecs, receivedSkShares, aggregated); + auto result = blsWorker.VerifyContributionShares(members[whoAmI].id, receivedVvecs, receivedSkShares, + /*parallel=*/true, aggregated); for (const size_t i : util::irange(receivedVvecs.size())) { if (invalidIndexes.count(i)) { assert(!result[i]); diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index 8d5b1b83703c..96a188e2ae93 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -205,6 +205,13 @@ bool CBLSPublicKey::PublicKeyShare(Span mpk, const CBLSId& _id) return true; } +void CBLSPublicKey::MulInsecure(const CBLSSecretKey& factor) +{ + assert(IsValid() && factor.IsValid()); + impl = factor.impl * impl; + cachedHash.SetNull(); +} + bool CBLSPublicKey::DHKeyExchange(const CBLSSecretKey& sk, const CBLSPublicKey& pk) { fValid = false; @@ -293,6 +300,13 @@ void CBLSSignature::SubInsecure(const CBLSSignature& o) cachedHash.SetNull(); } +void CBLSSignature::MulInsecure(const CBLSSecretKey& factor) +{ + assert(IsValid() && factor.IsValid()); + impl = factor.impl * impl; + cachedHash.SetNull(); +} + bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash, const bool specificLegacyScheme) const { if (!IsValid() || !pubKey.IsValid()) { diff --git a/src/bls/bls.h b/src/bls/bls.h index ae2124cd38e1..82abc2c6de04 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -314,6 +314,8 @@ class CBLSPublicKey : public CBLSWrapper pks); + void MulInsecure(const CBLSSecretKey& factor); + bool PublicKeyShare(Span mpk, const CBLSId& id); bool DHKeyExchange(const CBLSSecretKey& sk, const CBLSPublicKey& pk); @@ -360,6 +362,7 @@ class CBLSSignature : public CBLSWrapper sigs, Span pks, const uint256& hash); void SubInsecure(const CBLSSignature& o); + void MulInsecure(const CBLSSecretKey& factor); [[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash, const bool specificLegacyScheme) const; [[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const; [[nodiscard]] bool VerifyInsecureAggregated(Span pubKeys, Span hashes) const; diff --git a/src/bls/bls_batchverifier.h b/src/bls/bls_batchverifier.h index 528b527f0090..647d38e675e3 100644 --- a/src/bls/bls_batchverifier.h +++ b/src/bls/bls_batchverifier.h @@ -6,10 +6,26 @@ #define DASH_CRYPTO_BLS_BATCHVERIFIER_H #include +#include +#include +#include #include #include +//! A secret random nonzero scalar of numBytes bytes (at most 16, so well below the group order). Weighting each +//! signature and its public key by one makes errors in several signatures unable to cancel out in an aggregate. +inline CBLSSecretKey MakeBatchVerificationWeight(FastRandomContext& rng, size_t numBytes = 16) +{ + assert(numBytes > 0 && numBytes <= 16); + std::array bytes{}; + do { + const uint256 rand{rng.rand256()}; + std::copy(rand.begin(), rand.begin() + numBytes, bytes.end() - numBytes); + } while (std::all_of(bytes.begin(), bytes.end(), [](uint8_t b) { return b == 0; })); + return CBLSSecretKey{bytes}; +} + template class CBLSBatchVerifier { @@ -19,6 +35,9 @@ class CBLSBatchVerifier uint256 msgHash; CBLSSignature sig; CBLSPublicKey pubKey; + //! sig and pubKey multiplied by the same MakeBatchVerificationWeight() + CBLSSignature weightedSig; + CBLSPublicKey weightedPubKey; }; using MessageMap = std::map; @@ -32,6 +51,8 @@ class CBLSBatchVerifier MessageMap messages; MessagesBySourceMap messagesBySource; + FastRandomContext rng; + public: std::set badSources; std::set badMessages; @@ -48,7 +69,12 @@ class CBLSBatchVerifier { assert(sig.IsValid() && pubKey.IsValid()); - auto it = messages.emplace(msgId, Message{msgId, msgHash, sig, pubKey}).first; + auto [it, inserted] = messages.emplace(msgId, Message{msgId, msgHash, sig, pubKey, sig, pubKey}); + if (inserted) { + const CBLSSecretKey weight{MakeBatchVerificationWeight(rng)}; + it->second.weightedSig.MulInsecure(weight); + it->second.weightedPubKey.MulInsecure(weight); + } messagesBySource[sourceId].emplace_back(it); if (subBatchSize != 0 && messages.size() >= subBatchSize) { @@ -155,8 +181,8 @@ class CBLSBatchVerifier continue; } - sigsToAggregate.push_back(msg.sig); - pubKeysToAggregate.push_back(msg.pubKey); + sigsToAggregate.push_back(msg.weightedSig); + pubKeysToAggregate.push_back(msg.weightedPubKey); } CBLSPublicKey aggPubKey = CBLSPublicKey::AggregateInsecure(pubKeysToAggregate); @@ -210,8 +236,8 @@ class CBLSBatchVerifier if (dups.emplace(msg.msgId).second) { msgHashes.emplace_back(msgHash); - pubKeys.emplace_back(msg.pubKey); - sigsToAggregate.push_back(msg.sig); + pubKeys.emplace_back(msg.weightedPubKey); + sigsToAggregate.push_back(msg.weightedSig); } messageIts.pop_back(); diff --git a/src/bls/bls_worker.cpp b/src/bls/bls_worker.cpp index aeae68b1ed3b..2ebc5eade3a0 100644 --- a/src/bls/bls_worker.cpp +++ b/src/bls/bls_worker.cpp @@ -3,11 +3,15 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include +#include #include #include +#include +#include #include #include #include @@ -392,13 +396,6 @@ struct ContributionVerifier : public std::enable_shared_from_this > aggDone; - // we can't directly update a vector in parallel // as vector is not thread safe (uses bitsets internally) // so we must use vector temporarily and concatenate/convert @@ -451,7 +448,6 @@ struct ContributionVerifier : public std::enable_shared_from_this>(0); batchState.start = i * batchSize; batchState.count = std::min(batchSize, vvecs.size() - batchState.start); batchState.verifyResults.assign(batchState.count, 0); @@ -483,32 +479,64 @@ struct ContributionVerifier : public std::enable_shared_from_thisshared_from_this()); - auto vvecAgg = std::make_shared>(vvecs.subspan(batchState.start, batchState.count), parallel, workerPool, [this, self, batchIdx] (const BLSVerificationVectorPtr& vvec) {HandleAggVvecDone(batchIdx, vvec);}); - auto skShareAgg = std::make_shared>(Span{skShares}.subspan(batchState.start, batchState.count), parallel, workerPool, [this, self, batchIdx] (const CBLSSecretKey& skShare) {HandleAggSkShareDone(batchIdx, skShare);}); - - vvecAgg->Start(); - skShareAgg->Start(); + auto f = [this, self, batchIdx](int threadId) { + auto& batchState = batchStates[batchIdx]; + if (VerifyWeightedBatch(batchState)) { + // whole batch is valid + batchState.verifyResults.assign(batchState.count, 1); + HandleVerifyDone(batchState.count); + } else { + // at least one entry in the batch is invalid, revert to per-contribution verification (but parallelized) + AsyncVerifyBatchOneByOne(batchIdx); + } + }; + PushOrDoWork(std::move(f)); } - void HandleAggVvecDone(size_t batchIdx, const BLSVerificationVectorPtr& vvec) + // Each contribution's vvec and the public key of its share are multiplied by the same secret random weight before + // they are aggregated, so that errors in the contributions of several dealers can't cancel out. 64-bit weights + // let a batch with an invalid contribution pass with a probability of at most 2^-64. + bool VerifyWeightedBatch(const BatchState& batchState) const { - auto& batchState = batchStates[batchIdx]; - batchState.vvec = vvec; - if (++(*batchState.aggDone) == 2) { - HandleAggDone(batchIdx); + FastRandomContext rng; + std::vector aggVvec; + std::vector pubKeys; + pubKeys.reserve(batchState.count); + for (size_t i = batchState.start; i < batchState.start + batchState.count; i++) { + const auto& vvec = vvecs[i]; + if (vvec == nullptr || vvec->empty() || !skShares[i].IsValid() || + (!aggVvec.empty() && vvec->size() != aggVvec.size())) { + return false; + } + const CBLSSecretKey weight{MakeBatchVerificationWeight(rng, /*numBytes=*/8)}; + CBLSPublicKey pubKey{skShares[i].GetPublicKey()}; + pubKey.MulInsecure(weight); + pubKeys.emplace_back(pubKey); + const bool first{aggVvec.empty()}; + for (size_t j = 0; j < vvec->size(); j++) { + CBLSPublicKey coefficient{(*vvec)[j]}; + if (!coefficient.IsValid()) { + return false; + } + coefficient.MulInsecure(weight); + if (first) { + aggVvec.emplace_back(coefficient); + } else { + aggVvec[j].AggregateInsecure(coefficient); + } + if (!aggVvec[j].IsValid()) { + // AggregateInsecure() asserts on an invalid operand, so leave this to the one-by-one fallback + return false; + } + } } - } - void HandleAggSkShareDone(size_t batchIdx, const CBLSSecretKey& skShare) - { - auto& batchState = batchStates[batchIdx]; - batchState.skShare = skShare; - if (++(*batchState.aggDone) == 2) { - HandleAggDone(batchIdx); + + CBLSPublicKey expected; + if (!expected.PublicKeyShare(aggVvec, forId)) { + return false; } + return expected == CBLSPublicKey::AggregateInsecure(pubKeys); } void HandleVerifyDone(size_t count) @@ -519,39 +547,6 @@ struct ContributionVerifier : public std::enable_shared_from_thisempty() || !batchState.skShare.IsValid()) { - // something went wrong while aggregating and there is nothing we can do now except mark the whole batch as failed - // this can only happen if inputs were invalid in some way - batchState.verifyResults.assign(batchState.count, 0); - HandleVerifyDone(batchState.count); - return; - } - - AsyncAggregatedVerifyBatch(batchIdx); - } - - void AsyncAggregatedVerifyBatch(size_t batchIdx) - { - auto self(this->shared_from_this()); - auto f = [this, self, batchIdx](int threadId) { - auto& batchState = batchStates[batchIdx]; - bool result = Verify(batchState.vvec, batchState.skShare); - if (result) { - // whole batch is valid - batchState.verifyResults.assign(batchState.count, 1); - HandleVerifyDone(batchState.count); - } else { - // at least one entry in the batch is invalid, revert to per-contribution verification (but parallelized) - AsyncVerifyBatchOneByOne(batchIdx); - } - }; - PushOrDoWork(std::move(f)); - } - void AsyncVerifyBatchOneByOne(size_t batchIdx) { size_t count = batchStates[batchIdx].count; @@ -619,13 +614,15 @@ BLSVerificationVectorPtr CBLSWorker::BuildQuorumVerificationVector(Span void AsyncAggregateHelper(ctpl::thread_pool& workerPool, Span vec, bool parallel, - std::function doneCallback) + std::function doneCallback, bool rejectDuplicates = true) { if (vec.empty()) { doneCallback(T()); return; } - if (!VerifyVectorHelper(vec)) { + const bool inputValid{rejectDuplicates ? VerifyVectorHelper(vec) + : std::all_of(vec.begin(), vec.end(), [](const T& item) { return item.IsValid(); })}; + if (!inputValid) { doneCallback(T()); return; } @@ -637,7 +634,10 @@ void AsyncAggregateHelper(ctpl::thread_pool& workerPool, Span vec, bool paral void CBLSWorker::AsyncAggregateSecretKeys(Span secKeys, bool parallel, std::function doneCallback) { - AsyncAggregateHelper(workerPool, secKeys, parallel, std::move(doneCallback)); + // These are the DKG shares dealt to one member. Two colluding dealers can deal it equal shares while their + // contributions stay valid and their vvecs share no coefficient, so rejecting duplicates here would let them keep + // chosen members from building their quorum secret key share. The sum is correct either way. + AsyncAggregateHelper(workerPool, secKeys, parallel, std::move(doneCallback), /*rejectDuplicates=*/false); } std::future CBLSWorker::AsyncAggregateSecretKeys(Span secKeys, bool parallel) @@ -804,6 +804,25 @@ bool CBLSWorker::VerifyVerificationVectors(Span vvecs) return true; } +std::set CBLSWorker::FindVerificationVectorsWithSharedCoefficients(Span vvecs) +{ + std::map owners; + std::set ret; + for (size_t i = 0; i < vvecs.size(); i++) { + if (vvecs[i] == nullptr) { + continue; + } + for (const auto& coefficient : *vvecs[i]) { + const auto [it, inserted] = owners.emplace(coefficient.GetHash(), i); + if (!inserted && it->second != i) { + ret.emplace(it->second); + ret.emplace(i); + } + } + } + return ret; +} + void CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash, const CBLSWorker::SignDoneCallback& doneCallback) { workerPool.push([secKey, msgHash, doneCallback](int threadId) { diff --git a/src/bls/bls_worker.h b/src/bls/bls_worker.h index 369058a9591a..74c7ffe67d9b 100644 --- a/src/bls/bls_worker.h +++ b/src/bls/bls_worker.h @@ -11,6 +11,7 @@ #include #include +#include #include // Low level BLS/DKG stuff. All very compute intensive and optimized for parallelization @@ -104,11 +105,13 @@ class CBLSWorker static constexpr size_t CONTRIBUTION_VERIFY_BATCH_SIZE{8}; // The following functions verify multiple verification vectors and contributions for the same id - // This is parallelized by performing batched verification. The verification vectors and the contributions of - // a batch are aggregated (in parallel, see AsyncBuildQuorumVerificationVector and AsyncBuildSecretKeyShare). The - // result per batch is a single aggregated verification vector and a single aggregated contribution, which are then - // verified with VerifyContributionShare. If verification of the aggregated inputs is successful, the whole batch - // is marked as valid. If the batch verification fails, the individual entries are verified in a non-aggregated manner + // This is parallelized by performing batched verification. Each verification vector of a batch and the public key of + // its contribution are multiplied by the same secret random 64-bit weight, so that errors in the contributions of + // several dealers can't cancel out, and aggregated. The public key share recovered from the aggregated verification + // vector must match the aggregated public key. If it does, the whole batch is marked as valid. If the batch + // verification fails, the individual entries are verified in a non-aggregated manner + // The weighting is only sound for points that passed subgroup checks, so callers that may hold objects deserialized + // under the legacy scheme must pass aggregated=false void AsyncVerifyContributionShares(const CBLSId& forId, Span vvecs, Span skShares, bool parallel, bool aggregated, std::function&)> doneCallback); std::future > AsyncVerifyContributionShares(const CBLSId& forId, Span vvecs, Span skShares, @@ -121,6 +124,9 @@ class CBLSWorker // Simple verification of vectors. Checks x.IsValid() for every entry and checks for duplicate entries static bool VerifyVerificationVector(Span vvec); static bool VerifyVerificationVectors(Span vvecs); + //! Indexes of the vvecs sharing a coefficient with another vvec, which VerifyVerificationVectors() rejects. + //! Null vvecs are skipped. + static std::set FindVerificationVectorsWithSharedCoefficients(Span vvecs); // Internally batched signature signing and verification void AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash, const SignDoneCallback& doneCallback); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index a69fdbc491e6..edb492c5b23b 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -179,10 +179,12 @@ std::optional GovernanceSigner::CreateGovernanceTrigger return std::make_optional(gov_sb); } -void GovernanceSigner::VoteGovernanceTriggers(const std::optional& trigger_opt) +void GovernanceSigner::VoteGovernanceTriggers(const std::optional& trigger_opt, int nHeight) { // only active masternodes can vote on triggers if (m_mn_activeman.GetProTxHash().IsNull()) return; + // Until we are synced we can't build a candidate, so we have nothing to compare triggers to + if (!m_mn_sync.IsSynced()) return; LOCK(::cs_main); @@ -241,6 +243,12 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetBlockHeight() - Params().GetConsensus().nSuperblockMaturityWindow) { + // We can't build a candidate for this trigger's superblock yet, so we have nothing to compare it to + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, its maturity window has not started\n", + __func__, trigger_hash.ToString()); + continue; + } if (trigger_hash == votedFundingYesTriggerHash) { // Skip actual trigger LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", @@ -301,7 +309,7 @@ void GovernanceSigner::UpdatedBlockTip(const CBlockIndex* pindex) { const auto sb_opt = CreateSuperblockCandidate(pindex->nHeight); const auto trigger_opt = CreateGovernanceTrigger(sb_opt); - VoteGovernanceTriggers(trigger_opt); + VoteGovernanceTriggers(trigger_opt, pindex->nHeight); CSuperblock_sptr pSuperblock; if (m_superblocks.GetBestSuperblock(m_dmnman.GetListAtChainTip(), pSuperblock, pindex->nHeight)) { ResetVotedFundingTrigger(); diff --git a/src/governance/signing.h b/src/governance/signing.h index b70f73df81fe..c58ef4357303 100644 --- a/src/governance/signing.h +++ b/src/governance/signing.h @@ -54,7 +54,7 @@ class GovernanceSigner std::optional CreateGovernanceTrigger(const std::optional& sb_opt); std::optional CreateSuperblockCandidate(int nHeight) const; void ResetVotedFundingTrigger(); - void VoteGovernanceTriggers(const std::optional& trigger_opt); + void VoteGovernanceTriggers(const std::optional& trigger_opt, int nHeight); }; #endif // BITCOIN_GOVERNANCE_SIGNING_H diff --git a/src/llmq/dkgmessages.h b/src/llmq/dkgmessages.h index 04a5592d827f..7080779abf2f 100644 --- a/src/llmq/dkgmessages.h +++ b/src/llmq/dkgmessages.h @@ -26,6 +26,8 @@ class CDKGContribution BLSVerificationVectorPtr vvec; std::shared_ptr> contributions; CBLSSignature sig; + //! Not serialized: whether vvec was deserialized under the legacy BLS scheme, which skips the subgroup check + bool vvecLegacyScheme{false}; public: template @@ -52,7 +54,11 @@ class CDKGContribution s >> llmqType; s >> quorumHash; s >> proTxHash; - s >> tmp1; + // Decode the whole vvec under one scheme, so that vvecLegacyScheme describes every point in it + vvecLegacyScheme = bls::bls_legacy_scheme.load(); + for (uint64_t i = 0, size = ReadCompactSize(s); i < size; i++) { + s >> CBLSPublicKeyVersionWrapper(tmp1.emplace_back(), vvecLegacyScheme); + } s >> tmp2; s >> sig; diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 0df95d9fbd49..848afca9f1ee 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -94,6 +94,7 @@ bool CDKGSession::Init(const uint256& _myProTxHash, int _quorumIndex) members.resize(mns.size()); memberIds.resize(members.size()); receivedVvecs.resize(members.size()); + receivedVvecsLegacyScheme.resize(members.size()); receivedSkContributions.resize(members.size()); vecEncryptedContributions.resize(members.size()); @@ -221,6 +222,7 @@ std::optional CDKGSession::ReceiveMessage(const CDKGContribution& qc) if (!should_process) return inv; receivedVvecs[member->idx] = qc.vvec; + receivedVvecsLegacyScheme[member->idx] = qc.vvecLegacyScheme; int receivedCount = std::ranges::count_if(members, [](const auto& m) { return !m->contributions.empty(); }); diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 6b29c3427b0e..e116e55bcd16 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -70,6 +70,7 @@ class DKGError { JUSTIFY_OMIT, JUSTIFY_LIE, CONTRIBUTION_INVALID, + CONTRIBUTION_CLONE, _COUNT }; static constexpr DKGError::type from_string(std::string_view in) { @@ -81,6 +82,7 @@ class DKGError { if (in == "justify-lie") return JUSTIFY_LIE; if (in == "justify-omit") return JUSTIFY_OMIT; if (in == "contribution-invalid") return CONTRIBUTION_INVALID; + if (in == "contribution-clone") return CONTRIBUTION_CLONE; return _COUNT; } }; @@ -144,6 +146,8 @@ class CDKGSession std::vector memberIds; std::vector receivedVvecs; + // whether each received vvec was deserialized under the legacy BLS scheme, see CDKGContribution::vvecLegacyScheme + std::vector receivedVvecsLegacyScheme; // these are not necessarily verified yet. Only trust in what was written to the DB std::vector receivedSkContributions; /// Contains the received unverified/encrypted DKG contributions diff --git a/src/llmq/net_dkg.cpp b/src/llmq/net_dkg.cpp index da1f2481a0b4..279c7dbfd40c 100644 --- a/src/llmq/net_dkg.cpp +++ b/src/llmq/net_dkg.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -223,6 +224,7 @@ std::unordered_set BatchVerifyMessageSigs(CDKGSession& session, std::unordered_set ret; bool revertToSingleVerification = false; + FastRandomContext rng; CBLSSignature aggSig; std::vector pubKeys; @@ -250,11 +252,22 @@ std::unordered_set BatchVerifyMessageSigs(CDKGSession& session, ret.emplace(nodeId); continue; } + CBLSPublicKey pubKey{member->dmn->pdmnState->pubKeyOperator.Get()}; + if (!pubKey.IsValid()) { + // should not happen as operator keys are validated on registration + ret.emplace(nodeId); + continue; + } + + const CBLSSecretKey weight{MakeBatchVerificationWeight(rng)}; + CBLSSignature weightedSig{msg->sig}; + weightedSig.MulInsecure(weight); + pubKey.MulInsecure(weight); if (first) { - aggSig = msg->sig; + aggSig = weightedSig; } else { - aggSig.AggregateInsecure(msg->sig); + aggSig.AggregateInsecure(weightedSig); } first = false; @@ -270,7 +283,7 @@ std::unordered_set BatchVerifyMessageSigs(CDKGSession& session, break; } - pubKeys.emplace_back(member->dmn->pdmnState->pubKeyOperator.Get()); + pubKeys.emplace_back(pubKey); messageHashes.emplace_back(msgHash); } if (!revertToSingleVerification) { diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index 49a6940598a0..83715e84f296 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -293,6 +293,30 @@ static void AddMessage(std::vector& vec, uint32_t sourceId, uint32_t ms vec.emplace_back(m); } +// Adds two wrong signatures whose errors cancel out when both are aggregated unweighted +static void AddCancellingMessages(std::vector& vec, uint32_t sourceId1, uint32_t msgId1, uint8_t msgHash1, + uint32_t sourceId2, uint32_t msgId2, uint8_t msgHash2, bool sameKey) +{ + const bool legacy_scheme = bls::bls_legacy_scheme.load(); + AddMessage(vec, sourceId1, msgId1, msgHash1, true); + AddMessage(vec, sourceId2, msgId2, msgHash2, true); + auto& m1 = vec[vec.size() - 2]; + auto& m2 = vec[vec.size() - 1]; + if (sameKey) { + m2.sk = m1.sk; + m2.pk = m1.pk; + m2.sig = m2.sk.Sign(m2.msgHash, legacy_scheme); + } + + CBLSSecretKey sk_error; + sk_error.MakeNewKey(); + const CBLSSignature error{sk_error.Sign(uint256::ONE, legacy_scheme)}; + m1.sig.AggregateInsecure(error); + m2.sig.SubInsecure(error); + m1.valid = false; + m2.valid = false; +} + static void Verify(std::vector& vec, bool secureVerification, bool perMessageFallback) { CBLSBatchVerifier batchVerifier(secureVerification, perMessageFallback); @@ -370,6 +394,18 @@ void FuncBatchVerifier(const bool legacy_scheme) // last message invalid from one source AddMessage(msgs, 1, 7, 1, false); Verify(msgs); + + msgs.clear(); + // errors cancelling out between distinct messages from one source + AddMessage(msgs, 1, 1, 1, true); + AddCancellingMessages(msgs, 2, 2, 2, 2, 3, 3, /*sameKey=*/false); + Verify(msgs); + + // errors cancelling out between distinct sources signing the same message with the same key + msgs.clear(); + AddMessage(msgs, 1, 1, 1, true); + AddCancellingMessages(msgs, 2, 2, 2, 3, 3, 2, /*sameKey=*/true); + Verify(msgs); } void FuncThresholdSignature(const bool legacy_scheme) @@ -637,6 +673,30 @@ void FuncVerifyContributionSharesHonest(const bool legacy_scheme) } } + // 3. With the defaults the DKG uses, only the dealers of wrong shares are blamed: + // - two colluding dealers in the same batch swap their shares: each one is wrong, but their sum is not, so the + // errors cancel in an unweighted aggregate; + // - a missing (invalid) share fails its batch and is blamed alone. + auto swapped = skContributions; + std::swap(swapped[1], swapped[2]); + auto missing = skContributions; + missing[5] = CBLSSecretKey{}; + std::vector, std::set>> cases{ + {swapped, {1, 2}}, + {missing, {5}}, + }; + for (auto& [shares, bad] : cases) { + for (const bool parallel : {true, false}) { + const auto result = worker.VerifyContributionShares(myId, vvecs, shares, parallel); + BOOST_REQUIRE_EQUAL(result.size(), QUORUM_SIZE); + for (const size_t i : util::irange(QUORUM_SIZE)) { + const bool expected{bad.count(i) == 0}; + BOOST_CHECK_MESSAGE(result[i] == expected, "index " << i << " expected " << expected << " got " + << result[i] << " (parallel=" << parallel << ")"); + } + } + } + worker.Stop(); } @@ -694,6 +754,23 @@ void FuncVerifyContributionSharesDuplicateCoefficient(const bool legacy_scheme) BOOST_CHECK(!res[0]); // the member that planted the duplicate is caught alone BOOST_CHECK(res[1]); // the honest member whose coefficient was copied is unaffected BOOST_CHECK(res[2]); // the other honest member is unaffected + BOOST_CHECK(CBLSWorker::FindVerificationVectorsWithSharedCoefficients(vvecs) == (std::set{0, 1})); + + // Dealer 0 copies dealer 1's whole contribution, vvec and share alike: every share verifies, + // but the quorum verification vector can't be built until both copies are dropped. + vvecs[0] = std::make_shared>(*vvecs[1]); + recvShares[0] = skShares[1][R]; + res = worker.VerifyContributionShares(recvId, vvecs, recvShares); + BOOST_REQUIRE_EQUAL(res.size(), 3U); + BOOST_CHECK(res[0] && res[1] && res[2]); + BOOST_CHECK(worker.BuildQuorumVerificationVector(vvecs) == nullptr); + BOOST_CHECK(CBLSWorker::FindVerificationVectorsWithSharedCoefficients(vvecs) == (std::set{0, 1})); + std::vector remaining{vvecs[2]}; + BOOST_CHECK(worker.BuildQuorumVerificationVector(remaining) != nullptr); + + // Distinct vvecs share nothing, and a missing vvec is skipped. + std::vector distinct{vvecs[1], nullptr, vvecs[2]}; + BOOST_CHECK(CBLSWorker::FindVerificationVectorsWithSharedCoefficients(distinct).empty()); worker.Stop(); } @@ -704,6 +781,61 @@ BOOST_AUTO_TEST_CASE(bls_verify_contribution_shares_duplicate_coefficient_tests) FuncVerifyContributionSharesDuplicateCoefficient(false); } +void FuncAggregateCollidingContributionShares(const bool legacy_scheme) +{ + bls::bls_legacy_scheme.store(legacy_scheme); + + CBLSWorker worker; + worker.Start(4); + + // The receiver's id is -1 mod the group order, so a degree-1 polynomial b0 + b1 * x takes the value b0 - b1 there + uint256 recvIdHash; + const auto minusOne{ParseHex("73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000000")}; + std::copy(minusOne.begin(), minusOne.end(), recvIdHash.begin()); + const CBLSId recvId{recvIdHash}; + + // Dealer 0 is honest. Dealers 1 and 2 collude: their polynomials differ, but agree at the receiver's id + const auto makeKey = [] { CBLSSecretKey sk; sk.MakeNewKey(); return sk; }; + std::vector> polys{{makeKey(), makeKey()}, {makeKey(), makeKey()}, {}}; + CBLSSecretKey share1; + BOOST_REQUIRE(share1.SecretKeyShare(polys[1], recvId)); + const CBLSSecretKey b1{makeKey()}; + CBLSSecretKey b0{share1}; + b0.AggregateInsecure(b1); + polys[2] = {b0, b1}; + + std::vector vvecs; + std::vector recvShares; + for (auto& poly : polys) { + auto vvec = std::make_shared>(); + for (const auto& coefficient : poly) { + vvec->emplace_back(coefficient.GetPublicKey()); + } + vvecs.emplace_back(vvec); + BOOST_REQUIRE(recvShares.emplace_back().SecretKeyShare(poly, recvId)); + } + BOOST_REQUIRE(recvShares[1] == recvShares[2]); + + // Every contribution verifies and nothing in the vvecs collides + const std::vector ok{worker.VerifyContributionShares(recvId, vvecs, recvShares)}; + BOOST_CHECK(ok == std::vector(3, true)); + const BLSVerificationVectorPtr quorumVvec{worker.BuildQuorumVerificationVector(vvecs)}; + BOOST_REQUIRE(quorumVvec != nullptr); + + // so the receiver must still get the quorum secret key share matching the quorum vvec + const CBLSSecretKey skShare{worker.AggregateSecretKeys(recvShares)}; + BOOST_REQUIRE(skShare.IsValid()); + BOOST_CHECK(skShare.GetPublicKey() == CBLSWorker::BuildPubKeyShare(quorumVvec, recvId)); + + worker.Stop(); +} + +BOOST_AUTO_TEST_CASE(bls_aggregate_colliding_contribution_shares_tests) +{ + FuncAggregateCollidingContributionShares(true); + FuncAggregateCollidingContributionShares(false); +} + // A dummy BLS object that satisfies the minimal interface expected by CBLSLazyWrapper. class DummyBLS { diff --git a/src/test/llmq_dkg_tests.cpp b/src/test/llmq_dkg_tests.cpp index 127d9eaffaf2..6595cf457a02 100644 --- a/src/test/llmq_dkg_tests.cpp +++ b/src/test/llmq_dkg_tests.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include #include #include @@ -120,4 +122,37 @@ BOOST_AUTO_TEST_CASE(pending_messages_quota_is_per_protx) BOOST_CHECK_EQUAL(pending.PopPendingMessages(6).size(), 5U); } +BOOST_AUTO_TEST_CASE(contribution_records_vvec_scheme) +{ + // A contribution remembers which BLS scheme its vvec was deserialized under, since only the basic scheme checks + // subgroup membership, which weighted contribution verification relies on + using namespace llmq; + + const bool legacy_scheme_before{bls::bls_legacy_scheme.load()}; + + uint256 seed{uint256S("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")}; + *seed.begin() = 0; // keep it below the group order + const CBLSSecretKey sk{Span{seed}}; + BOOST_REQUIRE(sk.IsValid()); + CDKGContribution qc; + qc.llmqType = Consensus::LLMQType::LLMQ_TEST; + qc.vvec = std::make_shared>(std::vector{sk.GetPublicKey()}); + qc.contributions = std::make_shared>(); + + for (const bool legacy : {true, false}) { + bls::bls_legacy_scheme.store(legacy); + CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; + stream << qc; + CDKGContribution received; + stream >> received; + BOOST_CHECK_EQUAL(received.vvecLegacyScheme, legacy); + BOOST_REQUIRE_EQUAL(received.vvec->size(), 1U); + BOOST_CHECK((*received.vvec)[0].IsValid()); + BOOST_CHECK((*received.vvec)[0] == (*qc.vvec)[0]); + BOOST_CHECK(stream.empty()); + } + + bls::bls_legacy_scheme.store(legacy_scheme_before); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/llmq_utils_tests.cpp b/src/test/llmq_utils_tests.cpp index 4a3a6aaf0a4f..0978a2e3ba3e 100644 --- a/src/test/llmq_utils_tests.cpp +++ b/src/test/llmq_utils_tests.cpp @@ -423,9 +423,16 @@ BOOST_AUTO_TEST_CASE(sig_share_batch_verifier_judges_each_sender_by_its_own_shar sk_forger.MakeNewKey(); const CBLSSignature forged_sig{sk_forger.Sign(sig_share.GetSignHash(), legacy_scheme)}; + // Two wrong shares whose errors cancel out: (valid + forged) + (valid - forged) == 2 * valid + CBLSSignature plus_error_sig{valid_sig}; + plus_error_sig.AggregateInsecure(forged_sig); + CBLSSignature minus_error_sig{valid_sig}; + minus_error_sig.SubInsecure(forged_sig); + constexpr NodeId honest{1}; constexpr NodeId attacker{2}; constexpr NodeId other_honest{3}; + constexpr NodeId other_attacker{4}; struct Push { NodeId from; @@ -438,6 +445,8 @@ BOOST_AUTO_TEST_CASE(sig_share_batch_verifier_judges_each_sender_by_its_own_shar {{{attacker, forged_sig}, {honest, valid_sig}}, {attacker}}, // The same valid share relayed by two peers is not misbehavior. {{{honest, valid_sig}, {other_honest, valid_sig}}, {}}, + // Complementary errors from two peers must not pass as one valid aggregate. + {{{attacker, plus_error_sig}, {other_attacker, minus_error_sig}}, {attacker, other_attacker}}, }; for (const auto& [pushes, expected_bad_sources] : cases) { diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 4f9d89f7f598..191700baf99c 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -489,6 +489,56 @@ def yes_votes_known(node, trigger_hash, yes_count): self.check_superblockbudget(True) self.check_superblock() + self.log.info("A matching trigger relayed before the maturity window must still be funded") + sb_block_height = 240 + window_start = sb_block_height - sb_maturity_window + # Blocks above came faster than 2.5 minutes apart, so get clear of the per-masternode trigger rate limit + self.bump_mocktime(3 * sb_cycle * 150, update_schedulers=False) + self.generate(self.nodes[0], window_start - 3 - self.nodes[0].getblockcount(), sync_fun=self.sync_blocks()) + + # The payee at window_start creates the trigger, so let it reach the window alone, then rewind it + payee_address = self.nodes[0].masternode("winners")[str(window_start + 1)] + early_idx = next(mn.nodeIdx for mn in self.mninfo + if self.nodes[0].protx("info", mn.proTxHash)["state"]["payoutAddress"] in payee_address) + early_node = self.nodes[early_idx] + self.isolate_node(early_idx) + fork_point = early_node.getblockcount() + 1 + for _ in range(3): + self.bump_mocktime(1) + self.generate(early_node, 1, sync_fun=self.no_op) + self.wait_until(lambda: have_trigger_for_height([early_node], sb_block_height), timeout=5) + early_trigger_hash = next(h for h, t in early_node.gobject("list", "valid", "triggers").items() + if json.loads(t["DataString"])["event_block_height"] == sb_block_height) + early_node.invalidateblock(early_node.getblockhash(fork_point)) + self.reconnect_isolated_node(early_idx, 0) + self.sync_blocks() + + def early_trigger_relayed(): + # Each bump runs the once-a-second governance relay flush + self.bump_mocktime(1) + return all(early_trigger_hash in node.gobject("list", "valid", "triggers") for node in self.nodes) + self.wait_until(early_trigger_relayed, timeout=10) + + self.log.info("Masternodes must not vote NO on it before the window opens") + for _ in range(2): + self.bump_mocktime(1) + self.generate(self.nodes[0], 1, sync_fun=self.sync_blocks()) + assert_equal(self.nodes[0].getblockcount(), window_start - 1) + for node in self.nodes: + # A masternode's own vote is visible on its own node without waiting for relay + node.syncwithvalidationinterfacequeue() + assert_equal(node.gobject("list", "valid", "triggers")[early_trigger_hash]["NoCount"], 0) + + self.log.info("Every masternode votes YES on it once the window opens") + self.bump_mocktime(1) + self.generate(self.nodes[0], 1, sync_fun=self.sync_blocks()) + self.wait_until(lambda: self.nodes[0].gobject("list", "valid", "triggers")[early_trigger_hash]["YesCount"] == self.mn_count, timeout=5) + n = sb_block_height - self.nodes[0].getblockcount() + self.bump_mocktime(n) + self.generate(self.nodes[0], n, sync_fun=self.sync_blocks()) + assert_equal(self.nodes[0].getblockcount(), sb_block_height) + self.check_superblock() + self.test_pruned_node(p3_collateral_prepare) def test_pruned_node(self, proposal): diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index b33f7c7784de..ccb98207493b 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -12,11 +12,13 @@ ''' +from test_framework.masternodes import check_banned, check_punished from test_framework.simplepose import SimplePoSeMixin from test_framework.test_framework import ( DashTestFramework, MasternodeInfo, ) +from test_framework.util import assert_equal class LLMQSimplePoSeTest(SimplePoSeMixin, DashTestFramework): @@ -26,6 +28,8 @@ def set_test_params(self): def run_test(self): self.set_pose_sporks() + self.test_cloned_contributions() + if not self.options.disable_spork23: # Lets isolate MNs one by one and verify that punishment/banning happens self.test_banning(self.isolate_mn, 2) @@ -57,6 +61,31 @@ def run_test(self): self.repair_masternodes(True) + def test_cloned_contributions(self): + self.log.info("Two members dealing identical contributions are both excluded and the quorum still forms") + cloners = self.mninfo[:2] + others = self.mninfo[2:] + for mn in cloners: + mn.get_node(self).quorum('dkgsimerror', 'contribution-clone', 100) + qh = self.mine_quorum(mninfos_valid=others) + for mn in cloners: + mn.get_node(self).quorum('dkgsimerror', 'contribution-clone', 0) + + valid_members = {m['proTxHash'] for m in self.nodes[0].quorum('info', 100, qh)['members'] if m['valid']} + assert_equal(valid_members, {mn.proTxHash for mn in others}) + for mn in cloners: + assert check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + + self.log.info("Let the PoSe penalty of the cloners decay without running any DKG") + self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 4070908800) + self.wait_for_sporks_same() + self.generate(self.nodes[0], 66) + for mn in self.mninfo: + assert not check_punished(self.nodes[0], mn) + self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) + self.wait_for_sporks_same() + def isolate_mn(self, mn: MasternodeInfo): mn.get_node(self).setnetworkactive(False) self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0)