From f7fd3225ea9fe89b21f7c13d352adb03e667f002 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 9 Oct 2023 20:45:49 +0300 Subject: [PATCH] Disallow block solutions in votes --- crates/pallet-subspace/src/lib.rs | 32 ++++++--- crates/pallet-subspace/src/mock.rs | 47 ++++++++++++-- crates/pallet-subspace/src/tests.rs | 65 +++++++++++++++++++ .../sc-consensus-subspace/src/slot_worker.rs | 28 +++++--- crates/sp-consensus-subspace/src/lib.rs | 39 +++++------ 5 files changed, 164 insertions(+), 47 deletions(-) diff --git a/crates/pallet-subspace/src/lib.rs b/crates/pallet-subspace/src/lib.rs index bd01363795..df4d4151c6 100644 --- a/crates/pallet-subspace/src/lib.rs +++ b/crates/pallet-subspace/src/lib.rs @@ -15,7 +15,7 @@ // limitations under the License. #![doc = include_str!("../README.md")] #![cfg_attr(not(feature = "std"), no_std)] -#![feature(assert_matches, const_option, let_chains)] +#![feature(array_chunks, assert_matches, const_option, let_chains, portable_simd)] #![warn(unused_must_use, unsafe_code, unused_variables, unused_must_use)] extern crate alloc; @@ -92,7 +92,9 @@ impl EraChangeTrigger for NormalEraChange { #[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, MaxEncodedLen, TypeInfo)] struct VoteVerificationData { + /// Block solution range, vote must not reach it solution_range: SolutionRange, + vote_solution_range: SolutionRange, current_slot: Slot, parent_slot: Slot, } @@ -1307,6 +1309,11 @@ fn current_vote_verification_data(is_block_initialized: bool) -> Vote let solution_ranges = SolutionRanges::::get(); VoteVerificationData { solution_range: if is_block_initialized { + solution_ranges.current + } else { + solution_ranges.next.unwrap_or(solution_ranges.current) + }, + vote_solution_range: if is_block_initialized { solution_ranges.voting_current } else { solution_ranges @@ -1339,6 +1346,7 @@ enum CheckVoteError { UnknownSegmentCommitment, InvalidHistorySize, InvalidSolution(String), + QualityTooHigh, InvalidProofOfTime, InvalidFutureProofOfTime, DuplicateVote, @@ -1360,6 +1368,7 @@ impl From for TransactionValidityError { CheckVoteError::UnknownSegmentCommitment => InvalidTransaction::Call, CheckVoteError::InvalidHistorySize => InvalidTransaction::Call, CheckVoteError::InvalidSolution(_) => InvalidTransaction::Call, + CheckVoteError::QualityTooHigh => InvalidTransaction::Call, CheckVoteError::InvalidProofOfTime => InvalidTransaction::Future, CheckVoteError::InvalidFutureProofOfTime => InvalidTransaction::Call, CheckVoteError::DuplicateVote => InvalidTransaction::Call, @@ -1530,12 +1539,12 @@ fn check_vote( .segment_index(), ); - if let Err(error) = verify_solution( + match verify_solution( solution.into(), slot.into(), (&VerifySolutionParams { proof_of_time: *proof_of_time, - solution_range: vote_verification_data.solution_range, + solution_range: vote_verification_data.vote_solution_range, piece_check_params: Some(PieceCheckParams { max_pieces_in_sector: T::MaxPiecesInSector::get(), segment_commitment, @@ -1548,11 +1557,18 @@ fn check_vote( }) .into(), ) { - debug!( - target: "runtime::subspace", - "Vote verification error: {error:?}" - ); - return Err(CheckVoteError::InvalidSolution(error)); + Ok(solution_distance) => { + if solution_distance <= vote_verification_data.solution_range { + return Err(CheckVoteError::QualityTooHigh); + } + } + Err(error) => { + debug!( + target: "runtime::subspace", + "Vote verification error: {error:?}" + ); + return Err(CheckVoteError::InvalidSolution(error)); + } } // Cheap proof of time verification is possible here because proof of time must have already diff --git a/crates/pallet-subspace/src/mock.rs b/crates/pallet-subspace/src/mock.rs index 2ff54e4a18..93ed5b26a9 100644 --- a/crates/pallet-subspace/src/mock.rs +++ b/crates/pallet-subspace/src/mock.rs @@ -39,18 +39,19 @@ use sp_runtime::testing::{Digest, DigestItem, Header, TestXt}; use sp_runtime::traits::{Block as BlockT, Header as _, IdentityLookup}; use sp_runtime::{BuildStorage, Perbill}; use sp_weights::Weight; -use std::iter; use std::marker::PhantomData; use std::num::{NonZeroU32, NonZeroU64, NonZeroUsize}; +use std::simd::Simd; use std::sync::{Once, OnceLock}; +use std::{iter, mem}; use subspace_archiving::archiver::{Archiver, NewArchivedSegment}; use subspace_core_primitives::crypto::kzg::{embedded_kzg_settings, Kzg}; use subspace_core_primitives::crypto::Scalar; use subspace_core_primitives::{ ArchivedBlockProgress, ArchivedHistorySegment, Blake2b256Hash, BlockNumber, HistorySize, LastArchivedBlock, Piece, PieceOffset, PosSeed, PotOutput, PublicKey, Record, - RecordedHistorySegment, SegmentCommitment, SegmentHeader, SegmentIndex, SlotNumber, Solution, - SolutionRange, REWARD_SIGNING_CONTEXT, + RecordedHistorySegment, SectorId, SegmentCommitment, SegmentHeader, SegmentIndex, SlotNumber, + Solution, SolutionRange, REWARD_SIGNING_CONTEXT, }; use subspace_erasure_coding::ErasureCoding; use subspace_farmer_components::auditing::audit_sector; @@ -58,6 +59,7 @@ use subspace_farmer_components::plotting::{plot_sector, PieceGetterRetryPolicy}; use subspace_farmer_components::FarmerProtocolInfo; use subspace_proof_of_space::shim::ShimTable; use subspace_proof_of_space::{Table, TableGenerator}; +use subspace_verification::is_within_solution_range; type PosTable = ShimTable; @@ -427,6 +429,7 @@ pub fn create_signed_vote( archived_history_segment: &ArchivedHistorySegment, reward_address: ::AccountId, solution_range: SolutionRange, + vote_solution_range: SolutionRange, ) -> SignedVote::Hash, ::AccountId> { let kzg = kzg_instance(); let erasure_coding = erasure_coding_instance(); @@ -466,13 +469,15 @@ pub fn create_signed_vote( )) .unwrap(); + let global_challenge = proof_of_time + .derive_global_randomness() + .derive_global_challenge(slot.into()); + let maybe_audit_result = audit_sector( &public_key, sector_index, - &proof_of_time - .derive_global_randomness() - .derive_global_challenge(slot.into()), - solution_range, + &global_challenge, + vote_solution_range, &plotted_sector_bytes, &plotted_sector.sector_metadata, ); @@ -492,6 +497,34 @@ pub fn create_signed_vote( .unwrap() .unwrap(); + let sector_id = SectorId::new( + PublicKey::from(keypair.public.to_bytes()).hash(), + solution.sector_index, + ); + let sector_slot_challenge = sector_id.derive_sector_slot_challenge(&global_challenge); + let masked_chunk = (Simd::from(solution.chunk.to_bytes()) + ^ Simd::from(solution.proof_of_space.hash())) + .to_array(); + // Extract audit chunk from masked chunk + let audit_chunk = SolutionRange::from_le_bytes( + *masked_chunk + .array_chunks::<{ mem::size_of::() }>() + .nth(usize::from(solution.audit_chunk_offset)) + .unwrap(), + ); + + // Check that solution quality is not too high + if is_within_solution_range( + &global_challenge, + audit_chunk, + §or_slot_challenge, + solution_range, + ) + .is_some() + { + continue; + } + let vote = Vote::::Hash, _>::V0 { height, parent_hash, diff --git a/crates/pallet-subspace/src/tests.rs b/crates/pallet-subspace/src/tests.rs index 0bf5131c7d..f55c844934 100644 --- a/crates/pallet-subspace/src/tests.rs +++ b/crates/pallet-subspace/src/tests.rs @@ -605,6 +605,7 @@ fn vote_block_listed() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -631,6 +632,7 @@ fn vote_after_genesis() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -661,6 +663,7 @@ fn vote_too_low_height() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -691,6 +694,7 @@ fn vote_past_future_height() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -711,6 +715,7 @@ fn vote_past_future_height() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -740,6 +745,7 @@ fn vote_wrong_parent() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -779,6 +785,7 @@ fn vote_past_future_slot() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -803,6 +810,7 @@ fn vote_past_future_slot() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -826,6 +834,7 @@ fn vote_past_future_slot() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -866,6 +875,7 @@ fn vote_same_slot() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -885,6 +895,7 @@ fn vote_same_slot() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -914,6 +925,7 @@ fn vote_bad_reward_signature() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -944,6 +956,7 @@ fn vote_unknown_segment_commitment() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -977,6 +990,7 @@ fn vote_outside_of_solution_range() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -988,6 +1002,47 @@ fn vote_outside_of_solution_range() { }); } +#[test] +fn vote_solution_quality_too_high() { + new_test_ext(allow_all_pot_extension()).execute_with(|| { + let keypair = Keypair::generate(); + let archived_segment = create_archived_segment(); + + progress_to_block(&keypair, 2, 1); + + SegmentCommitment::::insert( + archived_segment.segment_header.segment_index(), + archived_segment.segment_header.segment_commitment(), + ); + + // Reset so that any solution works for votes, but also block solution range is almost the + // same, resulting in quality being too high + pallet::SolutionRanges::::mutate(|solution_ranges| { + solution_ranges.current = u64::MAX - 1; + solution_ranges.voting_current = u64::MAX; + }); + + // Finally correct signature + let signed_vote = create_signed_vote( + &keypair, + 2, + frame_system::Pallet::::block_hash(1), + Subspace::current_slot() + 1, + Default::default(), + Default::default(), + &archived_segment.pieces, + 1, + SolutionRange::MIN, + SolutionRange::MAX, + ); + + assert_matches!( + super::check_vote::(&signed_vote, false), + Err(CheckVoteError::QualityTooHigh) + ); + }); +} + #[test] fn vote_invalid_proof_of_time() { let correct_proofs_of_time = Arc::new(Mutex::new(Vec::new())); @@ -1038,6 +1093,7 @@ fn vote_invalid_proof_of_time() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1065,6 +1121,7 @@ fn vote_invalid_proof_of_time() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1082,6 +1139,7 @@ fn vote_invalid_proof_of_time() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1109,6 +1167,7 @@ fn vote_invalid_proof_of_time() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1136,6 +1195,7 @@ fn vote_invalid_proof_of_time() { test_future_proof_of_time, &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1172,6 +1232,7 @@ fn vote_correct_signature() { Default::default(), &archived_segment.pieces, 1, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1209,6 +1270,7 @@ fn vote_equivocation_current_block_plus_vote() { Default::default(), &archived_segment.pieces, reward_address, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1264,6 +1326,7 @@ fn vote_equivocation_parent_block_plus_vote() { Default::default(), &archived_segment.pieces, reward_address, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1329,6 +1392,7 @@ fn vote_equivocation_current_voters_duplicate() { Default::default(), &archived_segment.pieces, reward_address, + SolutionRange::MIN, SolutionRange::MAX, ); @@ -1413,6 +1477,7 @@ fn vote_equivocation_parent_voters_duplicate() { Default::default(), &archived_segment.pieces, reward_address, + SolutionRange::MIN, SolutionRange::MAX, ); diff --git a/crates/sc-consensus-subspace/src/slot_worker.rs b/crates/sc-consensus-subspace/src/slot_worker.rs index 20f424bef0..dea79fc1f2 100644 --- a/crates/sc-consensus-subspace/src/slot_worker.rs +++ b/crates/sc-consensus-subspace/src/slot_worker.rs @@ -500,16 +500,24 @@ where Ok(solution_distance) => { // If solution is of high enough quality and block pre-digest wasn't produced yet, // block reward is claimed - if maybe_pre_digest.is_none() && solution_distance <= solution_range / 2 { - info!(target: "subspace", "🚜 Claimed block at slot {slot}"); - maybe_pre_digest.replace(PreDigest::V0 { - slot, - solution, - pot_info: PreDigestPotInfo::V0 { - proof_of_time, - future_proof_of_time, - }, - }); + if solution_distance <= solution_range / 2 { + if maybe_pre_digest.is_none() { + info!(target: "subspace", "🚜 Claimed block at slot {slot}"); + maybe_pre_digest.replace(PreDigest::V0 { + slot, + solution, + pot_info: PreDigestPotInfo::V0 { + proof_of_time, + future_proof_of_time, + }, + }); + } else { + info!( + target: "subspace", + "Skipping solution that has quality sufficient for block {slot} \ + because block pre-digest was already created", + ); + } } else if !parent_header.number().is_zero() { // Not sending vote on top of genesis block since segment headers since piece // verification wouldn't be possible due to missing (for now) segment commitment diff --git a/crates/sp-consensus-subspace/src/lib.rs b/crates/sp-consensus-subspace/src/lib.rs index e735408086..9ac1629376 100644 --- a/crates/sp-consensus-subspace/src/lib.rs +++ b/crates/sp-consensus-subspace/src/lib.rs @@ -587,13 +587,14 @@ impl PotExtension { /// Consensus-related runtime interface #[runtime_interface] pub trait Consensus { - /// Verify whether solution is valid. + /// Verify whether solution is valid, returns solution distance that is `<= solution_range/2` on + /// success. fn verify_solution( &mut self, solution: WrappedSolution, slot: SlotNumber, params: WrappedVerifySolutionParams<'_>, - ) -> Result<(), String> { + ) -> Result { use sp_externalities::ExternalitiesExt; use subspace_proof_of_space::PosTableType; @@ -608,27 +609,21 @@ pub trait Consensus { .0; match pos_table_type { - PosTableType::Chia => { - subspace_verification::verify_solution::( - &solution.0, - slot, - ¶ms.0, - kzg, - ) - .map_err(|error| error.to_string())?; - } - PosTableType::Shim => { - subspace_verification::verify_solution::( - &solution.0, - slot, - ¶ms.0, - kzg, - ) - .map_err(|error| error.to_string())?; - } + PosTableType::Chia => subspace_verification::verify_solution::( + &solution.0, + slot, + ¶ms.0, + kzg, + ) + .map_err(|error| error.to_string()), + PosTableType::Shim => subspace_verification::verify_solution::( + &solution.0, + slot, + ¶ms.0, + kzg, + ) + .map_err(|error| error.to_string()), } - - Ok(()) } /// Verify whether `proof_of_time` is valid at specified `slot` if built on top of `parent_hash`