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

Enhance the runtime API dry_run to eliminate the need for signatures and SignedExtra #4432

Closed
sekisamu opened this issue May 10, 2024 · 12 comments

Comments

@sekisamu
Copy link

As an application developer, it's essential to have simulation execution functionalities like eth_call that don't require accompanying signatures. This helps apps better protect user security and save on gas fees.

However, the system_dryRun provided by Substrate requires signatures and SignedExtra information, which can be disastrous for some apps, especially multi-signature apps.

I initially came across this issue which proposed a great idea: executing without signatures and being able to return events generated during simulation execution. However, it's unclear why this hasn't been implemented.

Do you have any plans to provide a dryRun API method that doesn't require signatures or SignedExtra. It would be even better if it could return events.

@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label May 10, 2024
@bkchr
Copy link
Member

bkchr commented May 10, 2024

Would you be interested on working on this? Generally it should not be that complicated, we mainly need some runtime api and implement it:

decl_runtime_apis! {
     trait DryRunCall<Call, Origin, Event> {
           fn dry_run_call(call: Call, origin: Origin) -> (DispatchResultWithInfo, Vec<Event>);
     }
}

impl_runtime_apis! {
    impl DryRunCall<RuntimeCall, PalletsOriginOf<Runtime>, RuntimeEvent> for Runtime {
         fn dry_run_call(call: RuntimeCall, origin: PalletsOriginOf<Runtime>) -> (DispatchResultWithInfo, Vec<RuntimeEvent>) {
               let res =  call.dispatch(origin);

               (res, System::read_events_no_consensus().collect())
         }
    }
}

@bkchr bkchr removed the I10-unconfirmed Issue might be valid, but it's not yet known. label May 10, 2024
@acatangiu
Copy link
Contributor

acatangiu commented May 12, 2024

This is already covered by #3872 (ignore the xcm bits, you can dry run any extrinsic)

Right?

@bkchr
Copy link
Member

bkchr commented May 12, 2024

No is it not. This is also only supports to execute either an extrinsic or a XCM. I mean yes, you could send a XCM that is only a Transact, but that only sounds shitty :P

@franciscoaguirre
Copy link
Contributor

You mean the issue is you have to pass an extrinsic, not a call, right?
Should we just change that to take a call instead of an extrinsic?
If it's more useful I don't see a reason why we shouldn't do it. Do we gain anything from it being an extrinsic instead of a call?

@bkchr
Copy link
Member

bkchr commented May 25, 2024

You mean the issue is you have to pass an extrinsic, not a call, right?

Yes.

Should we just change that to take a call instead of an extrinsic?

Change what? The XCM dry run?

@franciscoaguirre
Copy link
Contributor

Yeah change the XCM dry run to just take in a call.

@bkchr
Copy link
Member

bkchr commented May 25, 2024

We should change the DrynRunApi. This is not really a XCM specific API.

@franciscoaguirre
Copy link
Contributor

We can change both if it's more useful to take in a call. I think we'd like to have the same API so it's easier for UIs to use one or the other.

@bkchr
Copy link
Member

bkchr commented May 26, 2024

Yeah changing both sounds about right or maybe having one for call and one extrinsic.

@acatangiu
Copy link
Contributor

You mean the issue is you have to pass an extrinsic, not a call, right? Should we just change that to take a call instead of an extrinsic? If it's more useful I don't see a reason why we shouldn't do it. Do we gain anything from it being an extrinsic instead of a call?

you should check which (call vs extrinsic) is easily integrated in PJS and PAPI

the majority of usage will go through one of those two

@franciscoaguirre
Copy link
Contributor

The DryRunApi on XCM now takes a call instead of an extrinsic, checked with PAPI and now there's no need to double-sign or use SignedExtra 😁

@acatangiu
Copy link
Contributor

This is done, available in the SDK, and also integrated in system chains.

@github-project-automation github-project-automation bot moved this from Backlog to Done in Runtime / FRAME Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

4 participants