referendum - commitment salt max size

runtime-modules/referendum/src/lib.rs

@@ -122,6 +122,9 @@ pub trait Trait<I: Instance>: system::Trait /* + ReferendumManager<Self, I>*/ {
     /// Maximum number of options in one referendum.
     type MaxReferendumOptions: Get<u64>;
 
+    /// Maximum length of vote commitment salt.
+    type MaxSaltLength: Get<u64>;
+
     /// Currency for referendum staking.
     type Currency: LockableCurrency<Self::AccountId, Moment = Self::BlockNumber>;
 
@@ -248,6 +251,9 @@ decl_error! {
 
         /// Invalid time to release the locked stake
         InvalidTimeToRelease,
+
+        /// Salt is too long
+        SaltTooLong,
     }
 }
 
@@ -710,6 +716,11 @@ impl<T: Trait<I>, I: Instance> EnsureChecks<T, I> {
             return Err(Error::InvalidVote);
         }
 
+        // ensure salt is not too long
+        if salt.len() as u64 > T::MaxSaltLength::get() {
+            return Err(Error::SaltTooLong);
+        }
+
         // ensure commitment corresponds to salt and vote option
         let commitment = R::calculate_commitment(&account_id, salt, &cycle_id, vote_option_index);
         if commitment != cast_vote.commitment {

runtime-modules/referendum/src/mock.rs

@@ -45,6 +45,7 @@ pub struct Instance0;
 
 parameter_types! {
     pub const MaxReferendumOptions: u64 = 10;
+    pub const MaxSaltLength: u64 = 32; // use some multiple of 8 for ez testing
     pub const VoteStageDuration: u64 = 5;
     pub const RevealStageDuration: u64 = 5;
     pub const MinimumStake: u64 = 10000;
@@ -59,6 +60,7 @@ impl Trait<Instance0> for Runtime {
     type Event = TestEvent;
 
     type MaxReferendumOptions = MaxReferendumOptions;
+    type MaxSaltLength = MaxSaltLength;
 
     type Currency = pallet_balances::Module<Runtime>;
     type LockId = LockId;
@@ -284,16 +286,36 @@ where
         vote_option_index: &u64,
     ) -> (T::Hash, Vec<u8>) {
         let cycle_id = CurrentCycleId::<I>::get();
-        Self::calculate_commitment_for_cycle(account_id, &cycle_id, vote_option_index)
+        Self::calculate_commitment_for_cycle(account_id, &cycle_id, vote_option_index, None)
+    }
+
+    pub fn calculate_commitment_custom_salt(
+        account_id: &<T as system::Trait>::AccountId,
+        vote_option_index: &u64,
+        custom_salt: &[u8],
+    ) -> (T::Hash, Vec<u8>) {
+        let cycle_id = CurrentCycleId::<I>::get();
+        Self::calculate_commitment_for_cycle(account_id, &cycle_id, vote_option_index, Some(custom_salt))
+    }
+
+    pub fn generate_salt() -> Vec<u8> {
+        let mut rng = rand::thread_rng();
+
+        rng.gen::<u64>().to_be_bytes().to_vec()
     }
 
     pub fn calculate_commitment_for_cycle(
         account_id: &<T as system::Trait>::AccountId,
         cycle_id: &u64,
         vote_option_index: &u64,
+        custom_salt: Option<&[u8]>,
     ) -> (T::Hash, Vec<u8>) {
-        let mut rng = rand::thread_rng();
-        let salt = rng.gen::<u64>().to_be_bytes().to_vec();
+
+        let salt = match custom_salt {
+            Some(tmp_salt) => tmp_salt.to_vec(),
+            None => Self::generate_salt(),
+        };
+
         (
             <Module<T, I> as ReferendumManager<T, I>>::calculate_commitment(
                 account_id,
@@ -301,7 +323,7 @@ where
                 cycle_id,
                 vote_option_index,
             ),
-            salt,
+            salt.to_vec(),
         )
     }
 }

runtime-modules/referendum/src/tests.rs

@@ -345,6 +345,48 @@ fn reveal_no_vote() {
     });
 }
 
+/// Test that salt used to calculate commitment isn't too long.
+#[test]
+fn reveal_salt_too_long() {
+    let config = default_genesis_config();
+
+    build_test_externalities(config).execute_with(|| {
+        let max_salt_length = <Runtime as Trait<Instance0>>::MaxSaltLength::get();
+        let voting_stage_duration = <Runtime as Trait<Instance0>>::VoteStageDuration::get();
+        let account_id = USER_ADMIN;
+        let origin = OriginType::Signed(account_id);
+        let options = 3;
+        let winning_target_count = 1;
+
+        let mut salt = vec![];
+        for _ in 0..(max_salt_length / 8 + 1) {
+            salt.append(&mut MockUtils::generate_salt());
+        }
+
+        let option_to_vote_for = 1;
+        let stake = <Runtime as Trait<Instance0>>::MinimumStake::get();
+        let (commitment, _) = MockUtils::calculate_commitment_custom_salt(&account_id, &option_to_vote_for, &salt);
+
+        Mocks::start_referendum_extrinsic(
+            origin.clone(),
+            options.clone(),
+            winning_target_count,
+            Ok(()),
+        );
+        Mocks::vote(origin.clone(), account_id, commitment, stake, Ok(()));
+        MockUtils::increase_block_number(voting_stage_duration + 1);
+
+        Mocks::check_voting_finished(options, winning_target_count);
+        Mocks::reveal_vote(
+            origin.clone(),
+            account_id,
+            salt,
+            option_to_vote_for,
+            Err(Error::SaltTooLong),
+        );
+    });
+}
+
 /// Test that revealing of a vote for a not-existing option is rejected.
 #[test]
 fn reveal_invalid_vote() {