From 422679066e0c12ae094a1ececabe62cb28b27e26 Mon Sep 17 00:00:00 2001 From: code0xff Date: Thu, 14 Jul 2022 11:03:36 +0900 Subject: [PATCH 1/3] refactor: remove checking this == nullptr --- src/noir/consensus/bit_array.h | 23 +++-------------------- src/noir/consensus/consensus_reactor.cpp | 10 +++++++--- src/noir/consensus/consensus_state.cpp | 8 ++++---- src/noir/consensus/peer_state.h | 7 +++++-- src/noir/consensus/types/block.cpp | 4 ---- src/noir/consensus/types/block.h | 4 ---- 6 files changed, 19 insertions(+), 37 deletions(-) diff --git a/src/noir/consensus/bit_array.h b/src/noir/consensus/bit_array.h index ec3fb6fc..65d26fe8 100644 --- a/src/noir/consensus/bit_array.h +++ b/src/noir/consensus/bit_array.h @@ -39,16 +39,10 @@ struct bit_array : public std::enable_shared_from_this { } 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; @@ -56,9 +50,6 @@ struct bit_array : public std::enable_shared_from_this { } 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,7 +58,7 @@ struct bit_array : public std::enable_shared_from_this { } std::shared_ptr update(const std::shared_ptr& o) { - if (this == nullptr || o == nullptr) + if (o == nullptr) return nullptr; std::scoped_lock g(mtx, o->mtx); elem = o->elem; @@ -75,7 +66,7 @@ struct bit_array : public std::enable_shared_from_this { } std::shared_ptr sub(const std::shared_ptr& 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 g(mtx, o->mtx); auto c = copy_bits(bits); @@ -87,10 +78,6 @@ struct bit_array : public std::enable_shared_from_this { /// \brief returns a bit_array resulting from a bitwise OR of two bit_arrays std::shared_ptr or_op(const std::shared_ptr& o) { - if (this == nullptr && o == nullptr) - return nullptr; - if (this == nullptr && o != nullptr) - return o->copy(); if (o == nullptr) return copy(); std::scoped_lock g(mtx, o->mtx); @@ -102,8 +89,6 @@ struct bit_array : public std::enable_shared_from_this { } std::shared_ptr 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 { } std::shared_ptr copy() const { - if (this == nullptr) ///< NOT a very nice way of coding; need to refactor later - return nullptr; auto copy = std::make_shared(); copy->bits = bits; copy->elem = elem; @@ -163,7 +146,7 @@ struct bit_array : public std::enable_shared_from_this { /// \brief returns a random index for a bit. If there is no value, returns 0, false std::tuple pick_random() { - if (this == nullptr || elem.empty()) + if (elem.empty()) return {0, false}; std::scoped_lock g(mtx); auto true_indices = get_true_indices(); diff --git a/src/noir/consensus/consensus_reactor.cpp b/src/noir/consensus/consensus_reactor.cpp index ac2f2a98..47b1b206 100644 --- a/src/noir/consensus/consensus_reactor.cpp +++ b/src/noir/consensus/consensus_reactor.cpp @@ -300,8 +300,10 @@ void consensus_reactor::gossip_data_routine(std::shared_ptr 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 ps) { void consensus_reactor::gossip_data_for_catchup(const std::shared_ptr& rs, const std::shared_ptr& prs, const std::shared_ptr& 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_)) { diff --git a/src/noir/consensus/consensus_state.cpp b/src/noir/consensus/consensus_state.cpp index e4d17e39..39a8f92d 100644 --- a/src/noir/consensus/consensus_state.cpp +++ b/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 consensus_state::add_vote(const std::shared_ptr& 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); } diff --git a/src/noir/consensus/peer_state.h b/src/noir/consensus/peer_state.h index 49b08d22..dbec67ff 100644 --- a/src/noir/consensus/peer_state.h +++ b/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}; diff --git a/src/noir/consensus/types/block.cpp b/src/noir/consensus/types/block.cpp index 7f318427..ff49f01b 100644 --- a/src/noir/consensus/types/block.cpp +++ b/src/noir/consensus/types/block.cpp @@ -134,14 +134,10 @@ bool part_set::add_part(std::shared_ptr 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) diff --git a/src/noir/consensus/types/block.h b/src/noir/consensus/types/block.h index d0bf2183..aace11eb 100644 --- a/src/noir/consensus/types/block.h +++ b/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 make_part_set(uint32_t part_size); Bytes get_hash() { - if (this == nullptr) - return {}; std::scoped_lock g(mtx); if (!last_commit) return {}; From 25ee2369874db41517f45771a813e44f3944110b Mon Sep 17 00:00:00 2001 From: Jeeyong Um Date: Thu, 14 Jul 2022 14:35:53 +0800 Subject: [PATCH 2/3] build: disable check connection on macos build --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 004a15dc..9a9aec50 100644 --- a/.github/workflows/build.yml +++ b/.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 From 9d22017d977a5e1ba62a94e7e21a7c9dae7cbae9 Mon Sep 17 00:00:00 2001 From: Jeeyong Um Date: Thu, 14 Jul 2022 15:19:51 +0800 Subject: [PATCH 3/3] build: update fc --- libs/fc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/fc b/libs/fc index ef8d71bf..d1974990 160000 --- a/libs/fc +++ b/libs/fc @@ -1 +1 @@ -Subproject commit ef8d71bfe84b57491ed12046dcbe1369807bf1d0 +Subproject commit d19749901d2a039f75d30c3fd7f803949f4241d7