Skip to content

Commit

Permalink
pallet-scheduler: Unrequest call on failed lookup (paritytech#3849)
Browse files Browse the repository at this point in the history
When the scheduler fails to lookup a `call`, it should unrequest it,
because it will not be required anymore.
  • Loading branch information
bkchr authored and dharjeezy committed Apr 9, 2024
1 parent a1c28d8 commit c0a0b7b
Show file tree
Hide file tree
Showing 3 changed files with 28 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
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 c0a0b7b

Please sign in to comment.