From c8abbc9d1fd8f77b5dcf31cf7a6dd6aac5d1c75c Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 16 Oct 2020 00:18:44 +0200 Subject: p2p: declare Announcement::m_state as uint8_t, add getter/setter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to silence these Travis CI GCC compiler warnings: txrequest.cpp:73:21: warning: ‘{anonymous}::Announcement::m_state’ is too small to hold all values of ‘enum class {anonymous}::State’ State m_state : 3; ^ The warnings are based on the maximum value held by the underlying uint8_t enumerator type, though the intention of the bitfield declaration is the maximum declared enumerator value. The warning been silenced in GCC 8.4+ and 9.3+ according to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61414 --- src/txrequest.cpp | 100 +++++++++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 46 deletions(-) (limited to 'src') diff --git a/src/txrequest.cpp b/src/txrequest.cpp index 494786c201..09eb78e927 100644 --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -69,32 +69,40 @@ struct Announcement { /** Whether this is a wtxid request. */ const bool m_is_wtxid : 1; - /** What state this announcement is in. */ - State m_state : 3; + /** What state this announcement is in. + * This is a uint8_t instead of a State to silence a GCC warning in versions prior to 8.4 and 9.3. + * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61414 */ + uint8_t m_state : 3; + + /** Convert m_state to a State enum. */ + State GetState() const { return static_cast(m_state); } + + /** Convert a State enum to a uint8_t and store it in m_state. */ + void SetState(State state) { m_state = static_cast(state); } /** Whether this announcement is selected. There can be at most 1 selected peer per txhash. */ bool IsSelected() const { - return m_state == State::CANDIDATE_BEST || m_state == State::REQUESTED; + return GetState() == State::CANDIDATE_BEST || GetState() == State::REQUESTED; } /** Whether this announcement is waiting for a certain time to pass. */ bool IsWaiting() const { - return m_state == State::REQUESTED || m_state == State::CANDIDATE_DELAYED; + return GetState() == State::REQUESTED || GetState() == State::CANDIDATE_DELAYED; } /** Whether this announcement can feasibly be selected if the current IsSelected() one disappears. */ bool IsSelectable() const { - return m_state == State::CANDIDATE_READY || m_state == State::CANDIDATE_BEST; + return GetState() == State::CANDIDATE_READY || GetState() == State::CANDIDATE_BEST; } /** Construct a new announcement from scratch, initially in CANDIDATE_DELAYED state. */ Announcement(const GenTxid& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime, SequenceNumber sequence) : m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred), - m_is_wtxid(gtxid.IsWtxid()), m_state(State::CANDIDATE_DELAYED) {} + m_is_wtxid(gtxid.IsWtxid()), m_state(static_cast(State::CANDIDATE_DELAYED)) {} }; //! Type alias for priorities. @@ -143,7 +151,7 @@ struct ByPeerViewExtractor using result_type = ByPeerView; result_type operator()(const Announcement& ann) const { - return ByPeerView{ann.m_peer, ann.m_state == State::CANDIDATE_BEST, ann.m_txhash}; + return ByPeerView{ann.m_peer, ann.GetState() == State::CANDIDATE_BEST, ann.m_txhash}; } }; @@ -166,8 +174,8 @@ public: using result_type = ByTxHashView; result_type operator()(const Announcement& ann) const { - const Priority prio = (ann.m_state == State::CANDIDATE_READY) ? m_computer(ann) : 0; - return ByTxHashView{ann.m_txhash, ann.m_state, prio}; + const Priority prio = (ann.GetState() == State::CANDIDATE_READY) ? m_computer(ann) : 0; + return ByTxHashView{ann.m_txhash, ann.GetState(), prio}; } }; @@ -261,8 +269,8 @@ std::unordered_map RecomputePeerInfo(const Index& index) for (const Announcement& ann : index) { PeerInfo& info = ret[ann.m_peer]; ++info.m_total; - info.m_requested += (ann.m_state == State::REQUESTED); - info.m_completed += (ann.m_state == State::COMPLETED); + info.m_requested += (ann.GetState() == State::REQUESTED); + info.m_completed += (ann.GetState() == State::COMPLETED); } return ret; } @@ -274,15 +282,15 @@ std::map ComputeTxHashInfo(const Index& index, const Priori for (const Announcement& ann : index) { TxHashInfo& info = ret[ann.m_txhash]; // Classify how many announcements of each state we have for this txhash. - info.m_candidate_delayed += (ann.m_state == State::CANDIDATE_DELAYED); - info.m_candidate_ready += (ann.m_state == State::CANDIDATE_READY); - info.m_candidate_best += (ann.m_state == State::CANDIDATE_BEST); - info.m_requested += (ann.m_state == State::REQUESTED); + info.m_candidate_delayed += (ann.GetState() == State::CANDIDATE_DELAYED); + info.m_candidate_ready += (ann.GetState() == State::CANDIDATE_READY); + info.m_candidate_best += (ann.GetState() == State::CANDIDATE_BEST); + info.m_requested += (ann.GetState() == State::REQUESTED); // And track the priority of the best CANDIDATE_READY/CANDIDATE_BEST announcements. - if (ann.m_state == State::CANDIDATE_BEST) { + if (ann.GetState() == State::CANDIDATE_BEST) { info.m_priority_candidate_best = computer(ann); } - if (ann.m_state == State::CANDIDATE_READY) { + if (ann.GetState() == State::CANDIDATE_READY) { info.m_priority_best_candidate_ready = std::max(info.m_priority_best_candidate_ready, computer(ann)); } // Also keep track of which peers this txhash has an announcement for (so we can detect duplicates). @@ -369,8 +377,8 @@ private: Iter Erase(Iter it) { auto peerit = m_peerinfo.find(it->m_peer); - peerit->second.m_completed -= it->m_state == State::COMPLETED; - peerit->second.m_requested -= it->m_state == State::REQUESTED; + peerit->second.m_completed -= it->GetState() == State::COMPLETED; + peerit->second.m_requested -= it->GetState() == State::REQUESTED; if (--peerit->second.m_total == 0) m_peerinfo.erase(peerit); return m_index.get().erase(it); } @@ -380,11 +388,11 @@ private: void Modify(Iter it, Modifier modifier) { auto peerit = m_peerinfo.find(it->m_peer); - peerit->second.m_completed -= it->m_state == State::COMPLETED; - peerit->second.m_requested -= it->m_state == State::REQUESTED; + peerit->second.m_completed -= it->GetState() == State::COMPLETED; + peerit->second.m_requested -= it->GetState() == State::REQUESTED; m_index.get().modify(it, std::move(modifier)); - peerit->second.m_completed += it->m_state == State::COMPLETED; - peerit->second.m_requested += it->m_state == State::REQUESTED; + peerit->second.m_completed += it->GetState() == State::COMPLETED; + peerit->second.m_requested += it->GetState() == State::REQUESTED; } //! Convert a CANDIDATE_DELAYED announcement into a CANDIDATE_READY. If this makes it the new best @@ -393,26 +401,26 @@ private: void PromoteCandidateReady(Iter it) { assert(it != m_index.get().end()); - assert(it->m_state == State::CANDIDATE_DELAYED); + assert(it->GetState() == State::CANDIDATE_DELAYED); // Convert CANDIDATE_DELAYED to CANDIDATE_READY first. - Modify(it, [](Announcement& ann){ ann.m_state = State::CANDIDATE_READY; }); + Modify(it, [](Announcement& ann){ ann.SetState(State::CANDIDATE_READY); }); // The following code relies on the fact that the ByTxHash is sorted by txhash, and then by state (first // _DELAYED, then _READY, then _BEST/REQUESTED). Within the _READY announcements, the best one (highest // priority) comes last. Thus, if an existing _BEST exists for the same txhash that this announcement may // be preferred over, it must immediately follow the newly created _READY. auto it_next = std::next(it); if (it_next == m_index.get().end() || it_next->m_txhash != it->m_txhash || - it_next->m_state == State::COMPLETED) { + it_next->GetState() == State::COMPLETED) { // This is the new best CANDIDATE_READY, and there is no IsSelected() announcement for this txhash // already. - Modify(it, [](Announcement& ann){ ann.m_state = State::CANDIDATE_BEST; }); - } else if (it_next->m_state == State::CANDIDATE_BEST) { + Modify(it, [](Announcement& ann){ ann.SetState(State::CANDIDATE_BEST); }); + } else if (it_next->GetState() == State::CANDIDATE_BEST) { Priority priority_old = m_computer(*it_next); Priority priority_new = m_computer(*it); if (priority_new > priority_old) { // There is a CANDIDATE_BEST announcement already, but this one is better. - Modify(it_next, [](Announcement& ann){ ann.m_state = State::CANDIDATE_READY; }); - Modify(it, [](Announcement& ann){ ann.m_state = State::CANDIDATE_BEST; }); + Modify(it_next, [](Announcement& ann){ ann.SetState(State::CANDIDATE_READY); }); + Modify(it, [](Announcement& ann){ ann.SetState(State::CANDIDATE_BEST); }); } } } @@ -427,19 +435,19 @@ private: auto it_prev = std::prev(it); // The next best CANDIDATE_READY, if any, immediately precedes the REQUESTED or CANDIDATE_BEST // announcement in the ByTxHash index. - if (it_prev->m_txhash == it->m_txhash && it_prev->m_state == State::CANDIDATE_READY) { + if (it_prev->m_txhash == it->m_txhash && it_prev->GetState() == State::CANDIDATE_READY) { // If one such CANDIDATE_READY exists (for this txhash), convert it to CANDIDATE_BEST. - Modify(it_prev, [](Announcement& ann){ ann.m_state = State::CANDIDATE_BEST; }); + Modify(it_prev, [](Announcement& ann){ ann.SetState(State::CANDIDATE_BEST); }); } } - Modify(it, [new_state](Announcement& ann){ ann.m_state = new_state; }); + Modify(it, [new_state](Announcement& ann){ ann.SetState(new_state); }); } //! Check if 'it' is the only announcement for a given txhash that isn't COMPLETED. bool IsOnlyNonCompleted(Iter it) { assert(it != m_index.get().end()); - assert(it->m_state != State::COMPLETED); // Not allowed to call this on COMPLETED announcements. + assert(it->GetState() != State::COMPLETED); // Not allowed to call this on COMPLETED announcements. // This announcement has a predecessor that belongs to the same txhash. Due to ordering, and the // fact that 'it' is not COMPLETED, its predecessor cannot be COMPLETED here. @@ -447,7 +455,7 @@ private: // This announcement has a successor that belongs to the same txhash, and is not COMPLETED. if (std::next(it) != m_index.get().end() && std::next(it)->m_txhash == it->m_txhash && - std::next(it)->m_state != State::COMPLETED) return false; + std::next(it)->GetState() != State::COMPLETED) return false; return true; } @@ -460,7 +468,7 @@ private: assert(it != m_index.get().end()); // Nothing to be done if it's already COMPLETED. - if (it->m_state == State::COMPLETED) return true; + if (it->GetState() == State::COMPLETED) return true; if (IsOnlyNonCompleted(it)) { // This is the last non-COMPLETED announcement for this txhash. Delete all. @@ -490,9 +498,9 @@ private: // and convert them to CANDIDATE_READY and COMPLETED respectively. while (!m_index.empty()) { auto it = m_index.get().begin(); - if (it->m_state == State::CANDIDATE_DELAYED && it->m_time <= now) { + if (it->GetState() == State::CANDIDATE_DELAYED && it->m_time <= now) { PromoteCandidateReady(m_index.project(it)); - } else if (it->m_state == State::REQUESTED && it->m_time <= now) { + } else if (it->GetState() == State::REQUESTED && it->m_time <= now) { if (expired) expired->emplace_back(it->m_peer, ToGenTxid(*it)); MakeCompleted(m_index.project(it)); } else { @@ -596,7 +604,7 @@ public: std::vector selected; auto it_peer = m_index.get().lower_bound(ByPeerView{peer, true, uint256::ZERO}); while (it_peer != m_index.get().end() && it_peer->m_peer == peer && - it_peer->m_state == State::CANDIDATE_BEST) { + it_peer->GetState() == State::CANDIDATE_BEST) { selected.emplace_back(&*it_peer); ++it_peer; } @@ -625,8 +633,8 @@ public: // returned by GetRequestable always correspond to CANDIDATE_BEST announcements). it = m_index.get().find(ByPeerView{peer, false, txhash}); - if (it == m_index.get().end() || (it->m_state != State::CANDIDATE_DELAYED && - it->m_state != State::CANDIDATE_READY)) { + if (it == m_index.get().end() || (it->GetState() != State::CANDIDATE_DELAYED && + it->GetState() != State::CANDIDATE_READY)) { // There is no CANDIDATE announcement tracked for this peer, so we have nothing to do. Either this // txhash wasn't tracked at all (and the caller should have called ReceivedInv), or it was already // requested and/or completed for other reasons and this is just a superfluous RequestedTx call. @@ -638,24 +646,24 @@ public: // other CANDIDATE_BEST or REQUESTED can exist. auto it_old = m_index.get().lower_bound(ByTxHashView{txhash, State::CANDIDATE_BEST, 0}); if (it_old != m_index.get().end() && it_old->m_txhash == txhash) { - if (it_old->m_state == State::CANDIDATE_BEST) { + if (it_old->GetState() == State::CANDIDATE_BEST) { // The data structure's invariants require that there can be at most one CANDIDATE_BEST or one // REQUESTED announcement per txhash (but not both simultaneously), so we have to convert any // existing CANDIDATE_BEST to another CANDIDATE_* when constructing another REQUESTED. // It doesn't matter whether we pick CANDIDATE_READY or _DELAYED here, as SetTimePoint() // will correct it at GetRequestable() time. If time only goes forward, it will always be // _READY, so pick that to avoid extra work in SetTimePoint(). - Modify(it_old, [](Announcement& ann) { ann.m_state = State::CANDIDATE_READY; }); - } else if (it_old->m_state == State::REQUESTED) { + Modify(it_old, [](Announcement& ann) { ann.SetState(State::CANDIDATE_READY); }); + } else if (it_old->GetState() == State::REQUESTED) { // As we're no longer waiting for a response to the previous REQUESTED announcement, convert it // to COMPLETED. This also helps guaranteeing progress. - Modify(it_old, [](Announcement& ann) { ann.m_state = State::COMPLETED; }); + Modify(it_old, [](Announcement& ann) { ann.SetState(State::COMPLETED); }); } } } Modify(it, [expiry](Announcement& ann) { - ann.m_state = State::REQUESTED; + ann.SetState(State::REQUESTED); ann.m_time = expiry; }); } -- cgit v1.2.3