Merge #5953: fix: resolve a few very unlikely lifetime / undefined behavior issues

6f2b350baa fix: don't move out of pendingContributionVerifications; use a ref and then clear (pasta)
7f36f122b2 fix: fix potential mutex lifetime issue were we are returning a reference, and then releasing the mutex (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See individual commits

  ## What was done?
  Fix potential lifetime issue and potential undefined behavior

  ## How Has This Been Tested?
  Compiling

  ## Breaking Changes
  None

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: f76d88c1fe3c19a92bdf451f147520e29f5edf84342dfb0b6ea9bde901a3f826c09b5aa2334d8f6fa687aaae7d0c109f36779883c670915d55b69af3ea8affd4
This commit is contained in:
pasta 2024-04-03 14:10:50 -05:00
commit 27c0813c08
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984

View File

@ -361,8 +361,7 @@ void CDKGSession::VerifyPendingContributions()
cxxtimer::Timer t1(true); cxxtimer::Timer t1(true);
std::vector<size_t> pend = std::move(pendingContributionVerifications); if (pendingContributionVerifications.empty()) {
if (pend.empty()) {
return; return;
} }
@ -370,7 +369,7 @@ void CDKGSession::VerifyPendingContributions()
std::vector<BLSVerificationVectorPtr> vvecs; std::vector<BLSVerificationVectorPtr> vvecs;
std::vector<CBLSSecretKey> skContributions; std::vector<CBLSSecretKey> skContributions;
for (const auto& idx : pend) { for (const auto& idx : pendingContributionVerifications) {
const auto& m = members[idx]; const auto& m = members[idx];
if (m->bad || m->weComplain) { if (m->bad || m->weComplain) {
continue; continue;
@ -404,7 +403,8 @@ void CDKGSession::VerifyPendingContributions()
} }
} }
logger.Batch("verified %d pending contributions. time=%d", pend.size(), t1.count()); logger.Batch("verified %d pending contributions. time=%d", pendingContributionVerifications.size(), t1.count());
pendingContributionVerifications.clear();
} }
void CDKGSession::VerifyAndComplain(CDKGPendingMessages& pendingMessages) void CDKGSession::VerifyAndComplain(CDKGPendingMessages& pendingMessages)
@ -663,8 +663,9 @@ void CDKGSession::VerifyAndJustify(CDKGPendingMessages& pendingMessages)
continue; continue;
} }
const auto& qc = WITH_LOCK(invCs, return std::move(complaints.at(*m->complaints.begin()))); LOCK(invCs);
if (qc.complainForMembers[*myIdx]) { if (const auto& qc = complaints.at(*m->complaints.begin());
qc.complainForMembers[*myIdx]) {
justifyFor.emplace(qc.proTxHash); justifyFor.emplace(qc.proTxHash);
} }
} }