Skip to content

Commit

Permalink
paras: do not allow PVF vote submission if disabled (paritytech#4684)
Browse files Browse the repository at this point in the history
if the PVF pre-checking is disabled the runtime dispatchable will reject
any attempts of submission. This is also concern the unsigned tx
validation.

Right now, the `include_pvf_check_statement` dispatchable is effectively
uncallable because of the weight set to the maximum value. If we were to
benchmark it, it would become includable in a block, but since there
will be no active votes, the dispatchable won't do anything.

However, it will execute some code, like signature validation and
querying some storage entries. To be completely safe, we can bail out
early if the `pvf_checking_enabled` config is disabled. That's what this
PR does.
  • Loading branch information
pepyakin authored and Wizdave97 committed Feb 3, 2022
1 parent b511b16 commit 05ea55a
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
15 changes: 15 additions & 0 deletions runtime/parachains/src/paras/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,9 @@ pub mod pallet {
PvfCheckDoubleVote,
/// The given PVF does not exist at the moment of process a vote.
PvfCheckSubjectInvalid,
/// The PVF pre-checking statement cannot be included since the PVF pre-checking mechanism
/// is disabled.
PvfCheckDisabled,
}

/// All currently active PVF pre-checking votes.
Expand Down Expand Up @@ -875,6 +878,13 @@ pub mod pallet {
signature: ValidatorSignature,
) -> DispatchResult {
ensure_none(origin)?;

// Make sure that PVF pre-checking is enabled.
ensure!(
configuration::Pallet::<T>::config().pvf_checking_enabled,
Error::<T>::PvfCheckDisabled,
);

let validators = shared::Pallet::<T>::active_validator_keys();
let current_session = shared::Pallet::<T>::session_index();
if stmt.session_index < current_session {
Expand Down Expand Up @@ -957,6 +967,10 @@ pub mod pallet {
_ => return InvalidTransaction::Call.into(),
};

if !configuration::Pallet::<T>::config().pvf_checking_enabled {
return InvalidTransaction::Custom(INVALID_TX_PVF_CHECK_DISABLED).into()
}

let current_session = shared::Pallet::<T>::session_index();
if stmt.session_index < current_session {
return InvalidTransaction::Stale.into()
Expand Down Expand Up @@ -1017,6 +1031,7 @@ pub mod pallet {
const INVALID_TX_BAD_VALIDATOR_IDX: u8 = 1;
const INVALID_TX_BAD_SUBJECT: u8 = 2;
const INVALID_TX_DOUBLE_VOTE: u8 = 3;
const INVALID_TX_PVF_CHECK_DISABLED: u8 = 4;

impl<T: Config> Pallet<T> {
/// Called by the initializer to initialize the paras pallet.
Expand Down
41 changes: 41 additions & 0 deletions runtime/parachains/src/paras/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,47 @@ fn pvf_check_upgrade_reject() {
});
}

#[test]
fn pvf_check_submit_vote_while_disabled() {
let genesis_config = MockGenesisConfig {
configuration: crate::configuration::GenesisConfig {
config: HostConfiguration { pvf_checking_enabled: false, ..Default::default() },
..Default::default()
},
..Default::default()
};

new_test_ext(genesis_config).execute_with(|| {
// This will set the session index to 1 and seed the validators.
run_to_block(1, Some(vec![1]));

let stmt = PvfCheckStatement {
accept: false,
subject: ValidationCode(vec![1, 2, 3]).hash(),
session_index: 1,
validator_index: 1.into(),
};

let signature: ValidatorSignature =
Sr25519Keyring::Alice.sign(&stmt.signing_payload()).into();

let call =
Call::include_pvf_check_statement { stmt: stmt.clone(), signature: signature.clone() };

let validate_unsigned =
<Paras as ValidateUnsigned>::validate_unsigned(TransactionSource::InBlock, &call);
assert_eq!(
validate_unsigned,
InvalidTransaction::Custom(INVALID_TX_PVF_CHECK_DISABLED).into()
);

assert_err!(
Paras::include_pvf_check_statement(None.into(), stmt.clone(), signature.clone()),
Error::<Test>::PvfCheckDisabled
);
});
}

#[test]
fn pvf_check_submit_vote() {
let code_a: ValidationCode = vec![3, 2, 1].into();
Expand Down

0 comments on commit 05ea55a

Please sign in to comment.