From fc7496f4cf72da284ec1a880fd83a947c31373d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 27 Mar 2024 02:39:38 +0000 Subject: [PATCH 1/3] pallet-referenda: Detect incorrect pre-image length There has been a case that a referenda failed because the length given to `submit` was incorrect. The pallet can actually check the length if the pre-image already exists to ensure that these kind of issues are not happening again. --- substrate/frame/referenda/src/lib.rs | 14 ++++++++++++++ substrate/frame/referenda/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index e616056c3022..8c29ddd2a091 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -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] @@ -462,6 +464,18 @@ 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(hash) = proposal.lookup_hash() { + if let Some((l, r)) = + T::Preimages::len(&hash).and_then(|l| proposal.lookup_len().map(|r| (l, r))) + { + if l != r { + return Err(Error::::PreimageStoredWithDifferentLength.into()) + } + } + } + let track = T::Tracks::track_for(&proposal_origin).map_err(|_| Error::::NoTrack)?; let submission_deposit = Self::take_deposit(who, T::SubmissionDeposit::get())?; diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs index 8f51136de0bf..52251fcbdbee 100644 --- a/substrate/frame/referenda/src/tests.rs +++ b/substrate/frame/referenda/src/tests.rs @@ -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::::PreimageStoredWithDifferentLength + ); + }); +} From fba04b3aafd7104ce32a5965b4edb14fc14d185e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 27 Mar 2024 23:11:38 +0000 Subject: [PATCH 2/3] Improve readability --- substrate/frame/referenda/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index 8c29ddd2a091..fbe27e1a4784 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -466,13 +466,11 @@ pub mod pallet { // If the pre-image is already stored, ensure that it has the same length as given in // `proposal`. - if let Some(hash) = proposal.lookup_hash() { - if let Some((l, r)) = - T::Preimages::len(&hash).and_then(|l| proposal.lookup_len().map(|r| (l, r))) - { - if l != r { - return Err(Error::::PreimageStoredWithDifferentLength.into()) - } + 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::::PreimageStoredWithDifferentLength.into()) } } From 46a0f5d45b798b80a39287af58c80bc963d0eb15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 27 Mar 2024 23:27:29 +0000 Subject: [PATCH 3/3] PRDOC --- prdoc/pr_3850.prdoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 prdoc/pr_3850.prdoc diff --git a/prdoc/pr_3850.prdoc b/prdoc/pr_3850.prdoc new file mode 100644 index 000000000000..8f7ce16076e8 --- /dev/null +++ b/prdoc/pr_3850.prdoc @@ -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