Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve errors definition and docs #433

Merged
merged 29 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6d3786f
init the solution
open-junius May 17, 2024
751f834
update more errors
open-junius May 17, 2024
00c7a75
correct unit test
open-junius May 17, 2024
ac9a166
fix more unit test
open-junius May 17, 2024
894e6c7
fix unit test
open-junius May 17, 2024
8010214
update one more error
open-junius May 17, 2024
6ff738b
update one error
open-junius May 17, 2024
5cdf439
improve more errors
open-junius May 17, 2024
3f0c369
update more errors
open-junius May 17, 2024
7c18b5f
update more error
open-junius May 17, 2024
b0be095
fix unit test
open-junius May 17, 2024
f62af27
more error change
open-junius May 17, 2024
32adeca
fix fmt
open-junius May 17, 2024
7b58ba3
more error update
open-junius May 17, 2024
f0a2f36
update last one
open-junius May 17, 2024
10dcb20
Merge branch 'development' into junius/detailed-error-type
open-junius May 17, 2024
44ac0a9
Merge branch 'development' into junius/detailed-error-type
open-junius May 21, 2024
4407df4
remove unused method
open-junius May 21, 2024
d3b07a0
update error docs
open-junius May 22, 2024
ca5dfe6
fix one comment
open-junius May 22, 2024
0632431
fix one comment
open-junius May 22, 2024
481cf61
fix one comment
open-junius May 22, 2024
ef16b6d
fix one comment
open-junius May 22, 2024
8d3f182
fix one comment
open-junius May 22, 2024
4129c8a
fix one comment
open-junius May 22, 2024
6bf9072
fix one more comment
open-junius May 22, 2024
d3de3fd
update test comment
open-junius May 23, 2024
07d8f77
fix last comment
open-junius May 23, 2024
4f8f62e
Merge branch 'development' into junius/detailed-error-type
open-junius May 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions pallets/admin-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 max allowed validator number to be set is larger than max allowed UIDs
MaxValidatorsLargerThanMaxUIds,
/// The maximum allowed UIDs is less than UIDs already in the subnet
MaxAllowedUIdsLessThanCurrentUIds,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
}

/// Dispatchable functions allows users to interact with the pallet and invoke state changes.
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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);
Expand Down
89 changes: 35 additions & 54 deletions pallets/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// 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,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// The given length-bound for the proposal was too low.
ProposalLengthBoundLessThanProposalLength,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// The given motion duration for the proposal was too low.
WrongDuration,
DurationLowerThanConfiguredMotionDuration,
}

// Note that councillor operations are assigned to the operational class.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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())
})?;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 12 additions & 12 deletions pallets/collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ fn close_works() {
proposal_weight,
proposal_len
),
Error::<Test, Instance1>::TooEarly
Error::<Test, Instance1>::TooEarlyToCloseProposal
);

System::set_block_number(4);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
);
})
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
);
});
}
Expand Down Expand Up @@ -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
// Expecting error "ProposalNotExists" with Pays::Yes
open-junius marked this conversation as resolved.
Show resolved Hide resolved
let close_rval: DispatchResultWithPostInfo = Collective::close(
RuntimeOrigin::signed(2),
hash,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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));
Expand Down
16 changes: 8 additions & 8 deletions pallets/commitments/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// Account is trying to commit data too fast, rate limit exceeded
CommitmentSetRateLimitExceeded,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
}

/// Identity data by account
Expand Down Expand Up @@ -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
);
}

Expand Down
10 changes: 3 additions & 7 deletions pallets/registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
open-junius marked this conversation as resolved.
Show resolved Hide resolved
/// Account passed too many additional fields to their identity
TooManyFields,
TooManyFieldsInIdentityInfo,
/// Account doesn't have a registered identity
NotRegistered,
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Loading
Loading