Merge pull request #433 from opentensor/junius/detailed-error-type

Improve errors definition and docs

Author: open-junius

pallets/admin-utils/src/lib.rs

@@ -66,10 +66,10 @@ pub mod pallet {
     pub enum Error<T> {
         /// The subnet does not exist, check the netuid parameter
         SubnetDoesNotExist,
-        /// The max allowed validator number to be set is larger than threshold
-        StorageValueOutOfRange,
-        /// The maximum allowed UIDs is out of boundary, it not allowed
-        MaxAllowedUIdsNotAllowed,
+        /// The maximum number of subnet validators must be less than the maximum number of allowed UIDs in the subnet.
+        MaxValidatorsLargerThanMaxUIds,
+        /// The maximum number of subnet validators must be more than the current number of UIDs already in the subnet.
+        MaxAllowedUIdsLessThanCurrentUIds,
     }
 
     /// Dispatchable functions allows users to interact with the pallet and invoke state changes.
@@ -385,7 +385,7 @@ pub mod pallet {
             );
             ensure!(
                 T::Subtensor::get_subnetwork_n(netuid) < max_allowed_uids,
-                Error::<T>::MaxAllowedUIdsNotAllowed
+                Error::<T>::MaxAllowedUIdsLessThanCurrentUIds
             );
             T::Subtensor::set_max_allowed_uids(netuid, max_allowed_uids);
             log::info!(
@@ -625,7 +625,7 @@ pub mod pallet {
             );
             ensure!(
                 max_allowed_validators <= T::Subtensor::get_max_allowed_uids(netuid),
-                Error::<T>::StorageValueOutOfRange
+                Error::<T>::MaxValidatorsLargerThanMaxUIds
             );
 
             T::Subtensor::set_max_allowed_validators(netuid, max_allowed_validators);

pallets/collective/src/lib.rs

@@ -367,23 +367,21 @@ pub mod pallet {
         /// Duplicate proposals not allowed
         DuplicateProposal,
         /// Proposal must exist
-        ProposalMissing,
-        /// Mismatched index
-        WrongIndex,
+        ProposalNotExists,
+        /// Index mismatched the proposal hash
+        IndexMismatchProposalHash,
         /// Duplicate vote ignored
         DuplicateVote,
-        /// Members are already initialized.
-        AlreadyInitialized,
-        /// The close call was made too early, before the end of the voting.
-        TooEarly,
+        /// The call to close the proposal was made too early, before the end of the voting
+        TooEarlyToCloseProposal,
         /// There can only be a maximum of `MaxProposals` active proposals.
-        TooManyProposals,
-        /// The given weight bound for the proposal was too low.
-        WrongProposalWeight,
-        /// The given length bound for the proposal was too low.
-        WrongProposalLength,
+        TooManyActiveProposals,
+        /// The given weight-bound for the proposal was too low.
+        ProposalWeightLessThanDispatchCallWeight,
+        /// The given length-bound for the proposal was too low.
+        ProposalLengthBoundLessThanProposalLength,
         /// The given motion duration for the proposal was too low.
-        WrongDuration,
+        DurationLowerThanConfiguredMotionDuration,
     }
 
     // Note that councillor operations are assigned to the operational class.
@@ -489,7 +487,7 @@ pub mod pallet {
             let proposal_len = proposal.encoded_size();
             ensure!(
                 proposal_len <= length_bound as usize,
-                Error::<T, I>::WrongProposalLength
+                Error::<T, I>::ProposalLengthBoundLessThanProposalLength
             );
 
             let proposal_hash = T::Hashing::hash_of(&proposal);
@@ -544,7 +542,7 @@ pub mod pallet {
 
             ensure!(
                 duration >= T::MotionDuration::get(),
-                Error::<T, I>::WrongDuration
+                Error::<T, I>::DurationLowerThanConfiguredMotionDuration
             );
 
             let threshold = (T::GetVotingMembers::get_count() / 2) + 1;
@@ -689,32 +687,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         Self::members().contains(who)
     }
 
-    /// Execute immediately when adding a new proposal.
-    pub fn do_propose_execute(
-        proposal: Box<<T as Config<I>>::Proposal>,
-        length_bound: MemberCount,
-    ) -> Result<(u32, DispatchResultWithPostInfo), DispatchError> {
-        let proposal_len = proposal.encoded_size();
-        ensure!(
-            proposal_len <= length_bound as usize,
-            Error::<T, I>::WrongProposalLength
-        );
-
-        let proposal_hash = T::Hashing::hash_of(&proposal);
-        ensure!(
-            !<ProposalOf<T, I>>::contains_key(proposal_hash),
-            Error::<T, I>::DuplicateProposal
-        );
-
-        let seats = Self::members().len() as MemberCount;
-        let result = proposal.dispatch(RawOrigin::Members(1, seats).into());
-        Self::deposit_event(Event::Executed {
-            proposal_hash,
-            result: result.map(|_| ()).map_err(|e| e.error),
-        });
-        Ok((proposal_len as u32, result))
-    }
-
     /// Add a new proposal to be voted.
     pub fn do_propose_proposed(
         who: T::AccountId,
@@ -726,7 +698,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         let proposal_len = proposal.encoded_size();
         ensure!(
             proposal_len <= length_bound as usize,
-            Error::<T, I>::WrongProposalLength
+            Error::<T, I>::ProposalLengthBoundLessThanProposalLength
         );
 
         let proposal_hash = T::Hashing::hash_of(&proposal);
@@ -739,7 +711,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
             <Proposals<T, I>>::try_mutate(|proposals| -> Result<usize, DispatchError> {
                 proposals
                     .try_push(proposal_hash)
-                    .map_err(|_| Error::<T, I>::TooManyProposals)?;
+                    .map_err(|_| Error::<T, I>::TooManyActiveProposals)?;
                 Ok(proposals.len())
             })?;
 
@@ -775,8 +747,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         index: ProposalIndex,
         approve: bool,
     ) -> Result<bool, DispatchError> {
-        let mut voting = Self::voting(proposal).ok_or(Error::<T, I>::ProposalMissing)?;
-        ensure!(voting.index == index, Error::<T, I>::WrongIndex);
+        let mut voting = Self::voting(proposal).ok_or(Error::<T, I>::ProposalNotExists)?;
+        ensure!(
+            voting.index == index,
+            Error::<T, I>::IndexMismatchProposalHash
+        );
 
         let position_yes = voting.ayes.iter().position(|a| a == &who);
         let position_no = voting.nays.iter().position(|a| a == &who);
@@ -826,8 +801,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         proposal_weight_bound: Weight,
         length_bound: u32,
     ) -> DispatchResultWithPostInfo {
-        let voting = Self::voting(proposal_hash).ok_or(Error::<T, I>::ProposalMissing)?;
-        ensure!(voting.index == index, Error::<T, I>::WrongIndex);
+        let voting = Self::voting(proposal_hash).ok_or(Error::<T, I>::ProposalNotExists)?;
+        ensure!(
+            voting.index == index,
+            Error::<T, I>::IndexMismatchProposalHash
+        );
 
         let mut no_votes = voting.nays.len() as MemberCount;
         let mut yes_votes = voting.ayes.len() as MemberCount;
@@ -876,7 +854,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         // Only allow actual closing of the proposal after the voting period has ended.
         ensure!(
             frame_system::Pallet::<T>::block_number() >= voting.end,
-            Error::<T, I>::TooEarly
+            Error::<T, I>::TooEarlyToCloseProposal
         );
 
         let prime_vote = Self::prime().map(|who| voting.ayes.iter().any(|a| a == &who));
@@ -939,16 +917,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         let key = ProposalOf::<T, I>::hashed_key_for(hash);
         // read the length of the proposal storage entry directly
         let proposal_len =
-            storage::read(&key, &mut [0; 0], 0).ok_or(Error::<T, I>::ProposalMissing)?;
+            storage::read(&key, &mut [0; 0], 0).ok_or(Error::<T, I>::ProposalNotExists)?;
         ensure!(
             proposal_len <= length_bound,
-            Error::<T, I>::WrongProposalLength
+            Error::<T, I>::ProposalLengthBoundLessThanProposalLength
         );
-        let proposal = ProposalOf::<T, I>::get(hash).ok_or(Error::<T, I>::ProposalMissing)?;
+        let proposal = ProposalOf::<T, I>::get(hash).ok_or(Error::<T, I>::ProposalNotExists)?;
         let proposal_weight = proposal.get_dispatch_info().weight;
         ensure!(
             proposal_weight.all_lte(weight_bound),
-            Error::<T, I>::WrongProposalWeight
+            Error::<T, I>::ProposalWeightLessThanDispatchCallWeight
         );
         Ok((proposal, proposal_len as usize))
     }
@@ -1027,8 +1005,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
         index: ProposalIndex,
         who: &T::AccountId,
     ) -> Result<bool, DispatchError> {
-        let voting = Self::voting(proposal).ok_or(Error::<T, I>::ProposalMissing)?;
-        ensure!(voting.index == index, Error::<T, I>::WrongIndex);
+        let voting = Self::voting(proposal).ok_or(Error::<T, I>::ProposalNotExists)?;
+        ensure!(
+            voting.index == index,
+            Error::<T, I>::IndexMismatchProposalHash
+        );
 
         let position_yes = voting.ayes.iter().position(|a| a == who);
         let position_no = voting.nays.iter().position(|a| a == who);

pallets/collective/src/tests.rs

@@ -305,7 +305,7 @@ fn close_works() {
                 proposal_weight,
                 proposal_len
             ),
-            Error::<Test, Instance1>::TooEarly
+            Error::<Test, Instance1>::TooEarlyToCloseProposal
         );
 
         System::set_block_number(4);
@@ -376,7 +376,7 @@ fn proposal_weight_limit_works_on_approve() {
                 proposal_weight - Weight::from_parts(100, 0),
                 proposal_len
             ),
-            Error::<Test, Instance1>::WrongProposalWeight
+            Error::<Test, Instance1>::ProposalWeightLessThanDispatchCallWeight
         );
         assert_ok!(Collective::close(
             RuntimeOrigin::signed(4),
@@ -861,7 +861,7 @@ fn limit_active_proposals() {
                 TryInto::<BlockNumberFor<Test>>::try_into(3u64)
                     .expect("convert u64 to block number.")
             ),
-            Error::<Test, Instance1>::TooManyProposals
+            Error::<Test, Instance1>::TooManyActiveProposals
         );
     })
 }
