Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

parachains-runtime: Less cloning! #6896

Merged
merged 3 commits into from
Mar 16, 2023
Merged

parachains-runtime: Less cloning! #6896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6586c5
parachains-runtime: Less cloning!
bkchr Mar 16, 2023
e0aa54a
Fix tests
bkchr Mar 16, 2023
292d2cb
FMT
bkchr Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions runtime/parachains/src/disputes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ pub trait DisputesHandler<BlockNumber: Ord> {
/// Handle sets of dispute statements corresponding to 0 or more candidates.
/// Returns a vector of freshly created disputes.
fn process_checked_multi_dispute_data(
statement_sets: CheckedMultiDisputeStatementSet,
statement_sets: &CheckedMultiDisputeStatementSet,
) -> Result<Vec<(SessionIndex, CandidateHash)>, DispatchError>;

/// Note that the given candidate has been included.
Expand Down Expand Up @@ -325,7 +325,7 @@ impl<BlockNumber: Ord> DisputesHandler<BlockNumber> for () {
}

fn process_checked_multi_dispute_data(
_statement_sets: CheckedMultiDisputeStatementSet,
_statement_sets: &CheckedMultiDisputeStatementSet,
) -> Result<Vec<(SessionIndex, CandidateHash)>, DispatchError> {
Ok(Vec::new())
}
Expand Down Expand Up @@ -379,7 +379,7 @@ where
}

