diff --git a/src/noir/consensus/block_executor.h b/src/noir/consensus/block_executor.h index 7880e3f0..967c8a3e 100644 --- a/src/noir/consensus/block_executor.h +++ b/src/noir/consensus/block_executor.h @@ -92,8 +92,8 @@ struct block_executor { return true; /// Validate block - if (auto err = block_->validate_basic(); err.has_value()) { - elog(fmt::format("invalid header: {}", err.value())); + if (auto ok = block_->validate_basic(); !ok) { + elog(fmt::format("invalid header: {}", ok.error())); return false; } @@ -363,9 +363,8 @@ struct block_executor { if (abci_responses_->end_block.consensus_param_updates.has_value()) { // Note: must not mutate consensus_params next_params = abci_responses_->end_block.consensus_param_updates.value(); // todo - check if this is correct - auto err = next_params.validate_consensus_params(); - if (err.has_value()) { - elog(fmt::format("error updating consensus_params: {}", err.value())); + if (auto ok = next_params.validate_consensus_params(); !ok) { + elog(fmt::format("error updating consensus_params: {}", ok.error())); return {}; } diff --git a/src/noir/consensus/block_sync/reactor.cpp b/src/noir/consensus/block_sync/reactor.cpp index 9f803325..e6e30f5e 100644 --- a/src/noir/consensus/block_sync/reactor.cpp +++ b/src/noir/consensus/block_sync/reactor.cpp @@ -136,16 +136,16 @@ void reactor::try_sync_ticker() { auto first_id = p2p::block_id{first->get_hash(), first_part_set_header}; // Verify the first block using the second's commit - if (auto err = verify_commit_light(chain_id, latest_state.validators, first_id, first->header.height, + if (auto ok = verify_commit_light(chain_id, latest_state.validators, first_id, first->header.height, std::make_shared(*second->last_commit)); - err.has_value()) { - elog(fmt::format("invalid last commit: height={} err={}", first->header.height, err.value())); + !ok) { + elog(fmt::format("invalid last commit: height={} err={}", first->header.height, ok.error())); // We already removed peer request, but we need to clean up the rest auto peer_id1 = pool->redo_request(first->header.height); - pool->send_error(err.value(), peer_id1); + pool->send_error(ok.error().message(), peer_id1); auto peer_id2 = pool->redo_request(second->header.height); - pool->send_error(err.value(), peer_id2); + pool->send_error(ok.error().message(), peer_id2); } else { pool->pop_request(); diff --git a/src/noir/consensus/consensus_reactor.cpp b/src/noir/consensus/consensus_reactor.cpp index ac2f2a98..cb6892a6 100644 --- a/src/noir/consensus/consensus_reactor.cpp +++ b/src/noir/consensus/consensus_reactor.cpp @@ -112,8 +112,8 @@ void consensus_reactor::process_peer_msg(p2p::envelope_ptr info) { return; // Peer claims to have a maj23 for some block_id - if (auto err = votes->set_peer_maj23(msg.round, msg.type, ps->peer_id, msg.block_id_); err.has_value()) { - elog("${err}", ("err", err)); + if (auto ok = votes->set_peer_maj23(msg.round, msg.type, ps->peer_id, msg.block_id_); !ok) { + elog(ok.error().message()); return; } diff --git a/src/noir/consensus/consensus_state.cpp b/src/noir/consensus/consensus_state.cpp index e4d17e39..0bfdbf2c 100644 --- a/src/noir/consensus/consensus_state.cpp +++ b/src/noir/consensus/consensus_state.cpp @@ -676,7 +676,7 @@ void consensus_state::decide_proposal(int64_t height, int32_t round) { auto prop_block_id = p2p::block_id{block_->get_hash(), block_parts_->header()}; auto proposal_ = p2p::proposal_message{p2p::Proposal, height, round, rs.valid_round, prop_block_id, get_time()}; - if (auto err = local_priv_validator->sign_proposal(local_state.chain_id, proposal_); !err.has_value()) { + if (auto ok = local_priv_validator->sign_proposal(local_state.chain_id, proposal_); ok) { // proposal_.signature = p.signature; // TODO: no need; already updated proposal_.signature // Send proposal and block_parts diff --git a/src/noir/consensus/merkle/proof.h b/src/noir/consensus/merkle/proof.h index 9501098d..fe7fb35f 100644 --- a/src/noir/consensus/merkle/proof.h +++ b/src/noir/consensus/merkle/proof.h @@ -6,6 +6,7 @@ #pragma once #include #include +#include #include #include @@ -18,18 +19,18 @@ struct proof { Bytes leaf_hash{}; bytes_list aunts{}; - std::optional verify(const Bytes& root_hash, const Bytes& leaf) { + noir::Result verify(const Bytes& root_hash, const Bytes& leaf) { if (total < 0) - return "proof total must be positive"; + return noir::Error("proof total must be positive"); if (index < 0) - return "proof index must be positive"; + return noir::Error("proof index must be positive"); auto leaf_hash_ = leaf_hash_opt(leaf); if (leaf_hash_ != leaf_hash) - return "invalid leaf hash"; + return noir::Error("invalid leaf hash"); auto computed_hash = compute_root_hash(); if (computed_hash != root_hash) - return "invalid root hash"; - return {}; + return noir::Error("invalid root hash"); + return success(); } Bytes compute_root_hash() const { diff --git a/src/noir/consensus/merkle/test/tree_test.cpp b/src/noir/consensus/merkle/test/tree_test.cpp index 314c16e8..47007adc 100644 --- a/src/noir/consensus/merkle/test/tree_test.cpp +++ b/src/noir/consensus/merkle/test/tree_test.cpp @@ -70,20 +70,20 @@ TEST_CASE("merkle_tree: Verify proof", "[noir][consensus]") { CHECK(proof->index == i); CHECK(proof->total == total); - auto err = proof->verify(root_hash, items[i]); - CHECK(!err.has_value()); + auto ok = proof->verify(root_hash, items[i]); + CHECK(!ok.has_error()); // Trail too long should fail auto orig_aunts = proof->aunts; proof->aunts.push_back({static_cast(i % 256)}); - err = proof->verify(root_hash, items[i]); - CHECK(err.value() == "invalid root hash"); + ok = proof->verify(root_hash, items[i]); + CHECK(ok.error().message() == "invalid root hash"); proof->aunts = orig_aunts; // Trail too short should fail proof->aunts.pop_back(); - err = proof->verify(root_hash, items[i]); - CHECK(err.value() == "invalid root hash"); + ok = proof->verify(root_hash, items[i]); + CHECK(ok.error().message() == "invalid root hash"); proof->aunts = orig_aunts; } } diff --git a/src/noir/consensus/privval/file.h b/src/noir/consensus/privval/file.h index 7b00fcc3..093416dd 100644 --- a/src/noir/consensus/privval/file.h +++ b/src/noir/consensus/privval/file.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -172,22 +173,21 @@ struct file_pv : public noir::consensus::priv_validator { /// \brief signs a canonical representation of the vote, along with the chainID /// \param[in] vote_ /// \return - std::optional sign_vote(const std::string& chain_id, noir::consensus::vote& vote) override { + noir::Result sign_vote(const std::string& chain_id, noir::consensus::vote& vote) override { if (auto ok = sign_vote_internal(chain_id, vote); !ok) { - return "error signing vote" + ok.error().message(); + return noir::Error::format("error signing vote {}", ok.error()); } - return {}; + return success(); } /// \brief signs a canonical representation of the proposal, along with the chainID /// \param[in] proposal_ /// \return - std::optional sign_proposal( - const std::string& chain_id, noir::p2p::proposal_message& proposal) override { + noir::Result sign_proposal(const std::string& chain_id, noir::p2p::proposal_message& proposal) override { if (auto ok = sign_proposal_internal(chain_id, proposal); !ok) { - return "error signing proposal" + ok.error().message(); + return noir::Error::format("error signing proposal {}", ok.error()); } - return {}; + return success(); } Result sign_vote_pb(const std::string& chain_id, const ::tendermint::types::Vote& v) override { diff --git a/src/noir/consensus/privval/test/file_test.cpp b/src/noir/consensus/privval/test/file_test.cpp index c694f2fc..ecc22848 100644 --- a/src/noir/consensus/privval/test/file_test.cpp +++ b/src/noir/consensus/privval/test/file_test.cpp @@ -193,11 +193,10 @@ TEST_CASE("priv_val_file: test file_pv", "[noir][consensus]") { auto data_vote1 = vote::vote_sign_bytes(test_chain_id, *vote::to_proto(vote_)); // std::cout << "data_vote1=" << to_hex(data_vote1) << std::endl; // std::cout << "digest1=" << fc::Sha256::hash(data_vote1).str() << std::endl; - std::optional sig_org; if (st.expect_throw) { CHECK_THROWS(file_pv_ptr->sign_vote(test_chain_id, vote_)); } else { - CHECK_NOTHROW(sig_org = file_pv_ptr->sign_vote(test_chain_id, vote_)); + CHECK_NOTHROW(file_pv_ptr->sign_vote(test_chain_id, vote_)); // std::cout << "sig=" << std::string(vote_.signature.begin(), vote_.signature.end()) << std::endl; auto data_vote2 = vote::vote_sign_bytes(test_chain_id, *vote::to_proto(vote_)); diff --git a/src/noir/consensus/types/block.cpp b/src/noir/consensus/types/block.cpp index 7f318427..aa59ece6 100644 --- a/src/noir/consensus/types/block.cpp +++ b/src/noir/consensus/types/block.cpp @@ -120,7 +120,7 @@ bool part_set::add_part(std::shared_ptr part_) { } // Check hash proof - if (auto err = part_->proof_.verify(get_hash(), part_->bytes_); err.has_value()) { + if (auto ok = part_->proof_.verify(get_hash(), part_->bytes_); !ok) { elog("error part set invalid proof"); return false; } diff --git a/src/noir/consensus/types/block.h b/src/noir/consensus/types/block.h index d0bf2183..dae116be 100644 --- a/src/noir/consensus/types/block.h +++ b/src/noir/consensus/types/block.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -327,19 +328,19 @@ struct block_header { proposer_address = proposer_address_; } - std::optional validate_basic() { + noir::Result validate_basic() { if (chain_id.length() > 50) - return "chain_id is too long"; + return noir::Error("chain_id is too long"); if (height < 0) - return "negative height"; + return noir::Error("negative height"); else if (height == 0) - return "zero height"; + return noir::Error("zero height"); // last_block_id->validate_basic(); // validate_hash(last_commit_hash); // validate_hash(data_hash); // validate_hash(evidence_hash); // todo - add more - return {}; + return success(); } static Result make_header(block_header&& h) { @@ -369,8 +370,8 @@ struct block_header { h.evidence_hash = random_hash(); if (h.proposer_address.empty()) h.proposer_address = random_address(); - if (auto r = h.validate_basic(); r.has_value()) - return Error::format("{}", r.value()); + if (auto ok = h.validate_basic(); !ok) + return ok.error(); return h; } @@ -478,11 +479,11 @@ struct block { static std::shared_ptr new_block_from_part_set(const std::shared_ptr& ps); - std::optional validate_basic() { + noir::Result validate_basic() { std::scoped_lock g(mtx); - if (auto err = header.validate_basic(); err.has_value()) - return "invalid header: " + err.value(); + if (auto ok = header.validate_basic(); !ok) + return noir::Error::format("invalid header: {}", ok.error()); // Validate last commit // if (last_commit.height == 0) @@ -491,7 +492,7 @@ struct block { // if (last_commit.get_hash() != header.last_commit_hash) // return "wrong last_commit_hash"; - return {}; + return success(); } void fill_header() { diff --git a/src/noir/consensus/types/event_bus.h b/src/noir/consensus/types/event_bus.h index 1d2c3d06..e830e987 100644 --- a/src/noir/consensus/types/event_bus.h +++ b/src/noir/consensus/types/event_bus.h @@ -6,6 +6,7 @@ #pragma once #include +#include #include #include #include @@ -49,27 +50,27 @@ class event_bus { subscription(const subscription&) = delete; subscription& operator=(const subscription&) = delete; - std::optional unsubscribe() { + noir::Result unsubscribe() { auto handle_ptr = handle_.lock(); auto map_ptr = map_.lock(); if (!handle_ptr || !map_ptr) { - return "invalid subscription"; + return noir::Error("invalid subscription"); } handle_ptr->unsubscribe(); auto it = map_ptr->find(subscriber); if (it == map_ptr->end()) { - return fmt::format("invalid subscriber:{}", subscriber); + return noir::Error::format("invalid subscriber:{}", subscriber); } auto it2 = it->second.find(id); if (it2 == it->second.end()) { - return fmt::format("invalid subscription id: {}", id); + return noir::Error::format("invalid subscription id: {}", id); } it->second.erase(id); if (it->second.size() == 0) { // if all elements are deleted map_ptr->erase(subscriber); } - return {}; + return success(); } private: @@ -105,20 +106,20 @@ class event_bus { return std::move(ret); } - std::optional unsubscribe(subscription& handle) { + noir::Result unsubscribe(subscription& handle) { return handle.unsubscribe(); } - std::optional unsubscribe_all(const std::string& subscriber) { + noir::Result unsubscribe_all(const std::string& subscriber) { auto it = subscription_map_->find(subscriber); if (it == subscription_map_->end()) { - return fmt::format("invalid subscriber:{}", subscriber); + return noir::Error::format("invalid subscriber:{}", subscriber); } for (auto it2 = it->second.begin(); it2 != it->second.end(); ++it2) { it2->second->unsubscribe(); } subscription_map_->erase(subscriber); - return {}; + return success(); } void publish(const std::string& event_value, const tm_event_data& data) { diff --git a/src/noir/consensus/types/genesis.cpp b/src/noir/consensus/types/genesis.cpp index ce8b7cdc..ce3cb0f1 100644 --- a/src/noir/consensus/types/genesis.cpp +++ b/src/noir/consensus/types/genesis.cpp @@ -70,9 +70,8 @@ bool genesis_doc::validate_and_complete() { if (!cs_params.has_value()) { cs_params = consensus_params::get_default(); } else { - auto err = cs_params->validate_consensus_params(); - if (err.has_value()) { - elog(err.value()); + if (auto ok = cs_params->validate_consensus_params(); !ok) { + elog(ok.error().message()); return false; } } diff --git a/src/noir/consensus/types/height_vote_set.h b/src/noir/consensus/types/height_vote_set.h index d8d2a643..0a1bc517 100644 --- a/src/noir/consensus/types/height_vote_set.h +++ b/src/noir/consensus/types/height_vote_set.h @@ -7,6 +7,7 @@ #include #include #include +#include #include namespace noir::consensus { @@ -79,14 +80,14 @@ struct height_vote_set { round = round_; } - std::optional set_peer_maj23( + noir::Result set_peer_maj23( int32_t round, p2p::signed_msg_type vote_type, std::string peer_id, p2p::block_id block_id_) { std::scoped_lock g(mtx); if (vote_type != p2p::Prevote && vote_type != p2p::Precommit) - return "setPeerMaj23: Invalid vote type"; + return noir::Error("setPeerMaj23: Invalid vote type"); auto vote_set_ = get_vote_set(round, vote_type); if (vote_set_ == nullptr) - return {}; + return success(); return vote_set_->set_peer_maj23(peer_id, block_id_); } diff --git a/src/noir/consensus/types/params.h b/src/noir/consensus/types/params.h index 523750fb..e709fd74 100644 --- a/src/noir/consensus/types/params.h +++ b/src/noir/consensus/types/params.h @@ -5,6 +5,7 @@ // #pragma once #include +#include #include #include #include @@ -56,18 +57,18 @@ struct consensus_params { validator_params validator; version_params version; - std::optional validate_consensus_params() const { + noir::Result validate_consensus_params() const { if (block.max_bytes <= 0) - return "block.MaxBytes must be greater than 0."; + return noir::Error("block.MaxBytes must be greater than 0."); if (block.max_bytes > max_block_size_bytes) - return "block.MaxBytes is too big."; + return noir::Error("block.MaxBytes is too big."); if (block.max_gas < -1) - return "block.MaxGas must be greater or equal to -1."; + return noir::Error("block.MaxGas must be greater or equal to -1."); // check evidence // todo - necessary? // if (validator.pub_key_types.empty()) // return "validator.pub_key_types must not be empty."; // check if key_type is known // todo - return {}; + return success(); } static consensus_params get_default() { diff --git a/src/noir/consensus/types/priv_validator.cpp b/src/noir/consensus/types/priv_validator.cpp index 90b4ce42..adf05e76 100644 --- a/src/noir/consensus/types/priv_validator.cpp +++ b/src/noir/consensus/types/priv_validator.cpp @@ -9,22 +9,22 @@ namespace noir::consensus { -std::optional mock_pv::sign_vote(const std::string& chain_id, vote& vote_) { +noir::Result mock_pv::sign_vote(const std::string& chain_id, vote& vote_) { // TODO: add some validation checks auto vote_sign_bytes = vote::vote_sign_bytes(chain_id, *vote::to_proto(vote_)); auto sig = priv_key_.sign(vote_sign_bytes); vote_.signature = sig; - return {}; + return success(); } -std::optional mock_pv::sign_proposal(const std::string& chain_id, noir::p2p::proposal_message& msg) { +noir::Result mock_pv::sign_proposal(const std::string& chain_id, noir::p2p::proposal_message& msg) { // TODO: add some validation checks auto sign_bytes = proposal::proposal_sign_bytes(chain_id, *proposal::to_proto({msg})); auto sig = priv_key_.sign(sign_bytes); msg.signature = sig; - return {}; + return success(); } Result mock_pv::sign_vote_pb(const std::string& chain_id, const ::tendermint::types::Vote& v) { diff --git a/src/noir/consensus/types/priv_validator.h b/src/noir/consensus/types/priv_validator.h index 716e6d44..2ccbb60f 100644 --- a/src/noir/consensus/types/priv_validator.h +++ b/src/noir/consensus/types/priv_validator.h @@ -6,6 +6,7 @@ #pragma once #include #include +#include namespace noir::consensus { @@ -22,9 +23,8 @@ struct priv_validator { virtual priv_validator_type get_type() const = 0; virtual pub_key get_pub_key() const = 0; virtual priv_key get_priv_key() const = 0; - virtual std::optional sign_vote(const std::string& chain_id, vote& vote_) = 0; - virtual std::optional sign_proposal( - const std::string& chain_id, noir::p2p::proposal_message& proposal_) = 0; + virtual noir::Result sign_vote(const std::string& chain_id, vote& vote_) = 0; + virtual noir::Result sign_proposal(const std::string& chain_id, noir::p2p::proposal_message& proposal_) = 0; virtual Result sign_vote_pb(const std::string& chain_id, const ::tendermint::types::Vote& v) = 0; }; @@ -41,9 +41,8 @@ struct mock_pv : public priv_validator { priv_key get_priv_key() const override { return priv_key_; } - std::optional sign_vote(const std::string& chain_id, vote& vote_) override; - std::optional sign_proposal( - const std::string& chain_id, noir::p2p::proposal_message& proposal_) override; + noir::Result sign_vote(const std::string& chain_id, vote& vote_) override; + noir::Result sign_proposal(const std::string& chain_id, noir::p2p::proposal_message& proposal_) override; Result sign_vote_pb(const std::string& chain_id, const ::tendermint::types::Vote& v) override; }; diff --git a/src/noir/consensus/types/test/event_bus_test.cpp b/src/noir/consensus/types/test/event_bus_test.cpp index 5ca7eac0..dbb975ba 100644 --- a/src/noir/consensus/types/test/event_bus_test.cpp +++ b/src/noir/consensus/types/test/event_bus_test.cpp @@ -31,8 +31,8 @@ TEST_CASE("event_bus: subscribe/unsubscribe", "[noir][consensus][events]") { invalid_subscription.subscriber = "invalid_subscriber"; invalid_subscription.id = "invalid_id"; - CHECK(ev_bus.unsubscribe(invalid_subscription) != std::nullopt); - CHECK(ev_bus.unsubscribe_all("invalid_id") != std::nullopt); + CHECK(ev_bus.unsubscribe(invalid_subscription).has_error()); + CHECK(ev_bus.unsubscribe_all("invalid_id").has_error()); { auto handle = ev_bus.subscribe("test", [&](const message msg) {}); @@ -62,19 +62,19 @@ TEST_CASE("event_bus: subscribe/unsubscribe", "[noir][consensus][events]") { } SECTION("unsubscribe") { for (size_t i = 9; i > 0; --i) { - CHECK(ev_bus.unsubscribe(handles[i]) == std::nullopt); + CHECK(!ev_bus.unsubscribe(handles[i]).has_error()); CHECK(ev_bus.has_subscribers() == true); CHECK(ev_bus.num_clients() == 1); CHECK(ev_bus.num_client_subscription("test") == i); } // unsubscribe last element - CHECK(ev_bus.unsubscribe(handles[0]) == std::nullopt); + CHECK(!ev_bus.unsubscribe(handles[0]).has_error()); CHECK(ev_bus.has_subscribers() == false); CHECK(ev_bus.num_clients() == 0); CHECK(ev_bus.num_client_subscription("test") == 0); } SECTION("unsubscribe_all") { - CHECK(ev_bus.unsubscribe_all("test") == std::nullopt); + CHECK(!ev_bus.unsubscribe_all("test").has_error()); CHECK(ev_bus.has_subscribers() == false); CHECK(ev_bus.num_clients() == 0); CHECK(ev_bus.num_client_subscription("test") == 0); @@ -93,13 +93,13 @@ TEST_CASE("event_bus: subscribe/unsubscribe", "[noir][consensus][events]") { SECTION("unsubscribe") { for (size_t i = 9; i > 0; --i) { auto subscriber = "test" + std::to_string(i); - CHECK(ev_bus.unsubscribe(handles[i]) == std::nullopt); + CHECK(!ev_bus.unsubscribe(handles[i]).has_error()); CHECK(ev_bus.has_subscribers() == true); CHECK(ev_bus.num_clients() == i); CHECK(ev_bus.num_client_subscription(subscriber) == 0); } // unsubscribe last element - CHECK(ev_bus.unsubscribe(handles[0]) == std::nullopt); + CHECK(!ev_bus.unsubscribe(handles[0]).has_error()); CHECK(ev_bus.has_subscribers() == false); CHECK(ev_bus.num_clients() == 0); CHECK(ev_bus.num_client_subscription("test0") == 0); @@ -107,13 +107,13 @@ TEST_CASE("event_bus: subscribe/unsubscribe", "[noir][consensus][events]") { SECTION("unsubscribe_all") { for (size_t i = 9; i > 0; --i) { auto subscriber = "test" + std::to_string(i); - CHECK(ev_bus.unsubscribe_all(subscriber) == std::nullopt); + CHECK(!ev_bus.unsubscribe_all(subscriber).has_error()); CHECK(ev_bus.has_subscribers() == true); CHECK(ev_bus.num_clients() == i); CHECK(ev_bus.num_client_subscription(subscriber) == 0); } // unsubscribe last subscriber - CHECK(ev_bus.unsubscribe_all("test0") == std::nullopt); + CHECK(!ev_bus.unsubscribe_all("test0").has_error()); CHECK(ev_bus.has_subscribers() == false); CHECK(ev_bus.num_clients() == 0); CHECK(ev_bus.num_client_subscription("test0") == 0); diff --git a/src/noir/consensus/types/test/vote_test.cpp b/src/noir/consensus/types/test/vote_test.cpp index 97e6e2c7..4353727a 100644 --- a/src/noir/consensus/types/test/vote_test.cpp +++ b/src/noir/consensus/types/test/vote_test.cpp @@ -71,7 +71,7 @@ TEST_CASE("vote: verify signature", "[noir][consensus]") { CHECK(bz_sign_bytes.size() == 125); // Sign - CHECK(!val.sign_vote("test_chain_id", vote_).has_value()); + CHECK(!val.sign_vote("test_chain_id", vote_).has_error()); // Verify CHECK(val.get_pub_key().verify_signature(bz_sign_bytes, vote_.signature)); diff --git a/src/noir/consensus/types/validation.cpp b/src/noir/consensus/types/validation.cpp index e86a82d8..30185b80 100644 --- a/src/noir/consensus/types/validation.cpp +++ b/src/noir/consensus/types/validation.cpp @@ -11,25 +11,25 @@ namespace noir::consensus { -std::optional verify_basic_vals_and_commit(const std::shared_ptr& vals, +noir::Result verify_basic_vals_and_commit(const std::shared_ptr& vals, std::shared_ptr commit_, int64_t height, p2p::block_id block_id_) { if (!vals) - return "verification failed: validator_set is not set"; + return noir::Error("verification failed: validator_set is not set"); if (!commit_) - return "verification failed: commit is not set"; + return noir::Error("verification failed: commit is not set"); if (vals->size() != commit_->signatures.size()) - return "verification failed: not enough signatures prepared"; + return noir::Error("verification failed: not enough signatures prepared"); if (height != commit_->height) - return "verification failed: incorrect height"; + return noir::Error("verification failed: incorrect height"); if (block_id_ != commit_->my_block_id) - return "verification failed: wrong block_id"; - return {}; + return noir::Error("verification failed: wrong block_id"); + return success(); } /// \brief check all signatures included in a commit -std::optional verify_commit_single(const std::string& chain_id_, +noir::Result verify_commit_single(const std::string& chain_id_, const std::shared_ptr& vals, const std::shared_ptr& commit_, int64_t voting_power_needed, @@ -59,7 +59,7 @@ std::optional verify_commit_single(const std::string& chain_id_, // Check if same validator committed twice if (auto it = seen_vals.find(val_index); it != seen_vals.end()) { auto second_index = i; - return "verification failed: double vote detected"; + return noir::Error("verification failed: double vote detected"); } seen_vals[val_index] = i; } @@ -67,28 +67,28 @@ std::optional verify_commit_single(const std::string& chain_id_, auto vote_ = commit_->get_vote(i); vote_sign_bytes = vote::vote_sign_bytes(chain_id_, *vote::to_proto(*vote_)); if (!val.pub_key_.verify_signature(vote_sign_bytes, commit_sig_.signature)) - return fmt::format("verification failed: wrong signature - index={}", i); + return noir::Error::format("verification failed: wrong signature - index={}", i); tallied_voting_power += val.voting_power; if (!count_all_signatures && tallied_voting_power > voting_power_needed) - return {}; + return success(); } if (tallied_voting_power <= voting_power_needed) - return "verification failed: not enough votes were signed"; - return {}; + return noir::Error("verification failed: not enough votes were signed"); + return success(); } /// \brief verifies +2/3 of set has signed given commit /// Used by the light client and does not check all signatures -std::optional verify_commit_light(const std::string& chain_id_, +noir::Result verify_commit_light(const std::string& chain_id_, const std::shared_ptr& vals, const p2p::block_id& block_id_, int64_t height, const std::shared_ptr& commit_) { // Validate params - if (auto err = verify_basic_vals_and_commit(vals, commit_, height, block_id_); err.has_value()) - return err; + if (auto ok = verify_basic_vals_and_commit(vals, commit_, height, block_id_); !ok) + return ok.error(); // Calculate required voting power auto voting_power_needed = vals->total_voting_power * 2 / 3; diff --git a/src/noir/consensus/types/validation.h b/src/noir/consensus/types/validation.h index eeaeadae..a23d75ca 100644 --- a/src/noir/consensus/types/validation.h +++ b/src/noir/consensus/types/validation.h @@ -8,13 +8,13 @@ namespace noir::consensus { -std::optional verify_basic_vals_and_commit(const std::shared_ptr& vals, +noir::Result verify_basic_vals_and_commit(const std::shared_ptr& vals, std::shared_ptr commit_, int64_t height, p2p::block_id block_id_); /// \brief check all signatures included in a commit -std::optional verify_commit_single(const std::string& chain_id_, +noir::Result verify_commit_single(const std::string& chain_id_, const std::shared_ptr& vals, const std::shared_ptr& commit_, int64_t voting_power_needed, @@ -23,7 +23,7 @@ std::optional verify_commit_single(const std::string& chain_id_, /// \brief verifies +2/3 of set has signed given commit /// Used by the light client and does not check all signatures -std::optional verify_commit_light(const std::string& chain_id_, +noir::Result verify_commit_light(const std::string& chain_id_, const std::shared_ptr& vals, const p2p::block_id& block_id_, int64_t height, diff --git a/src/noir/consensus/types/validator.cpp b/src/noir/consensus/types/validator.cpp index f844edc1..56cecbac 100644 --- a/src/noir/consensus/types/validator.cpp +++ b/src/noir/consensus/types/validator.cpp @@ -32,9 +32,8 @@ Bytes validator_set::get_hash() { Result validator_set::verify_commit_light( const std::string& chain_id_, p2p::block_id block_id_, int64_t height, const std::shared_ptr& commit_) { auto vals = std::make_shared(*this); - auto err = noir::consensus::verify_commit_light(chain_id_, vals, block_id_, height, commit_); - if (err.has_value()) - return Error::format("{}", err.value()); + if (auto ok = noir::consensus::verify_commit_light(chain_id_, vals, block_id_, height, commit_); !ok) + return ok.error(); return success(); } diff --git a/src/noir/consensus/types/vote.h b/src/noir/consensus/types/vote.h index fc6528e0..33983cae 100644 --- a/src/noir/consensus/types/vote.h +++ b/src/noir/consensus/types/vote.h @@ -6,6 +6,7 @@ #pragma once #include #include +#include #include #include @@ -191,7 +192,7 @@ struct vote_set { return {}; } - std::optional set_peer_maj23(std::string peer_id, p2p::block_id block_id_) { + noir::Result set_peer_maj23(std::string peer_id, p2p::block_id block_id_) { std::scoped_lock g(mtx); auto block_key = block_id_.key(); @@ -199,8 +200,8 @@ struct vote_set { // Make sure peer has not sent us something yet if (auto it = peer_maj23s.find(peer_id); it != peer_maj23s.end()) { if (it->second == block_id_) - return {}; // nothing to do - return "setPeerMaj23: Received conflicting blockID"; + return success(); // nothing to do + return noir::Error("setPeerMaj23: Received conflicting blockID"); } peer_maj23s[peer_id] = block_id_; @@ -208,13 +209,13 @@ struct vote_set { auto it = votes_by_block.find(block_key); if (it != votes_by_block.end()) { if (it->second->peer_maj23) - return {}; // nothing to do + return success(); // nothing to do it->second->peer_maj23 = true; } else { auto new_votes_by_block = block_votes::new_block_votes(true, val_set->size()); votes_by_block[block_key] = new_votes_by_block; } - return {}; + return success(); } bool has_two_thirds_majority() { diff --git a/src/noir/p2p/conn/secret_connection.cpp b/src/noir/p2p/conn/secret_connection.cpp index d36d6345..1d897386 100644 --- a/src/noir/p2p/conn/secret_connection.cpp +++ b/src/noir/p2p/conn/secret_connection.cpp @@ -31,7 +31,7 @@ std::shared_ptr secret_connection::make_secret_connection(Byt return sc; } -std::optional secret_connection::shared_eph_pub_key(Bytes32& received_pub_key) { +noir::Result secret_connection::shared_eph_pub_key(Bytes32& received_pub_key) { // By here, we have already exchanged eph_pub_keys with the other rem_eph_pub = received_pub_key; @@ -49,13 +49,13 @@ std::optional secret_connection::shared_eph_pub_key(Bytes32& receiv if (crypto_scalarmult(reinterpret_cast(dh_secret.data()), reinterpret_cast(loc_eph_priv.data()), reinterpret_cast(rem_eph_pub.data())) != 0) { - return "unable to compute dh_secret"; + return noir::Error("unable to compute dh_secret"); } // Generate secret keys used for receiving, sending, challenging via HKDF-SHA2 auto key = derive_secrets(dh_secret); if (key.size() < 96) - return "unable to derive secrets"; + return noir::Error("unable to derive secrets"); if (loc_is_least) { recv_secret = Bytes32(std::span(key.data(), 32)); @@ -85,12 +85,12 @@ std::optional secret_connection::shared_eph_pub_key(Bytes32& receiv reinterpret_cast(chal_secret.data()), chal_secret.size(), reinterpret_cast(loc_priv_key.data())) == 0) { loc_signature = sig; - return {}; + return success(); } - return "unable to sign challenge"; + return noir::Error("unable to sign challenge"); } -std::optional secret_connection::shared_auth_sig(auth_sig_message& received_msg) { +noir::Result secret_connection::shared_auth_sig(auth_sig_message& received_msg) { // By here, we have already exchanged auth_sig_message with the other rem_pub_key = received_msg.key; @@ -99,9 +99,9 @@ std::optional secret_connection::shared_auth_sig(auth_sig_message& reinterpret_cast(chal_secret.data()), chal_secret.size(), reinterpret_cast(rem_pub_key.data())) == 0) { is_authorized = true; - return {}; + return success(); } - return "unable to verify challenge"; + return noir::Error("unable to verify challenge"); } Bytes secret_connection::derive_secrets(Bytes32& dh_secret) { diff --git a/src/noir/p2p/conn/secret_connection.h b/src/noir/p2p/conn/secret_connection.h index cbfbf534..eb1003e6 100644 --- a/src/noir/p2p/conn/secret_connection.h +++ b/src/noir/p2p/conn/secret_connection.h @@ -80,9 +80,9 @@ struct secret_connection { static std::shared_ptr make_secret_connection(Bytes& loc_priv_key); - std::optional shared_eph_pub_key(Bytes32& received_pub_key); + noir::Result shared_eph_pub_key(Bytes32& received_pub_key); - std::optional shared_auth_sig(auth_sig_message& received_msg); + noir::Result shared_auth_sig(auth_sig_message& received_msg); Bytes derive_secrets(Bytes32& dh_secret); diff --git a/src/noir/p2p/conn/test/connection_test.cpp b/src/noir/p2p/conn/test/connection_test.cpp index 3f0d99f1..9229fad8 100644 --- a/src/noir/p2p/conn/test/connection_test.cpp +++ b/src/noir/p2p/conn/test/connection_test.cpp @@ -68,8 +68,8 @@ TEST_CASE("secret_connection: verify key exchanges", "[noir][p2p]") { auto auth_sig_msg1 = p2p::auth_sig_message{c_peer1->loc_pub_key, c_peer1->loc_signature}; auto auth_sig_msg2 = p2p::auth_sig_message{c_peer2->loc_pub_key, c_peer2->loc_signature}; - CHECK(!c_peer1->shared_auth_sig(auth_sig_msg2).has_value()); - CHECK(!c_peer2->shared_auth_sig(auth_sig_msg1).has_value()); + CHECK(!c_peer1->shared_auth_sig(auth_sig_msg2).has_error()); + CHECK(!c_peer2->shared_auth_sig(auth_sig_msg1).has_error()); } std::string crypto_box_recover_public_key(uint8_t secret_key[]) {