From e18f62193e5d6b1d7e306801578362b6c25fcd53 Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 23 Feb 2025 15:43:03 -0600 Subject: [PATCH] fix: improper default check; add tests; use in more dml places --- src/bls/bls.h | 4 +- src/evo/deterministicmns.cpp | 4 +- src/test/bls_tests.cpp | 138 ++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index 6bc80aa98cba6..0b0d81f74934c 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -509,8 +509,8 @@ class CBLSLazyWrapper bool operator==(const CBLSLazyWrapper& r) const { // If neither bufValid or objInitialized are set, then the object is the default object. - const bool is_default{bufValid || objInitialized}; - const bool r_is_default{r.bufValid || r.objInitialized}; + const bool is_default{!bufValid && !objInitialized}; + const bool r_is_default{!r.bufValid && !r.objInitialized}; // If both are default; they are equal. if (is_default && r_is_default) return true; // If one is default and the other isn't, we are not equal diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index e8b870cc1ddfc..cac468036b41c 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -578,7 +578,7 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash) throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a keyIDOwner=%s", __func__, proTxHash.ToString(), EncodeDestination(PKHash(dmn->pdmnState->keyIDOwner))))); } - if (dmn->pdmnState->pubKeyOperator.Get().IsValid() && !DeleteUniqueProperty(*dmn, dmn->pdmnState->pubKeyOperator)) { + if (dmn->pdmnState->pubKeyOperator != CBLSLazyPublicKey() && !DeleteUniqueProperty(*dmn, dmn->pdmnState->pubKeyOperator)) { mnUniquePropertyMap = mnUniquePropertyMapSaved; throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a pubKeyOperator=%s", __func__, proTxHash.ToString(), dmn->pdmnState->pubKeyOperator.ToString()))); @@ -840,7 +840,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no } if (newState->IsBanned()) { // only revive when all keys are set - if (newState->pubKeyOperator.Get().IsValid() && !newState->keyIDVoting.IsNull() && !newState->keyIDOwner.IsNull()) { + if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() && !newState->keyIDOwner.IsNull()) { newState->Revive(nHeight); if (debugLogs) { LogPrintf("CDeterministicMNManager::%s -- MN %s revived at height %d\n", diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index aef7f47f9f6e5..ead1c115fd5e4 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -472,4 +472,140 @@ BOOST_AUTO_TEST_CASE(bls_threshold_signature_tests) FuncThresholdSignature(false); } -BOOST_AUTO_TEST_SUITE_END() +// A dummy BLS object that satisfies the minimal interface expected by CBLSLazyWrapper. +class DummyBLS { +public: + // Define a fixed serialization size (for testing purposes). + static const size_t SerSize = 4; + std::array data{}; + + DummyBLS() { + data.fill(0); + } + + // A dummy validity check: valid if any byte is non-zero. + bool IsValid() const { + return std::any_of(data.begin(), data.end(), [](uint8_t c){ return c != 0; }); + } + + // Convert to bytes; ignore the legacy flag for simplicity. + std::array ToBytes(bool /*legacy*/) const { + return data; + } + + // Set from bytes; again, ignore the legacy flag. + void SetBytes(const std::array& bytes, bool /*legacy*/) { + data = bytes; + } + + // A dummy malleability check: simply compares the stored data to the given bytes. + bool CheckMalleable(const std::array& bytes, bool /*legacy*/) const { + return data == bytes; + } + + // Reset the object to an "empty" state. + void Reset() { + data.fill(0); + } + + // Produce a string representation. + std::string ToString(bool /*legacy*/) const { + std::ostringstream oss; + for (auto b : data) { + oss << std::setfill('0') << std::setw(2) << std::hex << static_cast(b); + } + return oss.str(); + } + + // Equality operator. + bool operator==(const DummyBLS& other) const { + return data == other.data; + } +}; + +// Define a type alias for our lazy wrapper instantiated with DummyBLS. +using LazyDummyBLS = CBLSLazyWrapper; + +// Test 1: Two default (unset) wrappers should compare equal. +BOOST_AUTO_TEST_CASE(test_default_equality) +{ + LazyDummyBLS lazy1; + LazyDummyBLS lazy2; + // Neither instance has been set, so they represent the default/null object. + BOOST_CHECK(lazy1 == lazy2); +} + +// Test 2: A default wrapper and one initialized with a nonzero DummyBLS should compare unequal. +BOOST_AUTO_TEST_CASE(test_non_default_vs_default) +{ + LazyDummyBLS lazy_default; + LazyDummyBLS lazy_set; + DummyBLS obj; + obj.data = {1, 2, 3, 4}; // nonzero data makes the object valid + lazy_set.Set(obj, false); + BOOST_CHECK(!(lazy_default == lazy_set)); + BOOST_CHECK(lazy_default != lazy_set); +} + +// Test 2: A default wrapper and one initialized with a nonzero DummyBLS should compare unequal. +BOOST_AUTO_TEST_CASE(test_non_default_vs_different) +{ + LazyDummyBLS lazy_a; + LazyDummyBLS lazy_b; + DummyBLS obj; + obj.data = {1, 2, 3, 4}; // nonzero data makes the object valid + lazy_a.Set(obj, false); + obj.data = {4, 3, 2, 1}; // nonzero data makes the object valid + lazy_b.Set(obj, false); + BOOST_CHECK(lazy_a != lazy_b); +} + +// Test 3: Two wrappers set with the same underlying DummyBLS value compare equal. +BOOST_AUTO_TEST_CASE(test_equality_same_value) +{ + LazyDummyBLS lazy1; + LazyDummyBLS lazy2; + BOOST_CHECK(lazy1 == lazy2); + DummyBLS obj; + obj.data = {5, 6, 7, 8}; + lazy1.Set(obj, false); + BOOST_CHECK(lazy1 != lazy2); + lazy2.Set(obj, false); + BOOST_CHECK(lazy1 == lazy2); +} + +// Test 4: Serialization and unserialization preserve the wrapped value. +BOOST_AUTO_TEST_CASE(test_serialization_unserialization) +{ + LazyDummyBLS lazy1; + DummyBLS obj; + obj.data = {9, 10, 11, 12}; + // Set with a specific legacy flag (true in this case) + lazy1.Set(obj, true); + + // Serialize the lazy object into a data stream. + CDataStream ds(SER_DISK, CLIENT_VERSION); + lazy1.Serialize(ds, true); + + // Create a new instance and unserialize the data into it. + LazyDummyBLS lazy2; + lazy2.Unserialize(ds, true); + BOOST_CHECK(lazy1 == lazy2); + BOOST_CHECK(lazy2.Get() == obj); +} + +// Test 5: Two wrappers wrapping the same object should have the same hash. +BOOST_AUTO_TEST_CASE(test_get_hash_consistency) +{ + LazyDummyBLS lazy1; + LazyDummyBLS lazy2; + DummyBLS obj; + obj.data = {13, 14, 15, 16}; + lazy1.Set(obj, false); + lazy2.Set(obj, false); + uint256 hash1 = lazy1.GetHash(); + uint256 hash2 = lazy2.GetHash(); + BOOST_CHECK(hash1 == hash2); +} + +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file