Continued refactoring in bls to resolve clang tidy warnings (#4189)

* refac/bls: use default for default constructor

* refac/bls: initialize doneCount via default member initializer

* refac/bls: initialize batchCount and verifyCount via default member initializer

* refac/bls: remove unused parameter

* refac/bls: add __attribute__((unused)) where functions are unused

* refac/bls: remove unused lambda this capture

* refac/bls: Move assignment operators should be marked noexcept

* refac/bls: Single-argument constructors must be marked explicit to avoid unintentional implicit conversions

* refac/bls: fix compilation due to explicit constructor

src/bench/bls_dkg.cpp

@@ -33,7 +33,7 @@ struct DKG
         for (int i = 0; i < quorumSize; i++) {
             uint256 id;
             WriteLE64(id.begin(), i + 1);
-            members.push_back({id, {}, {}});
+            members.push_back({CBLSId(id), {}, {}});
             ids.emplace_back(id);
         }
 

src/bls/bls.h

@@ -54,24 +54,24 @@ protected:
 public:
     static const size_t SerSize = _SerSize;
 
-    CBLSWrapper(const bool fLegacyIn = fLegacyDefault) : fLegacy(fLegacyIn)
+    explicit CBLSWrapper(const bool fLegacyIn = fLegacyDefault) : fLegacy(fLegacyIn)
     {
     }
-    CBLSWrapper(const std::vector<unsigned char>& vecBytes, const bool fLegacyIn = fLegacyDefault) : CBLSWrapper<ImplType, _SerSize, C>(fLegacyIn)
+    explicit CBLSWrapper(const std::vector<unsigned char>& vecBytes, const bool fLegacyIn = fLegacyDefault) : CBLSWrapper<ImplType, _SerSize, C>(fLegacyIn)
     {
         SetByteVector(vecBytes);
     }
 
     CBLSWrapper(const CBLSWrapper& ref) = default;
     CBLSWrapper& operator=(const CBLSWrapper& ref) = default;
-    CBLSWrapper(CBLSWrapper&& ref)
+    CBLSWrapper(CBLSWrapper&& ref) noexcept
     {
         std::swap(impl, ref.impl);
         std::swap(fValid, ref.fValid);
         std::swap(cachedHash, ref.cachedHash);
         std::swap(fLegacy, ref.fLegacy);
     }
-    CBLSWrapper& operator=(CBLSWrapper&& ref)
+    CBLSWrapper& operator=(CBLSWrapper&& ref) noexcept
     {
         std::swap(impl, ref.impl);
         std::swap(fValid, ref.fValid);
@@ -193,7 +193,7 @@ public:
 
 struct CBLSIdImplicit : public uint256
 {
-    CBLSIdImplicit() {}
+    CBLSIdImplicit() = default;
     CBLSIdImplicit(const uint256& id)
     {
         memcpy(begin(), id.begin(), sizeof(uint256));
@@ -218,8 +218,8 @@ public:
     using CBLSWrapper::operator!=;
     using CBLSWrapper::CBLSWrapper;
 
-    CBLSId() {}
+    CBLSId() = default;
-    CBLSId(const uint256& nHash);
+    explicit CBLSId(const uint256& nHash);
 };
 
 class CBLSSecretKey : public CBLSWrapper<bls::PrivateKey, BLS_CURVE_SECKEY_SIZE, CBLSSecretKey>
@@ -230,7 +230,7 @@ public:
     using CBLSWrapper::operator!=;
     using CBLSWrapper::CBLSWrapper;
 
-    CBLSSecretKey() {}
+    CBLSSecretKey() = default;
     CBLSSecretKey(const CBLSSecretKey&) = default;
     CBLSSecretKey& operator=(const CBLSSecretKey&) = default;
 
@@ -257,7 +257,7 @@ public:
     using CBLSWrapper::operator!=;
     using CBLSWrapper::CBLSWrapper;
 
-    CBLSPublicKey() {}
+    CBLSPublicKey() = default;
 
     void AggregateInsecure(const CBLSPublicKey& o);
     static CBLSPublicKey AggregateInsecure(const std::vector<CBLSPublicKey>& pks, bool fLegacy = fLegacyDefault);
@@ -276,7 +276,7 @@ public:
     using CBLSWrapper::operator!=;
     using CBLSWrapper::CBLSWrapper;
 
-    CBLSSignature() {}
+    CBLSSignature() = default;
     CBLSSignature(const CBLSSignature&) = default;
     CBLSSignature& operator=(const CBLSSignature&) = default;
 

src/bls/bls_ies.h

@@ -33,9 +33,7 @@ template <typename Object>
 class CBLSIESEncryptedObject : public CBLSIESEncryptedBlob
 {
 public:
-    CBLSIESEncryptedObject()
+    CBLSIESEncryptedObject() = default;
-    {
-    }
 
     CBLSIESEncryptedObject(const CBLSPublicKey& ephemeralPubKeyIn, const uint256& ivSeedIn, const std::vector<unsigned char>& dataIn)
     {

src/bls/bls_worker.cpp

@@ -52,9 +52,7 @@ std::pair<std::function<void(T)>, std::future<T> > BuildFutureDoneCallback2()
 
 /////
 
-CBLSWorker::CBLSWorker()
+CBLSWorker::CBLSWorker() = default;
-{
-}
 
 CBLSWorker::~CBLSWorker()
 {
@@ -361,12 +359,12 @@ struct VectorAggregator {
             start(_start),
             count(_count),
             workerPool(_workerPool),
-            doneCallback(std::move(_doneCallback))
+            doneCallback(std::move(_doneCallback)),
+            doneCount(0)
     {
         assert(!vecs.empty());
         vecSize = vecs[0]->size();
         result = std::make_shared<VectorType>(vecSize);
-        doneCount = 0;
     }
 
     void Start()
@@ -445,7 +443,9 @@ struct ContributionVerifier {
         parallel(_parallel),
         aggregated(_aggregated),
         workerPool(_workerPool),
-        doneCallback(std::move(_doneCallback))
+        doneCallback(std::move(_doneCallback)),
+        batchCount(1),
+        verifyCount(_vvecs.size())
     {
     }
 
@@ -454,11 +454,9 @@ struct ContributionVerifier {
         if (!aggregated) {
             // treat all inputs as one large batch
             batchSize = vvecs.size();
-            batchCount = 1;
         } else {
             batchCount = (vvecs.size() + batchSize - 1) / batchSize;
         }
-        verifyCount = vvecs.size();
 
         batchStates.resize(batchCount);
         for (size_t i = 0; i < batchCount; i++) {
@@ -524,7 +522,7 @@ struct ContributionVerifier {
         }
     }
 
-    void HandleVerifyDone(size_t batchIdx, size_t count)
+    void HandleVerifyDone(size_t count)
     {
         size_t c = verifyDoneCount += count;
         if (c == verifyCount) {
@@ -540,7 +538,7 @@ struct ContributionVerifier {
             // 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(batchIdx, batchState.count);
+            HandleVerifyDone(batchState.count);
             return;
         }
 
@@ -555,7 +553,7 @@ struct ContributionVerifier {
             if (result) {
                 // whole batch is valid
                 batchState.verifyResults.assign(batchState.count, 1);
-                HandleVerifyDone(batchIdx, batchState.count);
+                HandleVerifyDone(batchState.count);
             } else {
                 // at least one entry in the batch is invalid, revert to per-contribution verification (but parallelized)
                 AsyncVerifyBatchOneByOne(batchIdx);
@@ -572,7 +570,7 @@ struct ContributionVerifier {
             auto f = [this, i, batchIdx](int threadId) {
                 auto& batchState = batchStates[batchIdx];
                 batchState.verifyResults[i] = Verify(vvecs[batchState.start + i], skShares[batchState.start + i]);
-                HandleVerifyDone(batchIdx, 1);
+                HandleVerifyDone(1);
             };
             PushOrDoWork(std::move(f));
         }
@@ -691,7 +689,7 @@ std::future<CBLSPublicKey> CBLSWorker::AsyncAggregatePublicKeys(const BLSPublicK
     return std::move(p.second);
 }
 
-CBLSPublicKey CBLSWorker::AggregatePublicKeys(const BLSPublicKeyVector& pubKeys,
+__attribute__((unused)) CBLSPublicKey CBLSWorker::AggregatePublicKeys(const BLSPublicKeyVector& pubKeys,
                                               size_t start, size_t count, bool parallel)
 {
     return AsyncAggregatePublicKeys(pubKeys, start, count, parallel).get();
@@ -712,7 +710,7 @@ std::future<CBLSSignature> CBLSWorker::AsyncAggregateSigs(const BLSSignatureVect
     return std::move(p.second);
 }
 
-CBLSSignature CBLSWorker::AggregateSigs(const BLSSignatureVector& sigs,
+__attribute__((unused)) CBLSSignature CBLSWorker::AggregateSigs(const BLSSignatureVector& sigs,
                                         size_t start, size_t count, bool parallel)
 {
     return AsyncAggregateSigs(sigs, start, count, parallel).get();
@@ -764,7 +762,7 @@ std::future<bool> CBLSWorker::AsyncVerifyContributionShare(const CBLSId& forId,
         return std::move(p.second);
     }
 
-    auto f = [this, &forId, &vvec, &skContribution](int threadId) {
+    auto f = [&forId, &vvec, &skContribution](int threadId) {
         CBLSPublicKey pk1;
         if (!pk1.PublicKeyShare(*vvec, forId)) {
             return false;
@@ -776,7 +774,7 @@ std::future<bool> CBLSWorker::AsyncVerifyContributionShare(const CBLSId& forId,
     return workerPool.push(f);
 }
 
-bool CBLSWorker::VerifyContributionShare(const CBLSId& forId, const BLSVerificationVectorPtr& vvec,
+__attribute__((unused)) bool CBLSWorker::VerifyContributionShare(const CBLSId& forId, const BLSVerificationVectorPtr& vvec,
                                          const CBLSSecretKey& skContribution)
 {
     CBLSPublicKey pk1;
@@ -823,12 +821,12 @@ bool CBLSWorker::VerifyVerificationVectors(const std::vector<BLSVerificationVect
     return true;
 }
 
-bool CBLSWorker::VerifySecretKeyVector(const BLSSecretKeyVector& secKeys, size_t start, size_t count)
+__attribute__((unused)) bool CBLSWorker::VerifySecretKeyVector(const BLSSecretKeyVector& secKeys, size_t start, size_t count)
 {
     return VerifyVectorHelper(secKeys, start, count);
 }
 
-bool CBLSWorker::VerifySignatureVector(const BLSSignatureVector& sigs, size_t start, size_t count)
+__attribute__((unused)) bool CBLSWorker::VerifySignatureVector(const BLSSignatureVector& sigs, size_t start, size_t count)
 {
     return VerifyVectorHelper(sigs, start, count);
 }
@@ -840,7 +838,7 @@ void CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash,
     });
 }
 
-std::future<CBLSSignature> CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash)
+__attribute__((unused)) std::future<CBLSSignature> CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash)
 {
     auto p = BuildFutureDoneCallback<CBLSSignature>();
     AsyncSign(secKey, msgHash, p.first);

src/bls/bls_worker.h

@@ -94,7 +94,7 @@ public:
                                   std::function<void(const CBLSPublicKey&)> doneCallback);
     std::future<CBLSPublicKey> AsyncAggregatePublicKeys(const BLSPublicKeyVector& pubKeys,
                                                         size_t start, size_t count, bool parallel);
-    CBLSPublicKey AggregatePublicKeys(const BLSPublicKeyVector& pubKeys, size_t start = 0, size_t count = 0, bool parallel = true);
+    __attribute__((unused)) CBLSPublicKey AggregatePublicKeys(const BLSPublicKeyVector& pubKeys, size_t start = 0, size_t count = 0, bool parallel = true);
 
     void AsyncAggregateSigs(const BLSSignatureVector& sigs,
                             size_t start, size_t count, bool parallel,