From ad64fc77f0634bd1e2a5979a847dd8af2a2384e8 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 2 Apr 2024 13:05:15 -0500 Subject: [PATCH] GH-2102 Use vector of atomic bools to avoid mutex lock for duplicate check --- libraries/chain/hotstuff/hotstuff.cpp | 57 +++++++++---------- .../include/eosio/chain/hotstuff/hotstuff.hpp | 19 +++++-- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/libraries/chain/hotstuff/hotstuff.cpp b/libraries/chain/hotstuff/hotstuff.cpp index a0839affb8..1aaffabfad 100644 --- a/libraries/chain/hotstuff/hotstuff.cpp +++ b/libraries/chain/hotstuff/hotstuff.cpp @@ -21,33 +21,32 @@ inline std::vector bitset_to_vector(const hs_bitset& bs) { } bool pending_quorum_certificate::has_voted(size_t index) const { - std::lock_guard g(*_mtx); - return _strong_votes._bitset.at(index) || _weak_votes._bitset.at(index); + return _strong_votes.has_voted(index) || _weak_votes.has_voted(index); } bool pending_quorum_certificate::has_voted_no_lock(bool strong, size_t index) const { if (strong) { - return _strong_votes._bitset[index]; + return _strong_votes.has_voted(index); } - return _weak_votes._bitset[index]; + return _weak_votes.has_voted(index); +} + +bool pending_quorum_certificate::votes_t::has_voted(size_t index) const { + assert(index <= _processed.size()); + return _processed[index].load(std::memory_order_relaxed); } + vote_status pending_quorum_certificate::votes_t::add_vote(size_t index, const bls_signature& sig) { if (_bitset[index]) { // check here as could have come in while unlocked return vote_status::duplicate; // shouldn't be already present } + _processed[index].store(true, std::memory_order_relaxed); _bitset.set(index); _sig.aggregate(sig); // works even if _sig is default initialized (fp2::zero()) return vote_status::success; } -void pending_quorum_certificate::votes_t::reset(size_t num_finalizers) { - if (num_finalizers != _bitset.size()) - _bitset.resize(num_finalizers); - _bitset.reset(); - _sig = bls_aggregate_signature(); -} - pending_quorum_certificate::pending_quorum_certificate() : _mtx(std::make_unique()) { } @@ -55,9 +54,9 @@ pending_quorum_certificate::pending_quorum_certificate() pending_quorum_certificate::pending_quorum_certificate(size_t num_finalizers, uint64_t quorum, uint64_t max_weak_sum_before_weak_final) : _mtx(std::make_unique()) , _quorum(quorum) - , _max_weak_sum_before_weak_final(max_weak_sum_before_weak_final) { - _weak_votes.resize(num_finalizers); - _strong_votes.resize(num_finalizers); + , _max_weak_sum_before_weak_final(max_weak_sum_before_weak_final) + , _weak_votes(num_finalizers) + , _strong_votes(num_finalizers) { } bool pending_quorum_certificate::is_quorum_met() const { @@ -133,25 +132,23 @@ vote_status pending_quorum_certificate::add_vote(block_num_type block_num, bool const bls_public_key& pubkey, const bls_signature& sig, uint64_t weight) { vote_status s = vote_status::success; - std::unique_lock g(*_mtx); - state_t pre_state = _state; - state_t post_state = pre_state; if (has_voted_no_lock(strong, index)) { - s = vote_status::duplicate; - } else { - g.unlock(); - if (!fc::crypto::blslib::verify(pubkey, proposal_digest, sig)) { - wlog( "signature from finalizer ${i} cannot be verified", ("i", index) ); - s = vote_status::invalid_signature; - } else { - g.lock(); - s = strong ? add_strong_vote(index, sig, weight) - : add_weak_vote(index, sig, weight); - post_state = _state; - g.unlock(); - } + dlog("block_num: ${bn}, vote strong: ${sv}, duplicate", ("bn", block_num)("sv", strong)); + return vote_status::duplicate; + } + + if (!fc::crypto::blslib::verify(pubkey, proposal_digest, sig)) { + wlog( "signature from finalizer ${i} cannot be verified", ("i", index) ); + return vote_status::invalid_signature; } + std::unique_lock g(*_mtx); + state_t pre_state = _state; + s = strong ? add_strong_vote(index, sig, weight) + : add_weak_vote(index, sig, weight); + state_t post_state = _state; + g.unlock(); + dlog("block_num: ${bn}, vote strong: ${sv}, status: ${s}, pre-state: ${pre}, post-state: ${state}, quorum_met: ${q}", ("bn", block_num)("sv", strong)("s", s)("pre", pre_state)("state", post_state)("q", is_quorum_met(post_state))); return s; diff --git a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp index a98cbd023a..f67f7d65be 100644 --- a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp +++ b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp @@ -79,15 +79,22 @@ namespace eosio::chain { }; struct votes_t { + private: + friend struct fc::reflector; + friend class pending_quorum_certificate; hs_bitset _bitset; bls_aggregate_signature _sig; + std::vector> _processed; // avoid locking mutex for _bitset duplicate check - void resize(size_t num_finalizers) { _bitset.resize(num_finalizers); } - size_t count() const { return _bitset.count(); } + public: + explicit votes_t(size_t num_finalizers) + : _bitset(num_finalizers) + , _processed(num_finalizers) {} - vote_status add_vote(size_t index, const bls_signature& sig); + // thread safe + bool has_voted(size_t index) const; - void reset(size_t num_finalizers); + vote_status add_vote(size_t index, const bls_signature& sig); }; pending_quorum_certificate(); @@ -124,8 +131,8 @@ namespace eosio::chain { state_t _state { state_t::unrestricted }; uint64_t _strong_sum {0}; // accumulated sum of strong votes so far uint64_t _weak_sum {0}; // accumulated sum of weak votes so far - votes_t _weak_votes; - votes_t _strong_votes; + votes_t _weak_votes {0}; + votes_t _strong_votes {0}; // called by add_vote, already protected by mutex vote_status add_strong_vote(size_t index,