fn process_checked_multi_dispute_data(
statement_sets: CheckedMultiDisputeStatementSet,
statement_sets: &CheckedMultiDisputeStatementSet,
) -> Result<Vec<(SessionIndex, CandidateHash)>, DispatchError> {
pallet::Pallet::<T>::process_checked_multi_dispute_data(statement_sets)
}
Expand Down Expand Up @@ -983,14 +983,14 @@ impl<T: Config> Pallet<T> {
/// and to fail the extrinsic on error. As invalid inherents are not allowed, the dirty state
/// is not committed.
pub(crate) fn process_checked_multi_dispute_data(
statement_sets: CheckedMultiDisputeStatementSet,
statement_sets: &CheckedMultiDisputeStatementSet,
) -> Result<Vec<(SessionIndex, CandidateHash)>, DispatchError> {
let config = <configuration::Pallet<T>>::config();

let mut fresh = Vec::with_capacity(statement_sets.len());
for statement_set in statement_sets {
let dispute_target = {
let statement_set: &DisputeStatementSet = statement_set.as_ref();
let statement_set = statement_set.as_ref();
(statement_set.session, statement_set.candidate_hash)
};
if Self::process_checked_dispute_data(
Expand Down Expand Up @@ -1126,7 +1126,7 @@ impl<T: Config> Pallet<T> {
/// Fails if the dispute data is invalid. Returns a Boolean indicating whether the
/// dispute is fresh.
fn process_checked_dispute_data(
set: CheckedDisputeStatementSet,
set: &CheckedDisputeStatementSet,
dispute_post_conclusion_acceptance_period: T::BlockNumber,
) -> Result<bool, DispatchError> {
// Dispute statement sets on any dispute which concluded
Expand Down
26 changes: 13 additions & 13 deletions runtime/parachains/src/disputes/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ fn test_dispute_timeout() {
let stmts = filter_dispute_set(stmts);

assert_ok!(
Pallet::<Test>::process_checked_multi_dispute_data(stmts),
Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
vec![(9, candidate_hash.clone())],
);

Expand Down Expand Up @@ -579,7 +579,7 @@ fn test_provide_multi_dispute_is_providing() {

assert_ok!(
Pallet::<Test>::process_checked_multi_dispute_data(
stmts
&stmts
.into_iter()
.map(CheckedDisputeStatementSet::unchecked_from_unchecked)
.collect()
Expand Down Expand Up @@ -644,7 +644,7 @@ fn test_disputes_with_missing_backing_votes_are_rejected() {
}];

assert!(Pallet::<Test>::process_checked_multi_dispute_data(
stmts
&stmts
.into_iter()
.map(CheckedDisputeStatementSet::unchecked_from_unchecked)
.collect()
Expand Down Expand Up @@ -714,7 +714,7 @@ fn test_freeze_on_note_included() {
],
}];
assert!(Pallet::<Test>::process_checked_multi_dispute_data(
stmts
&stmts
.into_iter()
.map(CheckedDisputeStatementSet::unchecked_from_unchecked)
.collect()
Expand Down Expand Up @@ -789,7 +789,7 @@ fn test_freeze_provided_against_supermajority_for_included() {

Pallet::<Test>::note_included(3, candidate_hash.clone(), 3);
assert!(Pallet::<Test>::process_checked_multi_dispute_data(
stmts
&stmts
.into_iter()
.map(CheckedDisputeStatementSet::unchecked_from_unchecked)
.collect()
Expand Down Expand Up @@ -891,7 +891,7 @@ mod unconfirmed_disputes {
let stmts = filter_dispute_set(stmts);

// Not confirmed => should be filtered out
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(stmts), vec![],);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(&stmts), vec![],);
});
}

Expand All @@ -903,7 +903,7 @@ mod unconfirmed_disputes {
let stmts = vec![CheckedDisputeStatementSet::unchecked_from_unchecked(stmts)];

assert_matches!(
Pallet::<Test>::process_checked_multi_dispute_data(stmts),
Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
Err(DispatchError::Module(ModuleError{index: _, error: _, message})) => assert_eq!(message, Some("UnconfirmedDispute"))
);

Expand Down Expand Up @@ -1011,7 +1011,7 @@ fn test_provide_multi_dispute_success_and_other() {
let stmts = filter_dispute_set(stmts);

assert_ok!(
Pallet::<Test>::process_checked_multi_dispute_data(stmts),
Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
vec![(3, candidate_hash.clone())],
);

Expand Down Expand Up @@ -1076,7 +1076,7 @@ fn test_provide_multi_dispute_success_and_other() {

let stmts = filter_dispute_set(stmts);
assert_ok!(
Pallet::<Test>::process_checked_multi_dispute_data(stmts),
Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
vec![(5, candidate_hash.clone())],
);

Expand All @@ -1098,7 +1098,7 @@ fn test_provide_multi_dispute_success_and_other() {
)],
}];
let stmts = filter_dispute_set(stmts);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(stmts), vec![]);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(&stmts), vec![]);

let stmts = vec![
// 0, 4, and 5 vote against 5
Expand Down Expand Up @@ -1177,7 +1177,7 @@ fn test_provide_multi_dispute_success_and_other() {
},
];
let stmts = filter_dispute_set(stmts);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(stmts), vec![]);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(&stmts), vec![]);

assert_eq!(
Pallet::<Test>::disputes(),
Expand Down Expand Up @@ -1386,7 +1386,7 @@ fn test_punish_post_conclusion() {

let stmts = filter_dispute_set(stmts);
assert_ok!(
Pallet::<Test>::process_checked_multi_dispute_data(stmts),
Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
vec![(session, candidate_hash)],
);

Expand Down Expand Up @@ -1429,7 +1429,7 @@ fn test_punish_post_conclusion() {
}];

let stmts = filter_dispute_set(stmts);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(stmts), vec![],);
assert_ok!(Pallet::<Test>::process_checked_multi_dispute_data(&stmts), vec![],);

// Ensure punishment for is called
assert_eq!(
Expand Down
15 changes: 6 additions & 9 deletions runtime/parachains/src/paras_inherent/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,7 @@ impl<T: Config> Pallet<T> {
// Note that `process_checked_multi_dispute_data` will iterate and import each
// dispute; so the input here must be reasonably bounded,
// which is guaranteed by the checks and weight limitation above.
let _ =
T::DisputesHandler::process_checked_multi_dispute_data(checked_disputes.clone())?;
let _ = T::DisputesHandler::process_checked_multi_dispute_data(&checked_disputes)?;
METRICS.on_disputes_imported(checked_disputes.len() as u64);

if T::DisputesHandler::is_frozen() {
Expand Down Expand Up @@ -625,13 +624,11 @@ impl<T: Config> Pallet<T> {
// we don't care about fresh or not disputes
// this writes them to storage, so let's query it via those means
// if this fails for whatever reason, that's ok
let _ = T::DisputesHandler::process_checked_multi_dispute_data(
checked_disputes_sets.clone(),
)
.map_err(|e| {
log::warn!(target: LOG_TARGET, "MultiDisputesData failed to update: {:?}", e);
e
});
let _ = T::DisputesHandler::process_checked_multi_dispute_data(&checked_disputes_sets)
.map_err(|e| {
log::warn!(target: LOG_TARGET, "MultiDisputesData failed to update: {:?}", e);
e
});

// Contains the disputes that are concluded in the current session only,
// since these are the only ones that are relevant for the occupied cores
Expand Down