@@ -890,19 +890,19 @@ fn correct_validate_and_get_proposal() {
                 length,
                 weight
             ),
-            Error::<Test, Instance1>::ProposalMissing
+            Error::<Test, Instance1>::ProposalNotExists
         );
         assert_noop!(
             Collective::validate_and_get_proposal(&hash, length - 2, weight),
-            Error::<Test, Instance1>::WrongProposalLength
+            Error::<Test, Instance1>::ProposalLengthBoundLessThanProposalLength
         );
         assert_noop!(
             Collective::validate_and_get_proposal(
                 &hash,
                 length,
                 weight - Weight::from_parts(10, 0)
             ),
-            Error::<Test, Instance1>::WrongProposalWeight
+            Error::<Test, Instance1>::ProposalWeightLessThanDispatchCallWeight
         );
         let res = Collective::validate_and_get_proposal(&hash, length, weight);
         assert_ok!(res.clone());
@@ -964,7 +964,7 @@ fn motions_ignoring_bad_index_collective_vote_works() {
         ));
         assert_noop!(
             Collective::vote(RuntimeOrigin::signed(2), hash, 1, true),
-            Error::<Test, Instance1>::WrongIndex,
+            Error::<Test, Instance1>::IndexMismatchProposalHash,
         );
     });
 }
