From 597ea9203ad57d48b766fa7fb1fcc1d388118cb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de>
Date: Wed, 27 Mar 2024 23:02:37 +0000
Subject: [PATCH] pallet-scheduler: Unrequest call on failed lookup (#3849)

When the scheduler fails to lookup a `call`, it should unrequest it,
because it will not be required anymore.
---
 prdoc/pr_3849.prdoc                    | 13 +++++++++++++
 substrate/frame/scheduler/src/lib.rs   | 11 +++++++++++
 substrate/frame/scheduler/src/tests.rs |  4 ++++
 3 files changed, 28 insertions(+)
 create mode 100644 prdoc/pr_3849.prdoc

diff --git a/prdoc/pr_3849.prdoc b/prdoc/pr_3849.prdoc
new file mode 100644
index 000000000000..a1372b60ffc6
--- /dev/null
+++ b/prdoc/pr_3849.prdoc
@@ -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
diff --git a/substrate/frame/scheduler/src/lib.rs b/substrate/frame/scheduler/src/lib.rs
index 62417b8d2cc2..a53742679e02 100644
--- a/substrate/frame/scheduler/src/lib.rs
+++ b/substrate/frame/scheduler/src/lib.rs
@@ -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)))
 			},
 		};
diff --git a/substrate/frame/scheduler/src/tests.rs b/substrate/frame/scheduler/src/tests.rs
index bb02320ad751..440355336396 100644
--- a/substrate/frame/scheduler/src/tests.rs
+++ b/substrate/frame/scheduler/src/tests.rs
@@ -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);
@@ -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));
 	});
 }