Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Introduce frame::system::Error::InvalidOrigin #4422

Closed
wants to merge 3 commits into from

Conversation

stanislav-tkach
Copy link
Contributor

@stanislav-tkach stanislav-tkach commented Dec 17, 2019

Key points:

  • InvalidOrigin has been added to frame::system::Error.
  • EnsureOrigin trait has been moved from primitives/runtime/src/traits.rs to frame::system.

Closes #4409.

@stanislav-tkach stanislav-tkach added the A3-in_progress Pull request is in progress. No review needed at this stage. label Dec 17, 2019
@stanislav-tkach stanislav-tkach added A0-please_review Pull request needs code review. A3-in_progress Pull request is in progress. No review needed at this stage. and removed A3-in_progress Pull request is in progress. No review needed at this stage. A0-please_review Pull request needs code review. labels Dec 17, 2019
@stanislav-tkach stanislav-tkach added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 17, 2019
@@ -134,28 +134,6 @@ impl<
}
}

/// An error type that indicates that the origin is invalid.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the trait should stay here and we introduce the BadOrigin type directly here.

We already have some inherent errors that are declared by decl_error!. I would add a new one BadOrigin that implements From<BadOrigin> that is defined in this file here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some upcoming changes in my mind for the error stuff, that would conflict in the way it is implemented currently.

@@ -168,7 +168,7 @@ decl_module! {
/// # </weight>
#[weight = SimpleDispatchInfo::FixedOperational(100_000)]
fn reject_proposal(origin, #[compact] proposal_id: ProposalIndex) {
T::RejectOrigin::ensure_origin(origin).map_err(|e| Into::<&str>::into(e))?;
T::RejectOrigin::ensure_origin(origin).map_err(|e| e.as_str())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does it need to be .as_str()? can't the error be returned back directly?

Copy link
Member

@niklasad1 niklasad1 Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ensure_origin returns system::Error which needs to be converted to treasury::Error accordingly to work, so it can't be returned directly.

@bkchr
Copy link
Member

bkchr commented Dec 18, 2019

Superseded by: #4449

@bkchr bkchr closed this Dec 18, 2019
@stanislav-tkach stanislav-tkach deleted the stas-4409-bad-origin-error branch December 18, 2019 22:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce general BadOrigin runtime error
4 participants