Simplify participation structures

- make checkpoint and pulse participation distinct types
- make the container act a little more like an stl container
- replace check_participation() with a failures() method that just
  returns the number of failures (so the caller can decide whether that
  is bad or not).
- As a consequence of the above, failures now trigger on N+1 failures,
  while previously they required N+1 failures *and* 8 total responses.
  There is no need to wait for all 8 as far as I can see since we will
  be failing no matter what the next 3 responses are.
- Removed the serialization code (hopefully this doesn't break
  everything outside the RPC code).
- Simplify/DRY the code that records participation

src/cryptonote_core/cryptonote_core.cpp

@@ -1652,9 +1652,7 @@ namespace cryptonote
               m_sn_times.add(entry);
 
               // Counts the number of times we have been out of sync
-              uint8_t num_sn_out_of_sync = std::count_if(m_sn_times.begin(), m_sn_times.end(),
-                [](const service_nodes::timesync_entry entry) { return !entry.in_sync; });
-              if (num_sn_out_of_sync > (m_sn_times.array.size() * service_nodes::MAXIMUM_EXTERNAL_OUT_OF_SYNC/100)) {
+              if (m_sn_times.failures() > (m_sn_times.size() * service_nodes::MAXIMUM_EXTERNAL_OUT_OF_SYNC/100)) {
                 MWARNING("service node time might be out of sync");
                 // If we are out of sync record the other service node as in sync
                 m_service_node_list.record_timesync_status(pubkey, true);

src/cryptonote_core/service_node_list.cpp

@@ -3190,57 +3190,29 @@ namespace service_nodes
   void service_node_list::record_checkpoint_participation(crypto::public_key const &pubkey, uint64_t height, bool participated)
   {
     std::lock_guard lock(m_sn_mutex);
-    if (!m_state.service_nodes_infos.count(pubkey))
-      return;
-
-    participation_entry entry  = {};
-    entry.height               = height;
-    entry.voted                = participated;
-
-    auto &info = proofs[pubkey];
-    info.checkpoint_participation.add(entry);
+    if (m_state.service_nodes_infos.count(pubkey))
+      proofs[pubkey].checkpoint_participation.add({height, participated});
   }
 
   void service_node_list::record_pulse_participation(crypto::public_key const &pubkey, uint64_t height, uint8_t round, bool participated)
   {
     std::lock_guard lock(m_sn_mutex);
-    if (!m_state.service_nodes_infos.count(pubkey))
-      return;
-
-    participation_entry entry  = {};
-    entry.is_pulse             = true;
-    entry.height               = height;
-    entry.voted                = participated;
-    entry.pulse.round          = round;
-
-    auto &info = proofs[pubkey];
-    info.pulse_participation.add(entry);
+    if (m_state.service_nodes_infos.count(pubkey))
+      proofs[pubkey].pulse_participation.add({height, round, participated});
   }
 
   void service_node_list::record_timestamp_participation(crypto::public_key const &pubkey, bool participated)
   {
     std::lock_guard lock(m_sn_mutex);
-    if (!m_state.service_nodes_infos.count(pubkey))
-      return;
-
-    timestamp_participation_entry entry  = {};
-    entry.participated                = participated;
-
-    auto &info = proofs[pubkey];
-    info.timestamp_participation.add(entry);
+    if (m_state.service_nodes_infos.count(pubkey))
+      proofs[pubkey].timestamp_participation.add({participated});
   }
 
   void service_node_list::record_timesync_status(crypto::public_key const &pubkey, bool synced)
   {
     std::lock_guard lock(m_sn_mutex);
-    if (!m_state.service_nodes_infos.count(pubkey))
-      return;
-
-    timesync_entry entry  = {};
-    entry.in_sync                = synced;
-
-    auto &info = proofs[pubkey];
-    info.timesync_status.add(entry);
+    if (m_state.service_nodes_infos.count(pubkey))
+      proofs[pubkey].timesync_status.add({synced});
   }
 
   std::optional<bool> proof_info::reachable_stats::reachable(const std::chrono::steady_clock::time_point& now) const {

src/cryptonote_core/service_node_list.h

@@ -51,89 +51,56 @@ namespace service_nodes
 {
   constexpr uint64_t INVALID_HEIGHT = static_cast<uint64_t>(-1);
 
-  OXEN_RPC_DOC_INTROSPECT
-  struct participation_entry
+  struct checkpoint_participation_entry
   {
-    bool is_pulse   = false;
     uint64_t height = INVALID_HEIGHT;
     bool voted      = true;
 
-    struct
-    {
-      uint8_t round = 0;
-    } pulse;
-
-    bool pass() const {
-      return voted;
-    }; 
-
-    BEGIN_KV_SERIALIZE_MAP()
-      KV_SERIALIZE(height);
-      KV_SERIALIZE(voted);
-      KV_SERIALIZE(is_pulse);
-      if (this_ref.is_pulse)
-      {
-        KV_SERIALIZE_N(pulse.round, "pulse_round");
-      }
-    END_KV_SERIALIZE_MAP()
+    bool pass() const { return voted; };
   };
+  struct pulse_participation_entry
+  {
+    uint64_t height = INVALID_HEIGHT;
+    uint8_t round = 0;
+    bool voted = true;
 
+    bool pass() const { return voted; }
+  };
   struct timestamp_participation_entry
   {
-    bool participated      = true;
-    bool pass() const {
-      return participated;
-    }; 
-
-    BEGIN_KV_SERIALIZE_MAP()
-      KV_SERIALIZE(participated);
-    END_KV_SERIALIZE_MAP()
+    bool participated = true;
+    bool pass() const { return participated; }; 
   };
-
   struct timesync_entry
   {
-    bool in_sync       = true;
-    bool pass() const {
-      return in_sync;
-    }; 
-
-    BEGIN_KV_SERIALIZE_MAP()
-      KV_SERIALIZE(in_sync);
-    END_KV_SERIALIZE_MAP()
+    bool in_sync = true;
+    bool pass() const { return in_sync; }
   };
 
   template <typename ValueType, size_t Count = QUORUM_VOTE_CHECK_COUNT>
   struct participation_history {
-    std::array<ValueType, Count> array;
-    size_t write_index;
+    std::array<ValueType, Count> history;
+    size_t write_index = 0;
 
     void reset() { write_index = 0; }
 
-    void add(ValueType &entry)
-    {
-      size_t real_write_index = write_index % array.size();
-      array[real_write_index] = entry;
-      write_index++;
+    void add(const ValueType& entry) { history[write_index++ % history.size()] = entry; }
+    void add(ValueType&& entry) { history[write_index++ % history.size()] = std::move(entry); }
+
+    // Returns the number of failures we have stored (of the last Count records).
+    size_t failures() const {
+      return std::count_if(begin(), end(), [](auto& e) { return !e.pass(); });
     }
+    size_t passes() const { return size() - failures(); }
 
-    bool check_participation(uint16_t threshold)
-    {
-      if (this->write_index >= Count)
-      {
-        int failed_counter = 0;
-        for (ValueType &entry : array)
-          if (!entry.pass()) failed_counter++;
+    bool empty() const { return write_index == 0; }
+    size_t size() const { return std::min(history.size(), write_index); }
+    constexpr size_t max_size() const noexcept { return Count; }
 
-        if (failed_counter > threshold)
-          return false;
-      }
-      return true;
-    }
-
-    ValueType *begin()       { return array.data(); }
-    ValueType *end()         { return array.data() + std::min(array.size(), write_index); }
-    ValueType const *begin() const { return array.data(); }
-    ValueType const *end()   const { return array.data() + std::min(array.size(), write_index); }
+    ValueType* begin() { return history.data(); }
+    ValueType* end() { return history.data() + size(); }
+    const ValueType* begin() const { return history.data(); }
+    const ValueType* end() const { return history.data() + size(); }
   };
 
   inline constexpr auto NEVER = std::chrono::steady_clock::time_point::min();
@@ -142,10 +109,10 @@ namespace service_nodes
   {
     proof_info();
 
-    participation_history<participation_entry> pulse_participation{};
-    participation_history<participation_entry> checkpoint_participation{};
-    participation_history<timestamp_participation_entry> timestamp_participation{};
-    participation_history<timesync_entry> timesync_status{};
+    participation_history<pulse_participation_entry> pulse_participation;
+    participation_history<checkpoint_participation_entry> checkpoint_participation;
+    participation_history<timestamp_participation_entry> timestamp_participation;
+    participation_history<timesync_entry> timesync_status;
 
     uint64_t timestamp           = 0; // The actual time we last received an uptime proof (serialized)
     uint64_t effective_timestamp = 0; // Typically the same, but on recommissions it is set to the recommission block time to fend off instant obligation checks

src/cryptonote_core/service_node_quorum_cop.cpp

@@ -83,10 +83,10 @@ namespace service_nodes
     uint64_t timestamp = 0;
     decltype(std::declval<proof_info>().public_ips) ips{};
 
-    service_nodes::participation_history<service_nodes::participation_entry> checkpoint_participation{};
-    service_nodes::participation_history<service_nodes::participation_entry> pulse_participation{};
-    service_nodes::participation_history<service_nodes::timestamp_participation_entry> timestamp_participation{};
-    service_nodes::participation_history<service_nodes::timesync_entry> timesync_status{};
+    participation_history<service_nodes::checkpoint_participation_entry> checkpoint_participation{};
+    participation_history<service_nodes::pulse_participation_entry> pulse_participation{};
+    participation_history<service_nodes::timestamp_participation_entry> timestamp_participation{};
+    participation_history<service_nodes::timesync_entry> timesync_status{};
 
     constexpr std::array<uint16_t, 3> MIN_TIMESTAMP_VERSION{9,1,0};
 
@@ -99,7 +99,6 @@ namespace service_nodes
       ips                      = proof.public_ips;
       checkpoint_participation = proof.checkpoint_participation;
       pulse_participation      = proof.pulse_participation;
-
       timestamp_participation  = proof.timestamp_participation;
       timesync_status          = proof.timesync_status;
 
@@ -147,24 +146,24 @@ namespace service_nodes
 
     if (!info.is_decommissioned())
     {
-      if (check_checkpoint_obligation && !checkpoint_participation.check_participation(CHECKPOINT_MAX_MISSABLE_VOTES) )
+      if (check_checkpoint_obligation && checkpoint_participation.failures() > CHECKPOINT_MAX_MISSABLE_VOTES)
       {
         LOG_PRINT_L1("Service Node: " << pubkey << ", failed checkpoint obligation check");
         result.checkpoint_participation = false;
       }
 
-      if (!pulse_participation.check_participation(PULSE_MAX_MISSABLE_VOTES) )
+      if (pulse_participation.failures() > PULSE_MAX_MISSABLE_VOTES)
       {
         LOG_PRINT_L1("Service Node: " << pubkey << ", failed pulse obligation check");
         result.pulse_participation = false;
       }
 
-      if (!timestamp_participation.check_participation(TIMESTAMP_MAX_MISSABLE_VOTES) )
+      if (timestamp_participation.failures() > TIMESTAMP_MAX_MISSABLE_VOTES)
       {
         LOG_PRINT_L1("Service Node: " << pubkey << ", failed timestamp obligation check");
         result.timestamp_participation = false;
       }
-      if (!timesync_status.check_participation(TIMESYNC_MAX_UNSYNCED_VOTES) )
+      if (timesync_status.failures() > TIMESYNC_MAX_UNSYNCED_VOTES)
       {
         LOG_PRINT_L1("Service Node: " << pubkey << ", failed timesync obligation check");
         result.timesync_status = false;