@@ -1117,8 +1117,8 @@ fn motions_all_first_vote_free_works() {
         );
         assert_eq!(close_rval.unwrap().pays_fee, Pays::No);
 
-        // trying to close the proposal, which is already closed.
-        // Expecting error "ProposalMissing" with Pays::Yes
+        // Trying to close the proposal, which is already closed
+        // Error: "ProposalNotExists" with Pays::Yes.
         let close_rval: DispatchResultWithPostInfo = Collective::close(
             RuntimeOrigin::signed(2),
             hash,
@@ -1454,7 +1454,7 @@ fn motion_with_no_votes_closes_with_disapproval() {
                 proposal_weight,
                 proposal_len
             ),
-            Error::<Test, Instance1>::TooEarly
+            Error::<Test, Instance1>::TooEarlyToCloseProposal
         );
 
         // Once the motion duration passes,
@@ -1508,7 +1508,7 @@ fn close_disapprove_does_not_care_about_weight_or_len() {
         // It will not close with bad weight/len information
         assert_noop!(
             Collective::close(RuntimeOrigin::signed(2), hash, 0, Weight::zero(), 0),
-            Error::<Test, Instance1>::WrongProposalLength,
+            Error::<Test, Instance1>::ProposalLengthBoundLessThanProposalLength,
         );
         assert_noop!(
             Collective::close(
@@ -1518,7 +1518,7 @@ fn close_disapprove_does_not_care_about_weight_or_len() {
                 Weight::zero(),
                 proposal_len
             ),
-            Error::<Test, Instance1>::WrongProposalWeight,
+            Error::<Test, Instance1>::ProposalWeightLessThanDispatchCallWeight,
         );
         // Now we make the proposal fail
         assert_ok!(Collective::vote(RuntimeOrigin::signed(1), hash, 0, false));

pallets/commitments/src/lib.rs

@@ -77,11 +77,11 @@ pub mod pallet {
     #[pallet::error]
     pub enum Error<T> {
         /// Account passed too many additional fields to their commitment
-        TooManyFields,
-        /// Account isn't allow to make commitments to the chain
-        CannotCommit,
-        /// Account is trying to commit data too fast
-        RateLimitExceeded,
+        TooManyFieldsInCommitmentInfo,
+        /// Account is not allow to make commitments to the chain
+        AccountNotAllowedCommit,
+        /// Account is trying to commit data too fast, rate limit exceeded
+        CommitmentSetRateLimitExceeded,
     }
 
     /// Identity data by account
@@ -126,20 +126,20 @@ pub mod pallet {
             let who = ensure_signed(origin)?;
             ensure!(
                 T::CanCommit::can_commit(netuid, &who),
-                Error::<T>::CannotCommit
+                Error::<T>::AccountNotAllowedCommit
             );
 
             let extra_fields = info.fields.len() as u32;
             ensure!(
                 extra_fields <= T::MaxFields::get(),
-                Error::<T>::TooManyFields
+                Error::<T>::TooManyFieldsInCommitmentInfo
             );
 
             let cur_block = <frame_system::Pallet<T>>::block_number();
             if let Some(last_commit) = <LastCommitment<T>>::get(netuid, &who) {
                 ensure!(
                     cur_block >= last_commit + T::RateLimit::get(),
-                    Error::<T>::RateLimitExceeded
+                    Error::<T>::CommitmentSetRateLimitExceeded
                 );
             }
 

pallets/registry/src/lib.rs

@@ -82,10 +82,10 @@ pub mod pallet {
 
     #[pallet::error]
     pub enum Error<T> {
-        /// Account attempted to register an identity but doesn't meet the requirements.
+        /// Account attempted to register an identity but does not meet the requirements.
         CannotRegister,
         /// Account passed too many additional fields to their identity
-        TooManyFields,
+        TooManyFieldsInIdentityInfo,
         /// Account doesn't have a registered identity
         NotRegistered,
     }
@@ -130,7 +130,7 @@ pub mod pallet {
             let extra_fields = info.additional.len() as u32;
             ensure!(
                 extra_fields <= T::MaxAdditionalFields::get(),
-                Error::<T>::TooManyFields
+                Error::<T>::TooManyFieldsInIdentityInfo
             );
 
             let fd = <BalanceOf<T>>::from(extra_fields) * T::FieldDeposit::get();
@@ -179,10 +179,6 @@ pub mod pallet {
             identified: T::AccountId,
         ) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin)?;
-            ensure!(
-                T::CanRegister::can_register(&who, &identified),
-                Error::<T>::CannotRegister
-            );
 
             let id = <IdentityOf<T>>::take(&identified).ok_or(Error::<T>::NotRegistered)?;
             let deposit = id.total_deposit();