Skip to content

Commit

Permalink
Merge branch 'master' into refactor/parachain-runtime-apis
Browse files Browse the repository at this point in the history
  • Loading branch information
asiniscalchi authored Mar 28, 2024
2 parents 3aef97f + 5d314eb commit 0bee19a
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 0 deletions.
13 changes: 13 additions & 0 deletions prdoc/pr_3849.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
title: Unrequest a pre-image when it failed to execute

doc:
- audience: Runtime User
description: |
When a referenda finished the proposal will be scheduled. When it is scheduled,
the pre-image is requested. The pre-image is unrequested after the proposal
was executed. However, if the proposal failed to execute it wasn't unrequested.
Thus, it could not be removed from the on-chain state. This issue is now solved
by ensuring to unrequest the pre-image when it failed to execute.

crates:
- name: pallet-scheduler
15 changes: 15 additions & 0 deletions prdoc/pr_3850.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
title: Detect incorrect pre-image length when submitting a referenda

doc:
- audience: Runtime User
description: |
When submitting a referenda the `proposal` is passed as argument.
The `proposal` is most of the time a reference to a `pre-image` and
which also contains the length of the `pre-image`. This pull request
adds some logic to check that if the `pre-image` already exists and if
it exists, it ensures that the length is passed correctly. This prevents
that the referenda can not be executed because of a mismatch of this
length.

crates:
- name: pallet-referenda
12 changes: 12 additions & 0 deletions substrate/frame/referenda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,8 @@ pub mod pallet {
BadStatus,
/// The preimage does not exist.
PreimageNotExist,
/// The preimage is stored with a different length than the one provided.
PreimageStoredWithDifferentLength,
}

#[pallet::hooks]
Expand Down Expand Up @@ -462,6 +464,16 @@ pub mod pallet {
let proposal_origin = *proposal_origin;
let who = T::SubmitOrigin::ensure_origin(origin, &proposal_origin)?;

// If the pre-image is already stored, ensure that it has the same length as given in
// `proposal`.
if let (Some(preimage_len), Some(proposal_len)) =
(proposal.lookup_hash().and_then(|h| T::Preimages::len(&h)), proposal.lookup_len())
{
if preimage_len != proposal_len {
return Err(Error::<T, I>::PreimageStoredWithDifferentLength.into())
}
}

let track =
T::Tracks::track_for(&proposal_origin).map_err(|_| Error::<T, I>::NoTrack)?;
let submission_deposit = Self::take_deposit(who, T::SubmissionDeposit::get())?;
Expand Down
16 changes: 16 additions & 0 deletions substrate/frame/referenda/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,3 +666,19 @@ fn clear_metadata_works() {
}));
});
}

#[test]
fn detects_incorrect_len() {
ExtBuilder::default().build_and_execute(|| {
let hash = note_preimage(1);
assert_noop!(
Referenda::submit(
RuntimeOrigin::signed(1),
Box::new(RawOrigin::Root.into()),
frame_support::traits::Bounded::Lookup { hash, len: 3 },
DispatchTime::At(1),
),
Error::<Test>::PreimageStoredWithDifferentLength
);
});
}
11 changes: 11 additions & 0 deletions substrate/frame/scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,17 @@ impl<T: Config> Pallet<T> {
id: task.maybe_id,
});

// It was not available when we needed it, so we don't need to have requested it
// anymore.
T::Preimages::drop(&task.call);

// We don't know why `peek` failed, thus we most account here for the "full weight".
let _ = weight.try_consume(T::WeightInfo::service_task(
task.call.lookup_len().map(|x| x as usize),
task.maybe_id.is_some(),
task.maybe_periodic.is_some(),
));

return Err((Unavailable, Some(task)))
},
};
Expand Down
4 changes: 4 additions & 0 deletions substrate/frame/scheduler/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3008,6 +3008,8 @@ fn unavailable_call_is_detected() {

// Ensure the preimage isn't available
assert!(!Preimage::have(&bound));
// But we have requested it
assert!(Preimage::is_requested(&hash));

// Executes in block 4.
run_to_block(4);
Expand All @@ -3016,5 +3018,7 @@ fn unavailable_call_is_detected() {
System::events().last().unwrap().event,
crate::Event::CallUnavailable { task: (4, 0), id: Some(name) }.into()
);
// It should not be requested anymore.
assert!(!Preimage::is_requested(&hash));
});
}

0 comments on commit 0bee19a

Please sign in to comment.