From 97ac45c2f22026430a3f464dbdec5ad091c39896 Mon Sep 17 00:00:00 2001
From: Shawn Tabrizi <shawntabrizi@gmail.com>
Date: Wed, 5 Aug 2020 16:22:21 +0200
Subject: [PATCH] Successful `note_imminent_preimage` is free (#6793)

* Successful `note_imminent_preimage` is free

* update docs

* Add test for duplicate preimage
---
 substrate/frame/democracy/src/lib.rs          | 20 ++++++++----
 .../frame/democracy/src/tests/preimage.rs     | 31 +++++++++++++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs
index f3a5960eb2f..e298b1e4508 100644
--- a/substrate/frame/democracy/src/lib.rs
+++ b/substrate/frame/democracy/src/lib.rs
@@ -160,7 +160,7 @@ use sp_runtime::{
 use codec::{Encode, Decode, Input};
 use frame_support::{
 	decl_module, decl_storage, decl_event, decl_error, ensure, Parameter,
-	weights::{Weight, DispatchClass},
+	weights::{Weight, DispatchClass, Pays},
 	traits::{
 		Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get,
 		OnUnbalanced, BalanceStatus, schedule::{Named as ScheduleNamed, DispatchTime}, EnsureOrigin
@@ -458,14 +458,14 @@ decl_event! {
 		Vetoed(AccountId, Hash, BlockNumber),
 		/// A proposal's preimage was noted, and the deposit taken. [proposal_hash, who, deposit]
 		PreimageNoted(Hash, AccountId, Balance),
-		/// A proposal preimage was removed and used (the deposit was returned). 
+		/// A proposal preimage was removed and used (the deposit was returned).
 		/// [proposal_hash, provider, deposit]
 		PreimageUsed(Hash, AccountId, Balance),
 		/// A proposal could not be executed because its preimage was invalid. [proposal_hash, ref_index]
 		PreimageInvalid(Hash, ReferendumIndex),
 		/// A proposal could not be executed because its preimage was missing. [proposal_hash, ref_index]
 		PreimageMissing(Hash, ReferendumIndex),
-		/// A registered preimage was removed and the deposit collected by the reaper. 
+		/// A registered preimage was removed and the deposit collected by the reaper.
 		/// [proposal_hash, provider, deposit, reaper]
 		PreimageReaped(Hash, AccountId, Balance, AccountId),
 		/// An [account] has been unlocked successfully.
@@ -1000,7 +1000,9 @@ decl_module! {
 		}
 
 		/// Register the preimage for an upcoming proposal. This requires the proposal to be
-		/// in the dispatch queue. No deposit is needed.
+		/// in the dispatch queue. No deposit is needed. When this call is successful, i.e.
+		/// the preimage has not been uploaded before and matches some imminent proposal,
+		/// no fee is paid.
 		///
 		/// The dispatch origin of this call must be _Signed_.
 		///
@@ -1014,8 +1016,11 @@ decl_module! {
 		/// - Db writes: `Preimages`
 		/// # </weight>
 		#[weight = T::WeightInfo::note_imminent_preimage(encoded_proposal.len() as u32)]
-		fn note_imminent_preimage(origin, encoded_proposal: Vec<u8>) {
+		fn note_imminent_preimage(origin, encoded_proposal: Vec<u8>) -> DispatchResultWithPostInfo {
 			Self::note_imminent_preimage_inner(ensure_signed(origin)?, encoded_proposal)?;
+			// We check that this preimage was not uploaded before in `note_imminent_preimage_inner`,
+			// thus this call can only be successful once. If successful, user does not pay a fee.
+			Ok(Pays::No.into())
 		}
 
 		/// Same as `note_imminent_preimage` but origin is `OperationalPreimageOrigin`.
@@ -1023,9 +1028,12 @@ decl_module! {
 			T::WeightInfo::note_imminent_preimage(encoded_proposal.len() as u32),
 			DispatchClass::Operational,
 		)]
-		fn note_imminent_preimage_operational(origin, encoded_proposal: Vec<u8>) {
+		fn note_imminent_preimage_operational(origin, encoded_proposal: Vec<u8>) -> DispatchResultWithPostInfo {
 			let who = T::OperationalPreimageOrigin::ensure_origin(origin)?;
 			Self::note_imminent_preimage_inner(who, encoded_proposal)?;
+			// We check that this preimage was not uploaded before in `note_imminent_preimage_inner`,
+			// thus this call can only be successful once. If successful, user does not pay a fee.
+			Ok(Pays::No.into())
 		}
 
 		/// Remove an expired proposal preimage and collect the deposit.
diff --git a/substrate/frame/democracy/src/tests/preimage.rs b/substrate/frame/democracy/src/tests/preimage.rs
index 4100a6a6b63..8a2cbaf5340 100644
--- a/substrate/frame/democracy/src/tests/preimage.rs
+++ b/substrate/frame/democracy/src/tests/preimage.rs
@@ -164,3 +164,34 @@ fn reaping_imminent_preimage_should_fail() {
 		assert_noop!(Democracy::reap_preimage(Origin::signed(6), h, u32::max_value()), Error::<Test>::Imminent);
 	});
 }
+
+#[test]
+fn note_imminent_preimage_can_only_be_successful_once() {
+	new_test_ext().execute_with(|| {
+		PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
+
+		let r = Democracy::inject_referendum(
+			2,
+			set_balance_proposal_hash(2),
+			VoteThreshold::SuperMajorityApprove,
+			1
+		);
+		assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));
+		next_block();
+
+		// First time works
+		assert_ok!(Democracy::note_imminent_preimage(Origin::signed(6), set_balance_proposal(2)));
+
+		// Second time fails
+		assert_noop!(
+			Democracy::note_imminent_preimage(Origin::signed(6), set_balance_proposal(2)),
+			Error::<Test>::DuplicatePreimage
+		);
+
+		// Fails from any user
+		assert_noop!(
+			Democracy::note_imminent_preimage(Origin::signed(5), set_balance_proposal(2)),
+			Error::<Test>::DuplicatePreimage
+		);
+	});
+}
-- 
GitLab