refactor: remove checking this == nullptr

.github/workflows/build.yml

@@ -83,7 +83,7 @@ jobs:
           key: ${{ runner.os }}-softfloat-${{ steps.softfloat-cache.outputs.version }}
       # build
       - run: cmake -E make_directory build
-      - run: cmake .. -DUSE_CACHED_LIBS=ON -DBUILD_SHARED_LIBS=OFF && make -j2
+      - run: cmake .. -DUSE_CACHED_LIBS=ON -DBUILD_SHARED_LIBS=OFF -DCHECK_CONNECTION=OFF && make -j2
         working-directory: ./build
       # test
       # - run: make test

src/noir/consensus/bit_array.h

@@ -39,26 +39,17 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
   }
 
   int size() const {
-    if (this == nullptr) { ///< NOT a very nice way of coding; need to refactor later
-      return 0;
-    }
     return bits;
   }
 
   bool get_index(int i) {
-    if (this == nullptr) { ///< NOT a very nice way of coding; need to refactor later
-      return false;
-    }
     std::scoped_lock g(mtx);
     if (i >= bits)
       return false;
     return elem[i];
   }
 
   bool set_index(int i, bool v) {
-    if (this == nullptr) { ///< NOT a very nice way of coding; need to refactor later
-      return false;
-    }
     std::scoped_lock g(mtx);
     if (i >= bits)
       return false;
@@ -67,15 +58,15 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
   }
 
   std::shared_ptr<bit_array> update(const std::shared_ptr<bit_array>& o) {
-    if (this == nullptr || o == nullptr)
+    if (o == nullptr)
       return nullptr;
     std::scoped_lock<std::mutex, std::mutex> g(mtx, o->mtx);
     elem = o->elem;
     return shared_from_this();
   }
 
   std::shared_ptr<bit_array> sub(const std::shared_ptr<bit_array>& o) {
-    if (this == nullptr || o == nullptr) ///< NOT a very nice way of coding; need to refactor later
+    if (o == nullptr)
       return nullptr;
     std::scoped_lock<std::mutex, std::mutex> g(mtx, o->mtx);
     auto c = copy_bits(bits);
@@ -87,10 +78,6 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
 
   /// \brief returns a bit_array resulting from a bitwise OR of two bit_arrays
   std::shared_ptr<bit_array> or_op(const std::shared_ptr<bit_array>& o) {
-    if (this == nullptr && o == nullptr)
-      return nullptr;
-    if (this == nullptr && o != nullptr)
-      return o->copy();
     if (o == nullptr)
       return copy();
     std::scoped_lock<std::mutex, std::mutex> g(mtx, o->mtx);
@@ -102,8 +89,6 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
   }
 
   std::shared_ptr<bit_array> not_op() {
-    if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later
-      return nullptr;
     std::scoped_lock g(mtx);
     auto c = copy();
     for (auto&& e : c->elem)
@@ -112,8 +97,6 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
   }
 
   std::shared_ptr<bit_array> copy() const {
-    if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later
-      return nullptr;
     auto copy = std::make_shared<bit_array>();
     copy->bits = bits;
     copy->elem = elem;
@@ -163,7 +146,7 @@ struct bit_array : public std::enable_shared_from_this<bit_array> {
 
   /// \brief returns a random index for a bit. If there is no value, returns 0, false
   std::tuple<int, bool> pick_random() {
-    if (this == nullptr || elem.empty())
+    if (elem.empty())
       return {0, false};
     std::scoped_lock g(mtx);
     auto true_indices = get_true_indices();

src/noir/consensus/consensus_reactor.cpp

@@ -300,8 +300,10 @@ void consensus_reactor::gossip_data_routine(std::shared_ptr<peer_state> ps) {
     int index{0};
     bool ok{false};
     if (rs->proposal_block_parts && rs->proposal_block_parts->has_header(prs->proposal_block_part_set_header)) {
-      std::tie(index, ok) =
-        rs->proposal_block_parts->get_bit_array()->sub(bit_array::copy(prs->proposal_block_parts))->pick_random();
+      auto pbp_bit_array = rs->proposal_block_parts->get_bit_array();
+      auto [index, ok] = pbp_bit_array && prs->proposal_block_parts
+        ? pbp_bit_array->sub(bit_array::copy(prs->proposal_block_parts))->pick_random()
+        : std::make_tuple(0, false);
     }
     if (ok) {
       // Send proposal_block_parts
@@ -363,7 +365,9 @@ void consensus_reactor::gossip_data_routine(std::shared_ptr<peer_state> ps) {
 void consensus_reactor::gossip_data_for_catchup(const std::shared_ptr<round_state>& rs,
   const std::shared_ptr<peer_round_state>& prs,
   const std::shared_ptr<peer_state>& ps) {
-  if (auto [index, ok] = prs->proposal_block_parts->not_op()->pick_random(); ok) {
+  auto [index, ok] =
+    prs->proposal_block_parts ? prs->proposal_block_parts->not_op()->pick_random() : std::make_tuple(0, false);
+  if (ok) {
     // Verify peer's part_set_header
     block_meta block_meta_;
     if (!cs_state->block_store_->load_block_meta(prs->height, block_meta_)) {

src/noir/consensus/consensus_state.cpp

@@ -856,7 +856,7 @@ void consensus_state::enter_precommit(int64_t height, int32_t round) {
   rs.locked_block = {};
   rs.locked_block_parts = {};
 
-  if (!rs.proposal_block_parts->has_header(block_id_->parts)) {
+  if (!rs.proposal_block_parts || !rs.proposal_block_parts->has_header(block_id_->parts)) {
     rs.proposal_block = {};
     rs.proposal_block_parts = part_set::new_part_set_from_header(block_id_->parts);
   }
@@ -925,7 +925,7 @@ void consensus_state::enter_commit(int64_t height, int32_t round) {
 
   // If we don't have the block being committed, set up to get it.
   if (!rs.proposal_block->hashes_to(block_id_->hash)) {
-    if (!rs.proposal_block_parts->has_header(block_id_->parts)) {
+    if (!rs.proposal_block_parts || !rs.proposal_block_parts->has_header(block_id_->parts)) {
       ilog("commit is for a block we do not know about; set ProposalBlock=nil");
       // We're getting the wrong block.
       // Set up ProposalBlockParts and keep waiting.
@@ -969,7 +969,7 @@ void consensus_state::finalize_commit(int64_t height) {
 
   if (!block_id_.has_value())
     throw std::runtime_error("panic: cannot finalize commit; commit does not have 2/3 majority");
-  if (!block_parts_->has_header(block_id_->parts))
+  if (!block_parts_ || !block_parts_->has_header(block_id_->parts))
     throw std::runtime_error("panic: expected ProposalBlockParts header to be commit header");
   if (!block_->hashes_to(block_id_->hash))
     throw std::runtime_error("panic: cannot finalize commit; proposal block does not hash to commit hash");
@@ -1255,7 +1255,7 @@ std::pair<bool, Error> consensus_state::add_vote(const std::shared_ptr<vote>& vo
           rs.proposal_block = {};
         }
 
-        if (!rs.proposal_block_parts->has_header(block_id_->parts)) {
+        if (!rs.proposal_block_parts || !rs.proposal_block_parts->has_header(block_id_->parts)) {
           rs.proposal_block_parts = part_set::new_part_set_from_header(block_id_->parts);
         }
 

src/noir/consensus/peer_state.h

@@ -149,7 +149,9 @@ struct peer_state {
     std::scoped_lock g(mtx);
     if (prs.height != height || prs.round != round)
       return;
-    prs.proposal_block_parts->set_index(index, true);
+    if (prs.proposal_block_parts) {
+      prs.proposal_block_parts->set_index(index, true);
+    }
   }
 
 private:
@@ -296,7 +298,8 @@ struct peer_state {
     if (!ps_votes)
       return {nullptr, false};
 
-    if (auto [index, ok] = votes.bit_array_->sub(ps_votes)->pick_random(); ok) {
+    auto [index, ok] = votes.bit_array_ ? votes.bit_array_->sub(ps_votes)->pick_random() : std::make_tuple(0, false);
+    if (ok) {
       if (!votes.get_by_index(index))
         return {nullptr, false};
       return {votes.get_by_index(index), true};

src/noir/consensus/types/block.cpp

@@ -134,14 +134,10 @@ bool part_set::add_part(std::shared_ptr<part> part_) {
 }
 
 Bytes part_set::get_hash() {
-  if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later
-    return merkle::hash_from_bytes_list({});
   return hash;
 }
 
 Bytes block_data::get_hash() {
-  if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later
-    return merkle::hash_from_bytes_list({});
   if (hash.empty()) {
     merkle::bytes_list items;
     for (const auto& tx : txs)

src/noir/consensus/types/block.h

@@ -239,8 +239,6 @@ struct part_set {
   }
 
   bool has_header(p2p::part_set_header header_) {
-    if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later
-      return false;
     return header() == header_;
   }
 
@@ -523,8 +521,6 @@ struct block {
   std::shared_ptr<part_set> make_part_set(uint32_t part_size);
 
   Bytes get_hash() {
-    if (this == nullptr)
-      return {};
     std::scoped_lock g(mtx);
     if (!last_commit)
       return {};