diff --git a/polkadot/node/core/approval-voting/src/criteria.rs b/polkadot/node/core/approval-voting/src/criteria.rs index a2bd0889f034b57d83150b9b45d13d6c7b639cdb..d7bedc9b0a7fba291e4e60cbb5b555fd01da0361 100644 --- a/polkadot/node/core/approval-voting/src/criteria.rs +++ b/polkadot/node/core/approval-voting/src/criteria.rs @@ -429,16 +429,30 @@ fn compute_relay_vrf_delay_assignments( /// Assignment invalid. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct InvalidAssignment; +pub struct InvalidAssignment(pub(crate) InvalidAssignmentReason); impl std::fmt::Display for InvalidAssignment { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "Invalid Assignment") + write!(f, "Invalid Assignment: {:?}", self.0) } } impl std::error::Error for InvalidAssignment {} +/// Failure conditions when checking an assignment cert. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum InvalidAssignmentReason { + ValidatorIndexOutOfBounds, + SampleOutOfBounds, + CoreIndexOutOfBounds, + InvalidAssignmentKey, + IsInBackingGroup, + VRFModuloCoreIndexMismatch, + VRFModuloOutputMismatch, + VRFDelayCoreIndexMismatch, + VRFDelayOutputMismatch, +} + /// Checks the crypto of an assignment cert. Failure conditions: /// * Validator index out of bounds /// * VRF signature check fails @@ -458,16 +472,18 @@ pub(crate) fn check_assignment_cert( assignment: &AssignmentCert, backing_group: GroupIndex, ) -> Result<DelayTranche, InvalidAssignment> { + use InvalidAssignmentReason as Reason; + let validator_public = config .assignment_keys .get(validator_index.0 as usize) - .ok_or(InvalidAssignment)?; + .ok_or(InvalidAssignment(Reason::ValidatorIndexOutOfBounds))?; let public = schnorrkel::PublicKey::from_bytes(validator_public.as_slice()) - .map_err(|_| InvalidAssignment)?; + .map_err(|_| InvalidAssignment(Reason::InvalidAssignmentKey))?; if claimed_core_index.0 >= config.n_cores { - return Err(InvalidAssignment) + return Err(InvalidAssignment(Reason::CoreIndexOutOfBounds)) } // Check that the validator was not part of the backing group @@ -476,14 +492,14 @@ pub(crate) fn check_assignment_cert( is_in_backing_group(&config.validator_groups, validator_index, backing_group); if is_in_backing { - return Err(InvalidAssignment) + return Err(InvalidAssignment(Reason::IsInBackingGroup)) } let &(ref vrf_output, ref vrf_proof) = &assignment.vrf; match assignment.kind { AssignmentCertKind::RelayVRFModulo { sample } => { if sample >= config.relay_vrf_modulo_samples { - return Err(InvalidAssignment) + return Err(InvalidAssignment(Reason::SampleOutOfBounds)) } let (vrf_in_out, _) = public @@ -493,18 +509,18 @@ pub(crate) fn check_assignment_cert( &vrf_proof.0, assigned_core_transcript(claimed_core_index), ) - .map_err(|_| InvalidAssignment)?; + .map_err(|_| InvalidAssignment(Reason::VRFModuloOutputMismatch))?; // ensure that the `vrf_in_out` actually gives us the claimed core. if relay_vrf_modulo_core(&vrf_in_out, config.n_cores) == claimed_core_index { Ok(0) } else { - Err(InvalidAssignment) + Err(InvalidAssignment(Reason::VRFModuloCoreIndexMismatch)) } }, AssignmentCertKind::RelayVRFDelay { core_index } => { if core_index != claimed_core_index { - return Err(InvalidAssignment) + return Err(InvalidAssignment(Reason::VRFDelayCoreIndexMismatch)) } let (vrf_in_out, _) = public @@ -513,7 +529,7 @@ pub(crate) fn check_assignment_cert( &vrf_output.0, &vrf_proof.0, ) - .map_err(|_| InvalidAssignment)?; + .map_err(|_| InvalidAssignment(Reason::VRFDelayOutputMismatch))?; Ok(relay_vrf_delay_tranche( &vrf_in_out, diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index c3892a160eeffeb32f15394d9db8638b8f9d80b9..a67cb4bbaa9ae93c6f6a2d16c81961a19311d7ed 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -1595,10 +1595,11 @@ fn check_and_import_assignment( ); let tranche = match res { - Err(crate::criteria::InvalidAssignment) => + Err(crate::criteria::InvalidAssignment(reason)) => return Ok(( AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert( assignment.validator, + format!("{:?}", reason), )), Vec::new(), )), diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index 165b66828ae68eb7cfa140c4ae21ce8417b3252d..9199da4f980e68332fb8d9fbbdcfcbb9681a7a1f 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -1007,8 +1007,11 @@ fn subsystem_rejects_bad_assignment_ok_criteria() { #[test] fn subsystem_rejects_bad_assignment_err_criteria() { - let assignment_criteria = - Box::new(MockAssignmentCriteria::check_only(move |_| Err(criteria::InvalidAssignment))); + let assignment_criteria = Box::new(MockAssignmentCriteria::check_only(move |_| { + Err(criteria::InvalidAssignment( + criteria::InvalidAssignmentReason::ValidatorIndexOutOfBounds, + )) + })); let config = HarnessConfigBuilder::default().assignment_criteria(assignment_criteria).build(); test_harness(config, |test_harness| async move { let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } = @@ -1045,7 +1048,10 @@ fn subsystem_rejects_bad_assignment_err_criteria() { assert_eq!( rx.await, - Ok(AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert(ValidatorIndex(0)))), + Ok(AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert( + ValidatorIndex(0), + "ValidatorIndexOutOfBounds".to_string(), + ))), ); virtual_overseer @@ -2813,7 +2819,9 @@ fn pre_covers_dont_stall_approval() { move |validator_index| match validator_index { ValidatorIndex(0 | 1) => Ok(0), ValidatorIndex(2) => Ok(1), - ValidatorIndex(_) => Err(criteria::InvalidAssignment), + ValidatorIndex(_) => Err(criteria::InvalidAssignment( + criteria::InvalidAssignmentReason::ValidatorIndexOutOfBounds, + )), }, )); diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index b4eef7c8c199ca7c7f41a51342a3436b2a8e762b..9394416d32f37db5627ea9146ea6382b48a577cd 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -810,8 +810,8 @@ pub enum AssignmentCheckError { InvalidCandidateIndex(CandidateIndex), #[error("Invalid candidate {0}: {1:?}")] InvalidCandidate(CandidateIndex, CandidateHash), - #[error("Invalid cert: {0:?}")] - InvalidCert(ValidatorIndex), + #[error("Invalid cert: {0:?}, reason: {1}")] + InvalidCert(ValidatorIndex, String), #[error("Internal state mismatch: {0:?}, {1:?}")] Internal(Hash, CandidateHash), }