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

Introduce new system origin: DispatchedAs(origin_before, origin_after) #275

Open
shawntabrizi opened this issue Jun 3, 2022 · 6 comments
Labels
I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 3, 2022

Substrate uses a lot of dispatch and origin manipulation tricks, mostly to dispatch calls from one origin as another origin.

Unfortunately, the history of dispatched origins is lost when this kind of thing happens, and runtime developers may want to know what that history looks like to make more complex decisions.

To enable a feature like this, I suggest we introduce a new system origin: DispatchedAs(origin_before, origin_after)

This will be an new enum variant, where the dispatcher can specifically include a "before" origin and an "after" origin.

We can make this new origin completely backwards compatible by having functions like ensure_signed and ensure_root look at the origin_after item. But, now this means we can introduce functions like origin.original_origin() which would either return origin_before or the base origin itself.

Then you could have logic like:

/// This is a function which can be called by root, but not when the original origin was a user.
/// This kind of logic may help prevent a Sudo user from abusing their power on specific functions.
fn pure_sudo_only(origin) {
     ensure!(origin.original_origin() == RawOrigin::Root, DispatchError::BadOrigin);
}

While ensure_root(origin) would continue to work just fine.

@shawntabrizi
Copy link
Member Author

I wonder if we would need to worry about depth of nesting origins here.

@xlc
Copy link
Contributor

xlc commented Jun 3, 2022

I don't understand the purpose. Sudo user should have their power on everything.

@shawntabrizi
Copy link
Member Author

shawntabrizi commented Jun 3, 2022

Sudo is maybe a bad example.

Other uses:

  • when one account proxies another, and you want to know who the original caller is
  • when a user dispatches a call on behalf of a parachain origin, and some calls want to explicitly forbid such a thing
  • when a governance origin is used to dispatch some call, and you want to know who triggered that

@xlc
Copy link
Contributor

xlc commented Jun 4, 2022

I am not sure. It still sounds like a leaky abstraction to me.

How do deal with chaining / nested dispatched as origin? How do handle the case when you actually want to modify the original origin? Now the origin permission depends on the original origin and you suddenly make the number of possible states from N to N^2 and that just means more areas of potential bugs.

@shawntabrizi
Copy link
Member Author

I think the runtime is in full control here.

For example, at any point, the runtime can drop the current origin, and construct a new one, or modify either field of the dispatch_as origin. I guess the point is that someone may want to write some logic which starts with "Who originally made this call, or paid the transaction fee for this call"

And that information is lost in any proxy/dispatch scenario.

@xlc
Copy link
Contributor

xlc commented Jun 6, 2022

I guess the point is that someone may want to write some logic which starts with "Who originally made this call, or paid the transaction fee for this call"

We do have this requirement to implement tx.origin in EVM and this is how we do it
https://github.com/AcalaNetwork/Acala/blob/3c25a51339bfda1391dd9c541a8f9e70f1187812/modules/evm/src/lib.rs#L1903

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed C3-medium labels Aug 25, 2023
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Backlog
Development

No branches or pull requests

4 participants