diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index 42656c3bc2..4cc4cce6cb 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -91,6 +91,7 @@ add_library( sysio_chain resource_limits.cpp block_log.cpp block_root_processor.cpp + s_root_extension.cpp contract_action_match.cpp root_txn_identification.cpp transaction_context.cpp diff --git a/libraries/chain/block_header.cpp b/libraries/chain/block_header.cpp index 010cb9827d..12483fe65b 100644 --- a/libraries/chain/block_header.cpp +++ b/libraries/chain/block_header.cpp @@ -21,7 +21,10 @@ namespace sysio { namespace chain { // Do not include signed_block_header attributes in id, specifically exclude producer_signature. block_id_type result = digest(); //fc::sha256::hash(*static_cast(this)); result._hash[0] &= 0xffffffff00000000; - result._hash[0] += fc::endian_reverse_u32(block_num()); // store the block num in the ID, 160 bits is plenty for the hash + // Store the block num in the low 32 bits. Use |= rather than += to make it + // explicit that this is bit-insertion into pre-cleared bits — no carry into + // the hash bytes is possible regardless of future mask changes. + result._hash[0] |= fc::endian_reverse_u32(block_num()); return result; } diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 990ea1a183..79f3e213a7 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -184,39 +184,102 @@ block_state::block_state(snapshot_detail::snapshot_block_state_v1&& sbs) .last_pending_finalizer_policy_digest = sbs.last_pending_finalizer_policy_digest, .last_pending_finalizer_policy_start_timestamp = sbs.last_pending_finalizer_policy_start_timestamp } - , strong_digest(compute_finality_digest()) - , weak_digest(create_weak_digest(strong_digest)) - , aggregating_qc(active_finalizer_policy, - pending_finalizer_policy ? pending_finalizer_policy->second : finalizer_policy_ptr{}) // just in case we receive votes + // strong_digest / weak_digest / aggregating_qc are value-initialized here and + // assigned their real values in the body after null-guards. compute_finality_digest() + // and aggregating_qc_sig_t's ctor both dereference active_finalizer_policy; a + // tampered snapshot with null pointers would crash inside the member init list + // before we could SYS_ASSERT on them. Explicit {} forces value-init (matters for + // weak_digest_t = std::array, whose default-init leaves bytes + // indeterminate). + , strong_digest{} + , weak_digest{} + , aggregating_qc{} , valid(std::move(sbs.valid)) { - header_exts = header.validate_and_extract_header_extensions(); + // fc::raw::unpack> permits null via a leading bool; a tampered + // snapshot can plant nulls in any of these slots. Null-check BEFORE any code + // dereferences them (compute_finality_digest / aggregating_qc construction / + // policy validate() calls all require non-null pointers). activated_protocol_features + // is reached through compute_base_digest() from compute_finality_digest() below. + SYS_ASSERT(activated_protocol_features, snapshot_exception, "activated_protocol_features must not be null"); + SYS_ASSERT(active_finalizer_policy, snapshot_exception, "active_finalizer_policy must not be null"); + SYS_ASSERT(active_proposer_policy, snapshot_exception, "active_proposer_policy must not be null"); + if (pending_finalizer_policy) { + SYS_ASSERT(pending_finalizer_policy->second, snapshot_exception, + "pending_finalizer_policy must not contain a null policy"); + } + for (const auto& [_, pol] : proposed_finalizer_policies) { + SYS_ASSERT(pol, snapshot_exception, "proposed_finalizer_policies entry must not be null"); + } - // Snapshot hardening: validate finality_core invariants + // Snapshot hardening: validate finality_core invariants BEFORE computing digests. + // compute_finality_digest() calls core.latest_qc_claim() (asserts !links.empty()) and + // core.get_block_reference() (asserts bounds on refs); an empty or malformed core + // would assert/segfault inside the digest path before we could throw snapshot_exception. core.validate_snapshot(); - // Snapshot hardening: validate finalizer policies - SYS_ASSERT(active_finalizer_policy, snapshot_exception, "active_finalizer_policy must not be null"); - SYS_ASSERT(active_proposer_policy, snapshot_exception, "active_proposer_policy must not be null"); - active_finalizer_policy->validate_snapshot(); + // Now safe to compute state that depends on active_finalizer_policy / activated_protocol_features / core. + strong_digest = compute_finality_digest(); + weak_digest = create_weak_digest(strong_digest); + aggregating_qc = aggregating_qc_t{ + active_finalizer_policy, + pending_finalizer_policy ? pending_finalizer_policy->second : finalizer_policy_ptr{} + }; + + header_exts = header.validate_and_extract_header_extensions(); + + // core.latest_qc_claim() is the authoritative reference for downstream QC-claim checks + // (see verify_basic_proper_block_invariants). For every non-genesis block it equals + // header.qc_claim by construction (finality_core::next sets latest_qc_claim from the + // header's claim). Genesis intentionally differs: core is {1, false}, header is {0, false}. + SYS_ASSERT(core.is_genesis_core() || core.latest_qc_claim() == header.qc_claim, snapshot_exception, + "snapshot block_state core.latest_qc_claim {} does not match header.qc_claim {}", + core.latest_qc_claim(), header.qc_claim); + + // Snapshot hardening: validate finalizer and proposer policies. + // Uses the shared validate() methods (same checks the set_finalizers / + // set_proposed_producers intrinsics enforce); rewraps as snapshot_exception. + auto validate_fin_pol = [](const finalizer_policy& p, const char* which) { + try { p.validate(); } + catch (const invalid_finalizer_policy_exception& e) { + SYS_THROW(snapshot_exception, "snapshot: {} finalizer policy invalid: {}", which, e.top_message()); + } + }; + auto validate_prop_pol = [](const proposer_policy& p, const char* which) { + try { p.validate(); } + catch (const producer_schedule_exception& e) { + SYS_THROW(snapshot_exception, "snapshot: {} proposer policy invalid: {}", which, e.top_message()); + } + }; + validate_fin_pol(*active_finalizer_policy, "active"); if (pending_finalizer_policy) { - pending_finalizer_policy->second->validate_snapshot(); + validate_fin_pol(*pending_finalizer_policy->second, "pending"); } for (const auto& [_, pol] : proposed_finalizer_policies) { - pol->validate_snapshot(); + validate_fin_pol(*pol, "proposed"); } if (latest_qc_claim_block_active_finalizer_policy) { - latest_qc_claim_block_active_finalizer_policy->validate_snapshot(); + validate_fin_pol(*latest_qc_claim_block_active_finalizer_policy, "latest_qc_claim_block_active"); } - - // Snapshot hardening: validate valid_t - if (valid) { - valid->validation_tree.validate_snapshot(); - auto expected_size = core.current_block_num() - core.last_final_block_num() + 1; - SYS_ASSERT(valid->validation_mroots.size() == expected_size, snapshot_exception, - "valid_t.validation_mroots size ({}) != expected ({})", - valid->validation_mroots.size(), expected_size); + validate_prop_pol(*active_proposer_policy, "active"); + if (latest_proposed_proposer_policy) { + validate_prop_pol(*latest_proposed_proposer_policy, "latest_proposed"); } + if (latest_pending_proposer_policy) { + validate_prop_pol(*latest_pending_proposer_policy, "latest_pending"); + } + + // Snapshot hardening: validate valid_t. A snapshot is never taken at genesis, so + // every snapshot block_state must carry a valid_t (finality-tree) structure. A null + // valid is a tampered or truncated snapshot; without this check get_validation_mroot() + // would later return an empty digest (after a debug-only assert) and the corruption + // would surface only indirectly as a finality_mroot mismatch in apply_block. + SYS_ASSERT(valid, snapshot_exception, "snapshot block_state must have a valid_t finality structure"); + valid->validation_tree.validate_snapshot(); + auto expected_size = core.current_block_num() - core.last_final_block_num() + 1; + SYS_ASSERT(valid->validation_mroots.size() == expected_size, snapshot_exception, + "valid_t.validation_mroots size ({}) != expected ({})", + valid->validation_mroots.size(), expected_size); } deque block_state::extract_trxs_metas() { @@ -302,6 +365,12 @@ valid_t block_state::new_valid(const block_header_state& next_bhs, const digest_ digest_type block_state::get_validation_mroot(block_num_type target_block_num) const { if (!valid) { + // The only legitimate case where a block_state has no valid structure is + // the Savanna genesis core (created without a finality tree). Any other + // null-valid is a construction or serialization bug; catching it here + // produces a clearer failure than the downstream finality_mroot mismatch + // apply_block would report. + assert(core.is_genesis_core()); return digest_type{}; } diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 73e77080a2..1f1c8cf714 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2459,6 +2459,13 @@ struct controller_impl { { const auto& pfs = protocol_features.get_protocol_feature_set(); + // Track digests seen earlier in this activation list so duplicates fail here rather than + // later in start_block (which only catches duplicates of preactivated features). A duplicate + // that wasn't preactivated would otherwise re-run trigger_activation_handler and increment + // num_new_protocol_features_activated twice, diverging side effects from a legitimate block. + flat_set seen_in_this_activation; + seen_in_this_activation.reserve( new_protocol_features.size() ); + for( auto itr = new_protocol_features.begin(); itr != new_protocol_features.end(); ++itr ) { const auto& f = *itr; @@ -2489,6 +2496,10 @@ struct controller_impl { "protocol feature with digest '{}' has already been activated", f ); + SYS_ASSERT( seen_in_this_activation.insert( f ).second, protocol_feature_exception, + "protocol feature with digest '{}' appears more than once in the activation list", f + ); + auto dependency_checker = [¤tly_activated_protocol_features, &new_protocol_features, &itr] ( const digest_type& f ) -> bool { @@ -2667,43 +2678,7 @@ struct controller_impl { } if( are_multiple_state_roots_supported() ) { - // New: Process the s_header from block header extensions - auto rcvd_it = b->header_extensions.begin(); - const auto next_rcvd = [&itr=rcvd_it, end=b->header_extensions.end()](bool include_start) { - if( !include_start ) ++itr; - // find the first s_root_extension in the received block header extensions - return std::find_if(itr, end, - [](const auto& ext) { return ext.first == s_root_extension::extension_id(); }); }; - auto crtd_it = ab.header().header_extensions.begin(); - const auto next_crtd = [&itr=crtd_it, end=ab.header().header_extensions.end()](bool include_start) { - if( !include_start ) ++itr; - // find the first s_root_extension in the locally constructed block header extensions - // (which should be the same as the received block header extensions) - return std::find_if(itr, end, - [](const auto& ext) { return ext.first == s_root_extension::extension_id(); }); }; - uint32_t count = 0; - bool rcvd = true; - bool crtd = true; - bool include_start = true; - while( rcvd ) { - rcvd_it = next_rcvd(include_start); - crtd_it = next_crtd(include_start); - ++count; - include_start = false; // only include start for the first iteration - rcvd = rcvd_it != b->header_extensions.end(); - crtd = crtd_it != ab.header().header_extensions.end(); - SYS_ASSERT( rcvd == crtd, block_validate_exception, - "The received block did{} have {} root header extensions, but the locally constructed one did{}", - (rcvd ? "" : " not"), count, (crtd ? "" : " not") ); - if( rcvd && rcvd_it->second != crtd_it->second ) { - s_header rcvd_s_header = fc::raw::unpack(rcvd_it->second); - s_header crtd_s_header = fc::raw::unpack(crtd_it->second); - SYS_THROW( block_validate_exception, - "The received block root header extension, at slot number {}: {}; and the locally " - "constructed one: {}; don't match!", - count, rcvd_s_header.to_string(), crtd_s_header.to_string() ); - } - } + validate_s_root_extensions_match(b->header_extensions, ab.header().header_extensions); } if( !use_bsp_cached ) { @@ -2798,9 +2773,6 @@ struct controller_impl { // ----------------------------------------------------------------------------- std::optional verify_basic_proper_block_invariants( const block_id_type& id, const signed_block_ptr& b, const block_state& prev ) { - if (prev.block_num() <= 1u) - return std::nullopt; - SYS_ASSERT( b->block_extensions.empty(), invalid_block_extension, "No block extensions currently supported"); const auto new_qc_claim = b->qc_claim; @@ -2816,7 +2788,12 @@ struct controller_impl { } } - const auto& prev_qc_claim = prev.header.qc_claim; + // Use prev.core.latest_qc_claim() rather than prev.header.qc_claim: for every non-genesis + // block they are equal by construction (finality_core::next sets latest_qc_claim from the + // header's claim), but genesis intentionally differs (core is {1, false}, header is {0, false}). + // The core is the authoritative source, so block 2 can validate cleanly with this reference. + // Snapshot-loaded block_state enforces this invariant in block_state(snapshot). + const qc_claim_t prev_qc_claim = prev.core.latest_qc_claim(); // validate QC claim against previous block QC info @@ -3261,7 +3238,7 @@ struct controller_impl { static checksum256_type calculate_trx_merkle( const deque& trxs) { deque trx_digests; for( const auto& a : trxs ) { - trx_digests.emplace_back( (a.digest)() ); + trx_digests.emplace_back( a.digest() ); } return calc_merkle(std::move(trx_digests)); diff --git a/libraries/chain/finality_core.cpp b/libraries/chain/finality_core.cpp index 1492abe957..284819a112 100644 --- a/libraries/chain/finality_core.cpp +++ b/libraries/chain/finality_core.cpp @@ -59,11 +59,15 @@ void finality_core::validate_snapshot() const { } // Invariant 8 (implied by 3-6): current_block_num - last_final_block_num == refs.size() + // Subtraction is safe: invariant 2 (checked above) established lfbn <= links.front().source_block_num, + // and by invariant 7's monotonicity links.front().source_block_num <= links.back().source_block_num == cur, + // so lfbn <= cur and the unsigned subtraction cannot underflow. SYS_ASSERT(cur - lfbn == refs.size(), snapshot_exception, "finality_core: current_block_num ({}) - last_final_block_num ({}) != refs.size() ({})", cur, lfbn, refs.size()); // Invariant 9 (implied by 1,7): current_block_num - links.front().source_block_num == links.size() - 1 + // Subtraction is safe by invariant 7's monotonicity (links.front().source_block_num <= cur). SYS_ASSERT(cur - links.front().source_block_num == links.size() - 1, snapshot_exception, "finality_core: link count mismatch: current_block_num ({}) - front source ({}) != links.size()-1 ({})", cur, links.front().source_block_num, links.size() - 1); diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 853b4b2b1b..920e6e52e5 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -26,12 +27,10 @@ namespace sysio::chain { */ std::string log_fork_comparison(const block_state& bs) { - std::string r; - r += "[ latest_qc_block_timestamp: " + bs.latest_qc_block_timestamp().to_time_point().to_iso_string() + ", "; - r += "timestamp: " + bs.timestamp().to_time_point().to_iso_string() + ", "; - r += "id: " + bs.id().str(); - r += " ]"; - return r; + return fmt::format("[ latest_qc_block_timestamp: {}, timestamp: {}, id: {} ]", + bs.latest_qc_block_timestamp().to_time_point().to_iso_string(), + bs.timestamp().to_time_point().to_iso_string(), + bs.id().str()); } struct by_block_id; diff --git a/libraries/chain/include/sysio/chain/exceptions.hpp b/libraries/chain/include/sysio/chain/exceptions.hpp index a01976af33..25fb05dbc2 100644 --- a/libraries/chain/include/sysio/chain/exceptions.hpp +++ b/libraries/chain/include/sysio/chain/exceptions.hpp @@ -669,6 +669,8 @@ namespace sysio { namespace chain { 3260000, "Finalizer exception" ) FC_DECLARE_DERIVED_EXCEPTION( finalizer_safety_exception, finalizer_exception, 3260001, "Finalizer safety file exception" ) + FC_DECLARE_DERIVED_EXCEPTION( invalid_finalizer_policy_exception, finalizer_exception, + 3260002, "Invalid finalizer policy" ) /// @return true if exception requires transaction to be retried in the next block inline bool exception_is_exhausted(const fc::exception& e) { diff --git a/libraries/chain/include/sysio/chain/finalizer_policy.hpp b/libraries/chain/include/sysio/chain/finalizer_policy.hpp index f38a3f619e..5d516c8f6f 100644 --- a/libraries/chain/include/sysio/chain/finalizer_policy.hpp +++ b/libraries/chain/include/sysio/chain/finalizer_policy.hpp @@ -1,9 +1,11 @@ #pragma once #include +#include #include #include #include +#include namespace sysio::chain { @@ -41,28 +43,41 @@ namespace sysio::chain { return result; } - // Validate threshold > 0 and threshold <= sum(weights). Throws snapshot_exception on violation. - void validate_snapshot() const { - SYS_ASSERT(threshold > 0, snapshot_exception, - "finalizer_policy generation {}: threshold must be positive", generation); - SYS_ASSERT(!finalizers.empty(), snapshot_exception, + // Validates structural well-formedness of the policy. Single source of truth + // reused by the set_finalizers host function and snapshot loading; each caller + // wraps the thrown invalid_finalizer_policy_exception with its own context. + // Throws invalid_finalizer_policy_exception on violation. + void validate() const { + SYS_ASSERT(!finalizers.empty(), invalid_finalizer_policy_exception, "finalizer_policy generation {}: finalizers must not be empty", generation); + SYS_ASSERT(finalizers.size() <= config::max_finalizers, invalid_finalizer_policy_exception, + "finalizer_policy generation {}: finalizer count ({}) exceeds max ({})", + generation, finalizers.size(), config::max_finalizers); + boost::container::flat_set unique_keys; + unique_keys.reserve(finalizers.size()); uint64_t sum = 0; for (const auto& f : finalizers) { - SYS_ASSERT(f.weight > 0, snapshot_exception, + SYS_ASSERT(f.description.size() <= config::max_finalizer_description_size, invalid_finalizer_policy_exception, + "finalizer_policy generation {}: description size ({}) exceeds max ({})", + generation, f.description.size(), config::max_finalizer_description_size); + SYS_ASSERT(f.weight > 0, invalid_finalizer_policy_exception, "finalizer_policy generation {}: finalizer weight must be positive", generation); - SYS_ASSERT(sum <= std::numeric_limits::max() - f.weight, snapshot_exception, + SYS_ASSERT(sum <= std::numeric_limits::max() - f.weight, invalid_finalizer_policy_exception, "finalizer_policy generation {}: sum of weights overflows uint64_t", generation); sum += f.weight; + SYS_ASSERT(unique_keys.insert(f.public_key).second, invalid_finalizer_policy_exception, + "finalizer_policy generation {}: duplicate public_key", generation); } - SYS_ASSERT(threshold <= sum, snapshot_exception, - "finalizer_policy generation {}: sum of weights ({}) didn't meet the required threshold ({})", - generation, sum, threshold); + // BFT safety bound: threshold strictly greater than half the sum of weights. + // Expressed as threshold > sum/2 (rather than 2*threshold > sum) to avoid overflow. + SYS_ASSERT(threshold > sum / 2 && threshold <= sum, invalid_finalizer_policy_exception, + "finalizer_policy generation {}: threshold ({}) must be > sum/2 ({}) and <= sum ({})", + generation, threshold, sum / 2, sum); } // max accumulated weak weight before becoming weak_final uint64_t max_weak_sum_before_weak_final() const { - uint64_t sum = std::accumulate( finalizers.begin(), finalizers.end(), 0, + uint64_t sum = std::accumulate( finalizers.begin(), finalizers.end(), uint64_t{0}, [](uint64_t acc, const finalizer_authority& f) { return acc + f.weight; } diff --git a/libraries/chain/include/sysio/chain/proposer_policy.hpp b/libraries/chain/include/sysio/chain/proposer_policy.hpp index 6ec1015d1c..2a4a9611e8 100644 --- a/libraries/chain/include/sysio/chain/proposer_policy.hpp +++ b/libraries/chain/include/sysio/chain/proposer_policy.hpp @@ -1,7 +1,11 @@ #pragma once #include +#include +#include #include +#include +#include namespace sysio::chain { @@ -39,6 +43,48 @@ struct proposer_policy { std::forward(diff).producer_auth_diff); return result; } + + // Validates structural well-formedness of the policy. Single source of truth + // reused by the set_proposed_producers host function and snapshot loading. + // Two things are intentionally NOT checked here and stay at the intrinsic + // call site instead: + // - account existence (requires apply_context) + // - K1/R1 key type enforcement (uses unactivated_key_type to signal that + // non-K1/R1 keys need a protocol feature; distinct from structural errors) + // Throws producer_schedule_exception on violation. + void validate() const { + const auto& producers = proposer_schedule.producers; + SYS_ASSERT(!producers.empty(), producer_schedule_exception, + "producer schedule must not be empty"); + SYS_ASSERT(producers.size() <= config::max_producers, producer_schedule_exception, + "producer count ({}) exceeds max ({})", + producers.size(), config::max_producers); + boost::container::flat_set unique_producers; + unique_producers.reserve(producers.size()); + for (const auto& p : producers) { + SYS_ASSERT(unique_producers.insert(p.producer_name).second, producer_schedule_exception, + "duplicate producer name {}", p.producer_name); + std::visit([&](const auto& a) { + SYS_ASSERT(a.threshold > 0, producer_schedule_exception, + "producer {} authority threshold must be positive", p.producer_name); + boost::container::flat_set unique_keys; + unique_keys.reserve(a.keys.size()); + uint32_t sum_weights = 0; + for (const auto& kw : a.keys) { + SYS_ASSERT(unique_keys.insert(kw.key).second, producer_schedule_exception, + "producer {} authority has duplicate key", p.producer_name); + if (std::numeric_limits::max() - sum_weights <= kw.weight) { + sum_weights = std::numeric_limits::max(); + } else { + sum_weights += kw.weight; + } + } + SYS_ASSERT(sum_weights >= a.threshold, producer_schedule_exception, + "producer {} authority threshold ({}) not satisfiable by sum of key weights ({})", + p.producer_name, a.threshold, sum_weights); + }, p.authority); + } + } }; using proposer_policy_ptr = std::shared_ptr; diff --git a/libraries/chain/include/sysio/chain/s_root_extension.hpp b/libraries/chain/include/sysio/chain/s_root_extension.hpp index 5756e6af90..c638b5d4c2 100644 --- a/libraries/chain/include/sysio/chain/s_root_extension.hpp +++ b/libraries/chain/include/sysio/chain/s_root_extension.hpp @@ -59,6 +59,19 @@ struct s_root_extension { } }; +// Walks the s_root_extension entries in `received` and `constructed` in order, +// asserting that there are the same number of them and that each corresponding +// pair has identical packed bytes. Other extension types (e.g. protocol_feature_activation) +// are ignored — only entries with extension_id() == s_root_extension::extension_id() +// are compared. Throws block_validate_exception on any mismatch. +// +// Precondition: both inputs must already be in the canonical ascending-id order +// that block_header::validate_and_extract_header_extensions enforces. Same-id +// entries preserve insertion order, which is the order s_root_extensions were +// added by block_header_state::next when building the block. +void validate_s_root_extensions_match(const extensions_type& received, + const extensions_type& constructed); + }} // namespace sysio::chain FC_REFLECT(sysio::chain::s_header, diff --git a/libraries/chain/qc.cpp b/libraries/chain/qc.cpp index cdb5d102d6..19931c64b2 100644 --- a/libraries/chain/qc.cpp +++ b/libraries/chain/qc.cpp @@ -54,21 +54,31 @@ void qc_t::verify_basic(const finalizer_policies_t& policies) const { SYS_ASSERT(policies.pending_finalizer_policy, invalid_qc, "qc {} contains pending policy signature for nonexistent pending finalizer policy", block_num); - // verify that every finalizer included in both policies voted the same - verify_dual_finalizers_votes(policies, active_policy_sig, *pending_policy_sig, block_num); - pending_policy_sig->verify_vote_format(policies.pending_finalizer_policy); pending_policy_sig->verify_weights(policies.pending_finalizer_policy); + + // verify that every finalizer included in both policies voted the same + verify_dual_finalizers_votes(policies, active_policy_sig, *pending_policy_sig, block_num); } else { SYS_ASSERT(!policies.pending_finalizer_policy, invalid_qc, "qc {} does not contain pending policy signature for pending finalizer policy", block_num); } } -// returns true iff the other and I voted in the same way. +// Returns true iff the other and I voted in the same way. +// +// Precondition chain: both *this and `other` must have passed verify_vote_format() +// before reaching this function. verify_vote_format asserts each qc_sig_t has at +// least one of strong_votes/weak_votes and that each present bitset's size matches +// the corresponding policy's finalizer count. Callers (qc_t::verify_basic -> +// verify_dual_finalizers_votes) run verify_vote_format on both qc_sigs first, so +// the index-in-range asserts below are coding-error guards, not attacker-input +// guards. bool qc_sig_t::vote_same_at(const qc_sig_t& other, uint32_t my_vote_index, uint32_t other_vote_index) const { assert(!strong_votes || my_vote_index < strong_votes->size()); - assert(!weak_votes || my_vote_index < weak_votes->size()); + assert(!weak_votes || my_vote_index < weak_votes->size()); + assert(!other.strong_votes || other_vote_index < other.strong_votes->size()); + assert(!other.weak_votes || other_vote_index < other.weak_votes->size()); // We have already verified the same index has not voted both strong // and weak for a given qc_sig_t (I or other). @@ -454,9 +464,14 @@ aggregate_vote_result_t aggregating_qc_t::aggregate_vote(uint32_t connection_id, return r; } - // Check has_voted, return duplicate if the vote is already present in both policies. + // Fast-path dedup hint: lock-free atomic reads on has_voted filter out most duplicates + // before the expensive BLS verify. This is a performance optimization, NOT the + // authoritative dedup — two voters racing here may both pass and both do the BLS verify. + // The authoritative check is aggregating_qc_sig_t::check_duplicate() called under the + // outer lock from add_vote() below, which ensures no double-counting of weight. + // // For dual finalizers both policies must have the vote; for single-policy finalizers only - // the applicable policy is checked. This rejects duplicates before the expensive BLS verify. + // the applicable policy is checked. bool active_dup = active_index < 0 || active_policy_sig.has_voted(active_index); bool pending_dup = pending_index < 0 || pending_policy_sig->has_voted(pending_index); if (active_dup && pending_dup) { diff --git a/libraries/chain/s_root_extension.cpp b/libraries/chain/s_root_extension.cpp new file mode 100644 index 0000000000..080d648794 --- /dev/null +++ b/libraries/chain/s_root_extension.cpp @@ -0,0 +1,39 @@ +#include +#include +#include +#include + +namespace sysio { namespace chain { + +void validate_s_root_extensions_match(const extensions_type& received, + const extensions_type& constructed) { + const auto is_sre = [](const auto& e) { return e.first == s_root_extension::extension_id(); }; + + auto rcvd_it = std::find_if(received.begin(), received.end(), is_sre); + auto crtd_it = std::find_if(constructed.begin(), constructed.end(), is_sre); + uint32_t slot = 0; + + while (rcvd_it != received.end() || crtd_it != constructed.end()) { + ++slot; + const bool rcvd_has = rcvd_it != received.end(); + const bool crtd_has = crtd_it != constructed.end(); + + SYS_ASSERT(rcvd_has == crtd_has, block_validate_exception, + "s_root_extension count mismatch at slot {}: received did{} have an entry, " + "constructed did{}", + slot, rcvd_has ? "" : " not", crtd_has ? "" : " not"); + + if (rcvd_it->second != crtd_it->second) { + s_header rcvd_h = fc::raw::unpack(rcvd_it->second); + s_header crtd_h = fc::raw::unpack(crtd_it->second); + SYS_THROW(block_validate_exception, + "s_root_extension payload mismatch at slot {}: received {}; constructed {}", + slot, rcvd_h.to_string(), crtd_h.to_string()); + } + + rcvd_it = std::find_if(std::next(rcvd_it), received.end(), is_sre); + crtd_it = std::find_if(std::next(crtd_it), constructed.end(), is_sre); + } +} + +}} // namespace sysio::chain diff --git a/libraries/chain/webassembly/privileged.cpp b/libraries/chain/webassembly/privileged.cpp index 2829d8ed51..0d4ab75373 100644 --- a/libraries/chain/webassembly/privileged.cpp +++ b/libraries/chain/webassembly/privileged.cpp @@ -45,42 +45,40 @@ namespace sysio { namespace chain { namespace webassembly { } int64_t set_proposed_producers_common( apply_context& context, vector&& producers ) { - SYS_ASSERT(producers.size() <= config::max_producers, wasm_execution_error, "Producer schedule exceeds the maximum producer count for this chain"); - SYS_ASSERT( producers.size() > 0, wasm_execution_error, "Producer schedule cannot be empty" ); + // Structural validation (bounds, uniqueness, per-authority key/threshold sanity) + // lives on proposer_policy::validate() so the snapshot-load path and this + // intrinsic enforce identical invariants. Move the producers into a policy + // shell for validation and re-use the shell's vector for the subsequent + // per-producer checks and the final downstream call — avoids a copy. + proposer_policy candidate; + candidate.proposer_schedule.producers = std::move(producers); + try { + candidate.validate(); + } catch (const producer_schedule_exception& e) { + SYS_THROW(wasm_execution_error, "set_proposed_producers: {}", e.top_message()); + } + // Remaining checks that don't belong in proposer_policy::validate(): + // - account existence (requires apply_context) + // - K1/R1 key type enforcement (uses unactivated_key_type to convey + // that non-K1/R1 keys need a protocol feature to be activated — a + // distinct category from structural validation errors) + // - key.valid() semantics using key_type = fc::crypto::public_key::key_type; - - // check that producers are unique - std::set unique_producers; - for (const auto& p: producers) { - SYS_ASSERT( context.is_account(p.producer_name), wasm_execution_error, "producer schedule includes a nonexisting account" ); + for (const auto& p : candidate.proposer_schedule.producers) { + SYS_ASSERT(context.is_account(p.producer_name), wasm_execution_error, + "producer schedule includes a nonexisting account"); std::visit([&p](const auto& a) { - uint32_t sum_weights = 0; - std::set unique_keys; - for (const auto& kw: a.keys ) { - SYS_ASSERT( kw.key.contains_type(key_type::k1, key_type::r1), unactivated_key_type, - "Unactivated key type used in proposed producer schedule"); - SYS_ASSERT( kw.key.valid(), wasm_execution_error, "producer schedule includes an invalid key" ); - - if (std::numeric_limits::max() - sum_weights <= kw.weight) { - sum_weights = std::numeric_limits::max(); - } else { - sum_weights += kw.weight; - } - - unique_keys.insert(kw.key); + for (const auto& kw : a.keys) { + SYS_ASSERT(kw.key.contains_type(key_type::k1, key_type::r1), unactivated_key_type, + "Unactivated key type used in proposed producer schedule"); + SYS_ASSERT(kw.key.valid(), wasm_execution_error, "producer schedule includes an invalid key"); } - - SYS_ASSERT( a.keys.size() == unique_keys.size(), wasm_execution_error, "producer schedule includes a duplicated key for {}", p.producer_name); - SYS_ASSERT( a.threshold > 0, wasm_execution_error, "producer schedule includes an authority with a threshold of 0 for {}", p.producer_name); - SYS_ASSERT( sum_weights >= a.threshold, wasm_execution_error, "producer schedule includes an unsatisfiable authority for {}", p.producer_name); }, p.authority); - - unique_producers.insert(p.producer_name); } - SYS_ASSERT( producers.size() == unique_producers.size(), wasm_execution_error, "duplicate producer name in producer schedule" ); - return context.control.set_proposed_producers( context.trx_context, std::move(producers) ); + return context.control.set_proposed_producers( context.trx_context, + std::move(candidate.proposer_schedule.producers) ); } uint32_t interface::get_wasm_parameters_packed( span packed_parameters, uint32_t max_version ) const { @@ -168,37 +166,28 @@ namespace sysio { namespace chain { namespace webassembly { finalizer_policy abi_finpol; fc::raw::unpack(ds, abi_finpol); - std::vector& finalizers = abi_finpol.finalizers; - - SYS_ASSERT( finalizers.size() <= config::max_finalizers, wasm_execution_error, - "Finalizer policy exceeds the maximum finalizer count for this chain" ); - SYS_ASSERT( finalizers.size() > 0, wasm_execution_error, "Finalizers cannot be empty" ); - - std::set unique_finalizer_keys; - - uint64_t weight_sum = 0; - + // Transform the ABI-provided payload into the chain-level finalizer_policy. + // The per-key length check stays here because it is a wire-format guard on the + // ABI input shape, not a property of the stored policy (the chain-level + // bls_public_key type enforces size=96 at construction). chain::finalizer_policy finpol; finpol.threshold = abi_finpol.threshold; - for (auto& f: finalizers) { - SYS_ASSERT( f.description.size() <= config::max_finalizer_description_size, wasm_execution_error, - "Finalizer description greater than {}", config::max_finalizer_description_size ); - SYS_ASSERT(std::numeric_limits::max() - weight_sum >= f.weight, wasm_execution_error, - "sum of weights causes uint64_t overflow"); - weight_sum += f.weight; + finpol.finalizers.reserve(abi_finpol.finalizers.size()); + for (auto& f: abi_finpol.finalizers) { SYS_ASSERT(f.public_key.size() == 96, wasm_execution_error, "Invalid bls public key length"); fc::crypto::bls::public_key pk(std::span(f.public_key.data(), 96)); - SYS_ASSERT( unique_finalizer_keys.insert(pk).second, wasm_execution_error, - "Duplicate public key: {}", fc::json::to_log_string(pk) ); finpol.finalizers.push_back(chain::finalizer_authority{.description = std::move(f.description), .weight = f.weight, .public_key{pk}}); } - SYS_ASSERT( weight_sum >= finpol.threshold && finpol.threshold > weight_sum / 2, wasm_execution_error, - "Finalizer policy threshold ({}) must be greater than half of the sum of the weights ({}), " - "and less than or equal to the sum of the weights", - finpol.threshold, weight_sum ); + // Structural validation is factored into finalizer_policy::validate() so the + // snapshot-load path and this intrinsic enforce identical invariants. + try { + finpol.validate(); + } catch (const invalid_finalizer_policy_exception& e) { + SYS_THROW(wasm_execution_error, "set_finalizers: {}", e.top_message()); + } context.trx_context.set_proposed_finalizers( std::move(finpol) ); } diff --git a/unittests/block_tests.cpp b/unittests/block_tests.cpp index 48b4ee1e14..14d9684ec1 100644 --- a/unittests/block_tests.cpp +++ b/unittests/block_tests.cpp @@ -1,11 +1,22 @@ #include #include +#include +#include +#include #include using namespace sysio; using namespace testing; using namespace chain; +// Accessor reaches into block_handle's private internal() for tests that need +// the underlying block_state_ptr. block_handle declares friend block_handle_accessor. +namespace sysio::chain { + struct block_handle_accessor { + static const block_state_ptr& get_bsp(const block_handle& h) { return h.internal(); } + }; +} + BOOST_AUTO_TEST_SUITE(block_tests) BOOST_AUTO_TEST_CASE( block_with_invalid_tx_test ) @@ -572,4 +583,102 @@ BOOST_AUTO_TEST_CASE( failed_trx_excluded_from_block_report ) BOOST_TEST( captured->cpu_usage_us == baseline.cpu_usage_us ); } +// Regression test for the snapshot hardening invariant added in block_state(snapshot). +// For any non-genesis block_state, core.latest_qc_claim() must equal header.qc_claim +// (finality_core::next sets latest_qc_claim from the header's claim). A snapshot with +// the two disagreeing would break verify_basic_proper_block_invariants' assumption +// that the core is the authoritative QC-claim reference. Snapshot loading must reject. +BOOST_AUTO_TEST_CASE(snapshot_core_header_qc_claim_mismatch_rejected) try { + savanna_tester chain; + chain.produce_blocks(4); // past genesis + + // controller::head() returns block_handle by value; bind to a named local so its + // embedded shared_ptr outlives live_bsp (a reference into it). Without this the + // temporary dies at the end of the full-expression and the get_bsp() reference + // dangles; clang and the sanitizer builds reuse the stack and read garbage. + const auto head_handle = chain.control->head(); + const auto& live_bsp = block_handle_accessor::get_bsp(head_handle); + BOOST_REQUIRE(!live_bsp->core.is_genesis_core()); + BOOST_REQUIRE(live_bsp->core.latest_qc_claim() == live_bsp->header.qc_claim); // sanity + + // Build a snapshot_block_state_v1 from the live one, then tamper header.qc_claim + // to something provably different (block_num far in the past, weak). Must NOT + // match core.latest_qc_claim(), which for a running chain reflects a recent block. + snapshot_detail::snapshot_block_state_v1 sbs{*live_bsp}; + sbs.header.qc_claim = qc_claim_t{0, false}; + BOOST_REQUIRE(sbs.core.latest_qc_claim() != sbs.header.qc_claim); // confirm setup + + BOOST_CHECK_EXCEPTION(block_state{std::move(sbs)}, snapshot_exception, + fc_exception_message_contains("core.latest_qc_claim")); +} FC_LOG_AND_RETHROW() + +// Genesis is intentionally exempt: core is constructed with latest_qc_claim={1,false} +// while header.qc_claim={0,false} (see create_genesis_block). Confirm a genesis-core +// snapshot loads cleanly despite the qc_claim disagreement. +BOOST_AUTO_TEST_CASE(snapshot_genesis_core_header_mismatch_allowed) try { + genesis_state gs; + auto genesis_bsp = block_state::create_genesis_block(gs); + BOOST_REQUIRE(genesis_bsp->core.is_genesis_core()); + BOOST_REQUIRE(genesis_bsp->core.latest_qc_claim() != genesis_bsp->header.qc_claim); // confirmed design + + snapshot_detail::snapshot_block_state_v1 sbs{*genesis_bsp}; + BOOST_CHECK_NO_THROW(block_state{std::move(sbs)}); +} FC_LOG_AND_RETHROW() + +// A tampered snapshot can carry a null activated_protocol_features. The snapshot +// block_state ctor reaches it transitively via compute_finality_digest -> compute_base_digest, +// which asserts then dereferences the pointer. The ctor must reject with snapshot_exception +// before the digest path runs. +BOOST_AUTO_TEST_CASE(snapshot_null_activated_protocol_features_rejected) try { + savanna_tester chain; + chain.produce_blocks(4); + + const auto head_handle = chain.control->head(); + const auto& live_bsp = block_handle_accessor::get_bsp(head_handle); + + snapshot_detail::snapshot_block_state_v1 sbs{*live_bsp}; + sbs.activated_protocol_features.reset(); // null out before construction + + BOOST_CHECK_EXCEPTION(block_state{std::move(sbs)}, snapshot_exception, + fc_exception_message_contains("activated_protocol_features")); +} FC_LOG_AND_RETHROW() + +// A tampered snapshot can carry an empty / malformed finality_core. Without validating +// the core first, compute_finality_digest's call to core.latest_qc_claim() would assert +// on the empty links vector. The ctor must reject with snapshot_exception instead. +BOOST_AUTO_TEST_CASE(snapshot_empty_core_rejected) try { + savanna_tester chain; + chain.produce_blocks(4); + + const auto head_handle = chain.control->head(); + const auto& live_bsp = block_handle_accessor::get_bsp(head_handle); + + snapshot_detail::snapshot_block_state_v1 sbs{*live_bsp}; + sbs.core = finality_core{}; // default-initialized: links and refs both empty + + BOOST_CHECK_EXCEPTION(block_state{std::move(sbs)}, snapshot_exception, + fc_exception_message_contains("links must not be empty")); +} FC_LOG_AND_RETHROW() + +// A snapshot is never taken at genesis, so every snapshot block_state must carry a +// valid_t (finality-tree) structure. A tampered snapshot that nulls it out must be +// rejected at construction: get_validation_mroot() would otherwise return an empty +// digest (after a debug-only assert) and the corruption would surface only later as +// a finality_mroot mismatch in apply_block. +BOOST_AUTO_TEST_CASE(snapshot_null_valid_rejected) try { + savanna_tester chain; + chain.produce_blocks(4); + + const auto head_handle = chain.control->head(); + const auto& live_bsp = block_handle_accessor::get_bsp(head_handle); + BOOST_REQUIRE(!live_bsp->core.is_genesis_core()); + + snapshot_detail::snapshot_block_state_v1 sbs{*live_bsp}; + BOOST_REQUIRE(sbs.valid); // a validated head block carries a valid_t structure + sbs.valid.reset(); // tamper: null it out before construction + + BOOST_CHECK_EXCEPTION(block_state{std::move(sbs)}, snapshot_exception, + fc_exception_message_contains("valid_t finality structure")); +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END() diff --git a/unittests/merkle_tree_tests.cpp b/unittests/merkle_tree_tests.cpp index 8e647c5088..2e6b8d3066 100644 --- a/unittests/merkle_tree_tests.cpp +++ b/unittests/merkle_tree_tests.cpp @@ -100,4 +100,50 @@ BOOST_AUTO_TEST_CASE(consistency_over_large_range) { } } +// calculate_merkle dispatches to a multi-threaded implementation at two +// thresholds (detail::calculate_merkle_pow2): +// size >= 256 -> 2 threads +// size >= 2048 -> 4 threads +// Output must be byte-identical to the single-threaded path and stable across +// repeated invocations. Consensus safety depends on this: every node that +// computes a merkle root over the same input must get the same bytes regardless +// of how the work is partitioned across threads. +BOOST_AUTO_TEST_CASE(async_matches_sequential_and_is_reproducible) { + // Sizes chosen to span both async thresholds and boundary cases: just below + // 256, exactly at 256, between the two thresholds, exactly at 2048, and well + // past the 4-thread threshold. + const std::vector sizes = { + 255, 256, 257, // 2-thread boundary + 512, 1024, // within 2-thread range + 2047, 2048, 2049, // 4-thread boundary + 4096, 8192 // well past + }; + + // Build the largest vector once; slice with std::span to exercise each size. + const std::vector digests = create_test_digests(sizes.back()); + + for (size_t n : sizes) { + auto input = std::span(digests.begin(), n); + + // Cross-check async calculate_merkle against the purely-sequential + // incremental_merkle_tree to pin down the expected byte pattern. + incremental_merkle_tree tree; + for (size_t j = 0; j < n; ++j) tree.append(digests[j]); + const digest_type expected = tree.get_root(); + + const digest_type async_first = calculate_merkle(input); + BOOST_CHECK_MESSAGE(async_first == expected, + "async calculate_merkle diverged from sequential tree at size " << n); + + // Reproducibility: re-running the same computation must return the same + // bytes. 10 iterations is enough to surface any thread-race that alters + // output (probabilistically; if races existed, they'd show up reliably + // at these sizes since the thread pool schedules all slices concurrently). + for (int i = 0; i < 10; ++i) { + BOOST_CHECK_MESSAGE(calculate_merkle(input) == expected, + "calculate_merkle not reproducible at size " << n << " on iteration " << i); + } + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/unittests/producer_schedule_tests.cpp b/unittests/producer_schedule_tests.cpp index f018cc7f79..f0b89a1a13 100644 --- a/unittests/producer_schedule_tests.cpp +++ b/unittests/producer_schedule_tests.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -120,10 +121,11 @@ BOOST_AUTO_TEST_CASE(satisfiable_msig_test) try { producer_authority{"alice"_n, block_signing_authority_v0{2, {{get_public_key("alice"_n, "bs1"), 1}}}} }; - // ensure that the entries in a wtmsig schedule are rejected if not satisfiable + // Threshold (2) exceeds sum of key weights (1) — rejected by proposer_policy::validate() + // (via set_proposed_producers_common). Error message prefix is "proposer policy version N:". BOOST_REQUIRE_EXCEPTION( chain.set_producer_schedule( sch1 ), wasm_execution_error, - fc_exception_message_is( "producer schedule includes an unsatisfiable authority for alice" ) + fc_exception_message_contains( "not satisfiable by sum of key weights" ) ); } FC_LOG_AND_RETHROW() @@ -139,10 +141,10 @@ BOOST_AUTO_TEST_CASE(duplicate_producers_test) try { producer_authority{"alice"_n, block_signing_authority_v0{1, {{get_public_key("alice"_n, "bs2"), 1}}}} }; - // ensure that the schedule is rejected if it has duplicate producers in it + // Duplicate producer name rejected by proposer_policy::validate(). BOOST_REQUIRE_EXCEPTION( chain.set_producer_schedule( sch1 ), wasm_execution_error, - fc_exception_message_is( "duplicate producer name in producer schedule" ) + fc_exception_message_contains( "duplicate producer name" ) ); } FC_LOG_AND_RETHROW() @@ -155,13 +157,13 @@ BOOST_FIXTURE_TEST_CASE( duplicate_keys_test, validating_tester ) try { producer_authority{"alice"_n, block_signing_authority_v0{2, {{get_public_key("alice"_n, "bs1"), 1}, {get_public_key("alice"_n, "bs1"), 1}}}} }; - // ensure that the schedule is rejected if it has duplicate keys for a single producer in it + // Duplicate keys within a single producer's authority rejected by proposer_policy::validate(). BOOST_REQUIRE_EXCEPTION( set_producer_schedule( sch1 ), wasm_execution_error, - fc_exception_message_is( "producer schedule includes a duplicated key for alice" ) + fc_exception_message_contains( "authority has duplicate key" ) ); - // ensure that multiple producers are allowed to share keys + // Multiple producers are allowed to share keys (key-uniqueness is per-authority, not global). vector sch2 = { producer_authority{"alice"_n, block_signing_authority_v0{1, {{get_public_key("alice"_n, "bs1"), 1}}}}, producer_authority{"bob"_n, block_signing_authority_v0{1, {{get_public_key("alice"_n, "bs1"), 1}}}} @@ -170,6 +172,78 @@ BOOST_FIXTURE_TEST_CASE( duplicate_keys_test, validating_tester ) try { set_producer_schedule( sch2 ); } FC_LOG_AND_RETHROW() +// Empty producer schedule is rejected by proposer_policy::validate(). +// Note: the transaction_context::set_proposed_producers() early-return-on-empty is +// behind this check so it never fires when the intrinsic is called via setprods. +BOOST_AUTO_TEST_CASE(empty_producer_schedule_test) try { + savanna_tester chain; + chain.produce_block(); + + vector empty_sch; + + BOOST_REQUIRE_EXCEPTION( + chain.set_producer_schedule( empty_sch ), wasm_execution_error, + fc_exception_message_contains( "producer schedule must not be empty" ) + ); +} FC_LOG_AND_RETHROW() + +// Happy-path direct test for proposer_policy::validate(). Guards against the +// method ever accidentally rejecting legitimate input — complements the negative +// tests below. The set_producer_schedule tests give indirect coverage, but a +// direct call pins the contract cheaply. +BOOST_AUTO_TEST_CASE(validate_accepts_well_formed_policy) try { + proposer_policy pol; + pol.proposer_schedule.version = 1; + pol.proposer_schedule.producers = { + producer_authority{ "alice"_n, block_signing_authority_v0{ 1, {{ get_public_key("alice"_n, "bs1"), 1 }} } }, + producer_authority{ "bob"_n, block_signing_authority_v0{ 2, {{ get_public_key("bob"_n, "bs1"), 1 }, + { get_public_key("bob"_n, "bs2"), 1 }} } } + }; + BOOST_CHECK_NO_THROW(pol.validate()); +} FC_LOG_AND_RETHROW() + +// proposer_policy::validate() caps producer count at config::max_producers. +// Tested directly against validate() rather than through set_producer_schedule +// because creating > max_producers accounts in one test exhausts block CPU +// budget. The intrinsic wiring is already exercised by the other tests in this +// suite; this test verifies the specific branch in validate(). +BOOST_AUTO_TEST_CASE(validate_rejects_too_many_producers) try { + proposer_policy pol; + const size_t n = config::max_producers + 1; + pol.proposer_schedule.version = 1; + pol.proposer_schedule.producers.reserve(n); + // Build deterministic valid account names ("paa", "pab", ..., "pzz", ...). + for (size_t i = 0; i < n; ++i) { + std::string nm = "p"; + for (size_t v = i; nm.size() < 12; ) { + nm.push_back(static_cast('a' + (v % 26))); + v /= 26; + if (v == 0) break; + } + pol.proposer_schedule.producers.push_back( + producer_authority{ account_name{nm}, + block_signing_authority_v0{ 1, {{ get_public_key(account_name{nm}, "bs1"), 1 }} } }); + } + BOOST_CHECK_EXCEPTION(pol.validate(), producer_schedule_exception, + fc_exception_message_contains("exceeds max")); +} FC_LOG_AND_RETHROW() + +// proposer_policy::validate() rejects a per-authority threshold of zero. +BOOST_AUTO_TEST_CASE(authority_threshold_zero_test) try { + savanna_tester chain; + chain.create_accounts( {"alice"_n} ); + chain.produce_block(); + + vector sch = { + producer_authority{ "alice"_n, block_signing_authority_v0{ 0, {{ get_public_key("alice"_n, "bs1"), 1 }} } } + }; + + BOOST_REQUIRE_EXCEPTION( + chain.set_producer_schedule( sch ), wasm_execution_error, + fc_exception_message_contains( "authority threshold must be positive" ) + ); +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_CASE( large_authority_overflow_test ) try { block_signing_authority_v0 auth; diff --git a/unittests/protocol_feature_tests.cpp b/unittests/protocol_feature_tests.cpp index 925fee3977..a5806acecc 100644 --- a/unittests/protocol_feature_tests.cpp +++ b/unittests/protocol_feature_tests.cpp @@ -126,9 +126,13 @@ BOOST_AUTO_TEST_CASE( double_activation ) try { c.schedule_protocol_features_wo_preactivation( {*d} ); + // _start_block concatenates the wo-preactivation list onto the preactivated list, + // producing a duplicate digest. check_protocol_features now detects the duplicate + // earlier (by dedup on its own activation-list scan) rather than at start_block's + // preactivation-map check. Both paths reject; this one is just earlier and more specific. BOOST_CHECK_EXCEPTION( c.produce_block();, - block_validate_exception, - fc_exception_message_starts_with( "attempted duplicate activation within a single block:" ) + protocol_feature_exception, + fc_exception_message_contains( "appears more than once in the activation list" ) ); c.protocol_features_to_be_activated_wo_preactivation.clear(); @@ -379,7 +383,7 @@ BOOST_AUTO_TEST_CASE( disallow_empty_producer_schedule_test ) { try { c.produce_block(); BOOST_REQUIRE_EXCEPTION( c.set_producers_legacy( {} ), wasm_execution_error, - fc_exception_message_is( "Producer schedule cannot be empty" ) ); + fc_exception_message_contains( "producer schedule must not be empty" ) ); // Setting non empty producer schedule should still be fine vector producer_names = {"alice"_n,"bob"_n,"carol"_n}; diff --git a/unittests/set_finalizers_intrinsic_tests.cpp b/unittests/set_finalizers_intrinsic_tests.cpp new file mode 100644 index 0000000000..102e923264 --- /dev/null +++ b/unittests/set_finalizers_intrinsic_tests.cpp @@ -0,0 +1,300 @@ +// Direct negative-path coverage for the set_finalizers host function and +// finalizer_policy::validate() (invoked from set_finalizers). The sysio.bios +// setfinalizer action performs its own validation before the intrinsic, so the +// high-level testing helpers never exercise the intrinsic's checks. These tests +// deploy a small WAST contract that forwards the action payload directly into +// set_finalizers, letting us inject hand-packed bytes that trigger each check. + +#include +#include +#include +#include + +#include + +#include + +using namespace sysio::testing; +using namespace sysio::chain; + +namespace { + +// Mirrors the wire format that set_finalizers unpacks. Must match the struct +// declared inside libraries/chain/webassembly/privileged.cpp exactly. +struct abi_finalizer_authority { + std::string description; + uint64_t weight = 0; + std::vector public_key; +}; + +struct abi_finalizer_policy { + uint64_t threshold = 0; + std::vector finalizers; +}; + +} // namespace + +FC_REFLECT(abi_finalizer_authority, (description)(weight)(public_key)) +FC_REFLECT(abi_finalizer_policy, (threshold)(finalizers)) + +namespace { + +// Forwards action payload bytes directly into set_finalizers with format=0. +// `apply(recv, code, action)` receives the action via the WASM ABI; the action +// data is whatever we pass to push_action, read via read_action_data / action_data_size. +constexpr const char set_finalizers_forward_wast[] = R"=====( +(module + (import "env" "set_finalizers" (func $set_finalizers (param i64 i32 i32))) + (import "env" "read_action_data" (func $read_action_data (param i32 i32) (result i32))) + (import "env" "action_data_size" (func $action_data_size (result i32))) + (memory $0 1) + (export "apply" (func $apply)) + (func $apply (param $0 i64) (param $1 i64) (param $2 i64) + (drop (call $read_action_data (i32.const 0) (call $action_data_size))) + (call $set_finalizers (i64.const 0) (i32.const 0) (call $action_data_size)) + ) +) +)====="; + +// Forwards with format=1 to exercise the "unknown format" error path. +constexpr const char set_finalizers_bad_format_wast[] = R"=====( +(module + (import "env" "set_finalizers" (func $set_finalizers (param i64 i32 i32))) + (import "env" "read_action_data" (func $read_action_data (param i32 i32) (result i32))) + (import "env" "action_data_size" (func $action_data_size (result i32))) + (memory $0 1) + (export "apply" (func $apply)) + (func $apply (param $0 i64) (param $1 i64) (param $2 i64) + (drop (call $read_action_data (i32.const 0) (call $action_data_size))) + (call $set_finalizers (i64.const 1) (i32.const 0) (call $action_data_size)) + ) +) +)====="; + +// Returns a valid 96-byte BLS public key derived from a name, in the +// affine-little-endian non-montgomery format expected by the intrinsic. The +// intrinsic calls `bls_public_key(std::span)` on the input +// bytes before validate() runs, which asserts on curve membership, so the +// negative tests for validate()-level checks must supply real keys. +// get_bls_key dispatches on the variant: `name` → SHA256(name) as seed. +std::vector pk_for(sysio::chain::name key_name) { + auto [_priv, pub, _pop, _sig] = get_bls_key(key_name); + auto serialized = pub.serialize(); + return std::vector(serialized.begin(), serialized.end()); +} + +struct set_finalizers_fixture { + tester chain{setup_policy::preactivate_feature_and_new_bios}; + account_name alice_account{"alice"_n}; + + set_finalizers_fixture() { + chain.create_accounts({alice_account}); + chain.produce_block(); + } + + // set_privileged requires the account to have code deployed first. + void set_code_and_produce(const char* wast) { + chain.set_code(alice_account, wast); + chain.set_privileged(alice_account); + chain.produce_block(); + } + + // Push an action on alice carrying `payload` as its data. Returns the trace. + transaction_trace_ptr push_payload(const std::vector& payload) { + action a({{alice_account, permission_name("active")}}, + alice_account, action_name(), payload); + signed_transaction trx; + trx.actions.emplace_back(std::move(a)); + chain.set_transaction_headers(trx); + trx.sign(chain.get_private_key(alice_account, "active"), chain.control->get_chain_id()); + return chain.push_transaction(trx); + } +}; + +} // namespace + +BOOST_AUTO_TEST_SUITE(set_finalizers_intrinsic_tests) + +// packed_finalizer_format != 0 rejected by the intrinsic. +BOOST_FIXTURE_TEST_CASE(unknown_format_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_bad_format_wast); + + // Any bytes work — the format check fires before unpack. + abi_finalizer_policy pol; + pol.threshold = 1; + pol.finalizers.push_back({.description = "f", .weight = 1, .public_key = pk_for("k1"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("unknown format")); +} FC_LOG_AND_RETHROW() + +// public_key.size() != 96 rejected at the wire-format guard before validate(). +BOOST_FIXTURE_TEST_CASE(bad_pubkey_length_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 1; + // 95-byte key — invalid BLS size. The wire-format guard fires before curve validation. + std::vector short_pk(95, 0x42); + pol.finalizers.push_back({.description = "f", .weight = 1, .public_key = std::move(short_pk)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("Invalid bls public key length")); +} FC_LOG_AND_RETHROW() + +// Empty finalizers rejected by validate(). +BOOST_FIXTURE_TEST_CASE(empty_finalizers_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 1; + // No finalizers. + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("finalizers must not be empty")); +} FC_LOG_AND_RETHROW() + +// finalizers.size() > config::max_finalizers rejected by validate(). +// Tested directly against finalizer_policy::validate() rather than through the +// WAST forwarder: max_finalizers = 65536 produces a ~7MB packed payload, which +// exceeds the default max_transaction_net_usage (512KB). The intrinsic wiring +// is already exercised by the other tests in this suite; this test verifies the +// specific branch in validate(). +BOOST_AUTO_TEST_CASE(validate_rejects_too_many_finalizers) try { + finalizer_policy pol; + const size_t n = config::max_finalizers + 1; + pol.generation = 1; + pol.threshold = n; // satisfy BFT invariant so only the count check can fire + pol.finalizers.reserve(n); + auto [_priv, pub, _pop, _sig] = get_bls_key("shared"_n); + for (size_t i = 0; i < n; ++i) { + pol.finalizers.push_back({.description = "f", .weight = 1, .public_key = pub}); + } + BOOST_CHECK_EXCEPTION(pol.validate(), invalid_finalizer_policy_exception, + fc_exception_message_contains("exceeds max")); +} FC_LOG_AND_RETHROW() + +// description.size() > config::max_finalizer_description_size rejected by validate(). +BOOST_FIXTURE_TEST_CASE(description_too_long_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 1; + pol.finalizers.push_back({ + .description = std::string(config::max_finalizer_description_size + 1, 'x'), + .weight = 1, + .public_key = pk_for("k1"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("description size")); +} FC_LOG_AND_RETHROW() + +// weight == 0 rejected by validate(). +BOOST_FIXTURE_TEST_CASE(zero_weight_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 1; + pol.finalizers.push_back({.description = "f", .weight = 0, .public_key = pk_for("k1"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("weight must be positive")); +} FC_LOG_AND_RETHROW() + +// uint64 weight-sum overflow rejected by validate(). +BOOST_FIXTURE_TEST_CASE(weight_overflow_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + // threshold arbitrary but satisfy BFT if no overflow (won't be reached). + pol.threshold = 1; + pol.finalizers.push_back({.description = "f", .weight = std::numeric_limits::max(), + .public_key = pk_for("k1"_n)}); + pol.finalizers.push_back({.description = "g", .weight = 1, + .public_key = pk_for("k2"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("sum of weights overflows")); +} FC_LOG_AND_RETHROW() + +// Duplicate public_key rejected by validate(). +BOOST_FIXTURE_TEST_CASE(duplicate_pubkey_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 2; + auto pk = pk_for("dup"_n); + pol.finalizers.push_back({.description = "a", .weight = 1, .public_key = pk}); + pol.finalizers.push_back({.description = "b", .weight = 1, .public_key = pk}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("duplicate public_key")); +} FC_LOG_AND_RETHROW() + +// threshold == 0 rejected (0 is not > 0/2 for sum > 0). +BOOST_FIXTURE_TEST_CASE(threshold_zero_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 0; + pol.finalizers.push_back({.description = "f", .weight = 3, .public_key = pk_for("k1"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("threshold")); +} FC_LOG_AND_RETHROW() + +// threshold > sum rejected. +BOOST_FIXTURE_TEST_CASE(threshold_exceeds_sum_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 10; + pol.finalizers.push_back({.description = "f", .weight = 3, .public_key = pk_for("k1"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("threshold")); +} FC_LOG_AND_RETHROW() + +// threshold == sum/2 exactly (BFT boundary) rejected. +// For sum=4, threshold=2 fails: 2 > 4/2 = 2 is false. +BOOST_FIXTURE_TEST_CASE(threshold_at_half_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 2; + pol.finalizers.push_back({.description = "f", .weight = 2, .public_key = pk_for("k1"_n)}); + pol.finalizers.push_back({.description = "g", .weight = 2, .public_key = pk_for("k2"_n)}); + auto bytes = fc::raw::pack(pol); + + BOOST_CHECK_EXCEPTION(push_payload(bytes), wasm_execution_error, + fc_exception_message_contains("threshold")); +} FC_LOG_AND_RETHROW() + +// Happy path — confirms the forwarding contract actually reaches validate() and +// that a well-formed policy is accepted. This guards against the negative tests +// all passing for the wrong reason (e.g. the fixture itself rejecting the +// action before the intrinsic is reached). +BOOST_FIXTURE_TEST_CASE(well_formed_policy_accepted_test, set_finalizers_fixture) try { + set_code_and_produce(set_finalizers_forward_wast); + + abi_finalizer_policy pol; + pol.threshold = 2; // > sum/2 = 1, <= sum = 2 + pol.finalizers.push_back({.description = "a", .weight = 1, .public_key = pk_for("k1"_n)}); + pol.finalizers.push_back({.description = "b", .weight = 1, .public_key = pk_for("k2"_n)}); + auto bytes = fc::raw::pack(pol); + + // Should not throw. push_payload returns a trace — any exception during intrinsic + // would have been re-raised as wasm_execution_error. + BOOST_REQUIRE_NO_THROW(push_payload(bytes)); +} FC_LOG_AND_RETHROW() + +BOOST_AUTO_TEST_SUITE_END() diff --git a/unittests/signature_recovery_tests.cpp b/unittests/signature_recovery_tests.cpp index 193ff15e7a..2859e6a706 100644 --- a/unittests/signature_recovery_tests.cpp +++ b/unittests/signature_recovery_tests.cpp @@ -286,4 +286,45 @@ BOOST_AUTO_TEST_CASE(trx_extension_rejected_on_push) { ); } +// Regression test for verify_signee's dedup assumption. verify_signee (block_state.cpp) +// collects recovered keys into std::set and relies on set equality to +// detect "block signed by same key twice". The equality semantics come from +// public_key_type's defaulted <=>, which in turn compares the underlying std::variant +// of curve-specific shim types. If recover() ever produces two structurally-distinct +// public_key_type values that represent the same logical key (e.g. different internal +// encodings of the same curve point), the "same key twice" check would silently miss +// and keys_satisfy_and_relevant would over-count relevant signatures. +// +// This test recovers the same key via two fresh calls with identical inputs and +// asserts the set collapses them to one entry. It's never expected to fail today but +// catches a regression if recover() or the variant ordering changes. +BOOST_AUTO_TEST_CASE(recovered_keys_dedup_in_set) try { + auto priv = fc::crypto::private_key::generate(); + auto digest = fc::sha256::hash("hello world"); + auto sig = priv.sign(digest); + + // Two fresh recover calls with the same inputs should yield the same public_key_type. + auto pk1 = fc::crypto::public_key::recover(sig, digest); + auto pk2 = fc::crypto::public_key::recover(sig, digest); + BOOST_CHECK(pk1 == pk2); + + // Insertion into std::set should collapse them. + std::set keys; + auto [it1, inserted1] = keys.emplace(pk1); + BOOST_CHECK(inserted1); + auto [it2, inserted2] = keys.emplace(pk2); + BOOST_CHECK(!inserted2); // second emplace must be rejected -- same key + BOOST_REQUIRE_EQUAL(keys.size(), 1u); + + // Paired with verify_signee's direct call shape: recover into set via distinct + // loop iterations and confirm count is 1. + std::vector sigs{sig, sig}; + std::set keys2; + for (const auto& s : sigs) { + auto [iter, inserted] = keys2.emplace(fc::crypto::public_key::recover(s, digest)); + (void)iter; (void)inserted; + } + BOOST_REQUIRE_EQUAL(keys2.size(), 1u); +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END() diff --git a/unittests/test_s_root_extension.cpp b/unittests/test_s_root_extension.cpp index 40b5f1ae6f..bd11e5e4dc 100644 --- a/unittests/test_s_root_extension.cpp +++ b/unittests/test_s_root_extension.cpp @@ -113,4 +113,128 @@ BOOST_AUTO_TEST_CASE(something_to_report) { } FC_LOG_AND_RETHROW() } +// Direct unit tests for the validate_s_root_extensions_match helper extracted +// from apply_block. Each test builds received/constructed extension vectors by +// hand and verifies the helper accepts or rejects as expected. +namespace { + +// Build a packed s_root_extension with distinct-but-valid fields. The `nonce` +// byte perturbs current_s_id so two entries with different nonces compare +// unequal after packing. +std::vector make_sre(uint8_t nonce) { + s_header h; + h.contract_name = "sysio"_n; + h.previous_s_id = checksum256_type(); + h.current_s_id = checksum256_type(); + h.current_s_id._hash[0] = nonce; // differentiate payloads + h.current_s_root = checksum256_type(); + h.previous_block_num = 0; + return fc::raw::pack(s_root_extension(h)); +} + +// Append an s_root_extension entry to an extensions_type. +void push_sre(extensions_type& v, uint8_t nonce) { + v.emplace_back(s_root_extension::extension_id(), make_sre(nonce)); +} + +// Append a non-s_root extension entry (extension_id = 0) with arbitrary bytes. +// Used to verify the helper filters by extension_id rather than just position. +void push_other(extensions_type& v, const std::string& s) { + v.emplace_back(uint16_t{0}, std::vector(s.begin(), s.end())); +} + +} // namespace + +BOOST_AUTO_TEST_CASE(validate_match_zero_entries) { + extensions_type received; + extensions_type constructed; + BOOST_CHECK_NO_THROW(validate_s_root_extensions_match(received, constructed)); +} + +BOOST_AUTO_TEST_CASE(validate_match_zero_among_other_extensions) { + // No s_root_extensions on either side, but both have unrelated extensions. + // Helper must filter by extension_id and accept. + extensions_type received; push_other(received, "pfa_bytes"); + extensions_type constructed; push_other(constructed, "pfa_bytes_different"); // different content - ignored + BOOST_CHECK_NO_THROW(validate_s_root_extensions_match(received, constructed)); +} + +BOOST_AUTO_TEST_CASE(validate_match_one_entry) { + extensions_type received; push_sre(received, 1); + extensions_type constructed; push_sre(constructed, 1); + BOOST_CHECK_NO_THROW(validate_s_root_extensions_match(received, constructed)); +} + +BOOST_AUTO_TEST_CASE(validate_match_three_entries_in_order) { + extensions_type received; + push_sre(received, 1); push_sre(received, 2); push_sre(received, 3); + extensions_type constructed; + push_sre(constructed, 1); push_sre(constructed, 2); push_sre(constructed, 3); + BOOST_CHECK_NO_THROW(validate_s_root_extensions_match(received, constructed)); +} + +BOOST_AUTO_TEST_CASE(validate_match_three_entries_with_interleaved_other) { + // s_root_extensions interleaved with other extension types. The helper only + // compares s_root_extensions by position-within-sre, ignoring everything else. + extensions_type received; + push_other(received, "x"); + push_sre(received, 1); + push_other(received, "y"); + push_sre(received, 2); + push_sre(received, 3); + extensions_type constructed; + push_sre(constructed, 1); + push_other(constructed, "z"); // different position of "other" - should not matter + push_sre(constructed, 2); + push_sre(constructed, 3); + BOOST_CHECK_NO_THROW(validate_s_root_extensions_match(received, constructed)); +} + +BOOST_AUTO_TEST_CASE(validate_rejects_received_has_more) { + extensions_type received; + push_sre(received, 1); push_sre(received, 2); + extensions_type constructed; + push_sre(constructed, 1); + BOOST_CHECK_EXCEPTION(validate_s_root_extensions_match(received, constructed), + block_validate_exception, + fc_exception_message_contains("count mismatch")); +} + +BOOST_AUTO_TEST_CASE(validate_rejects_constructed_has_more) { + extensions_type received; + push_sre(received, 1); + extensions_type constructed; + push_sre(constructed, 1); push_sre(constructed, 2); + BOOST_CHECK_EXCEPTION(validate_s_root_extensions_match(received, constructed), + block_validate_exception, + fc_exception_message_contains("count mismatch")); +} + +BOOST_AUTO_TEST_CASE(validate_rejects_only_one_side_has_sre) { + extensions_type received; + push_sre(received, 1); + extensions_type constructed; // empty + BOOST_CHECK_EXCEPTION(validate_s_root_extensions_match(received, constructed), + block_validate_exception, + fc_exception_message_contains("count mismatch")); +} + +BOOST_AUTO_TEST_CASE(validate_rejects_payload_mismatch_at_first_slot) { + extensions_type received; push_sre(received, 1); + extensions_type constructed; push_sre(constructed, 2); + BOOST_CHECK_EXCEPTION(validate_s_root_extensions_match(received, constructed), + block_validate_exception, + fc_exception_message_contains("payload mismatch at slot 1")); +} + +BOOST_AUTO_TEST_CASE(validate_rejects_payload_mismatch_at_second_slot) { + extensions_type received; + push_sre(received, 1); push_sre(received, 2); + extensions_type constructed; + push_sre(constructed, 1); push_sre(constructed, 99); + BOOST_CHECK_EXCEPTION(validate_s_root_extensions_match(received, constructed), + block_validate_exception, + fc_exception_message_contains("payload mismatch at slot 2")); +} + BOOST_AUTO_TEST_SUITE_END()