-
Notifications
You must be signed in to change notification settings - Fork 430
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
Drink backend: support runtime call #1891
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could potentially do some hacky type inference here, but current solutions looks good enough to me.
# Conflicts: # crates/ink/tests/ui/contract/fail/message-returns-non-codec.stderr # crates/ink/tests/ui/trait_def/fail/message_output_non_codec.stderr
crates/e2e/Cargo.toml
Outdated
@@ -33,6 +33,7 @@ tracing = { workspace = true } | |||
tracing-subscriber = { workspace = true, features = ["env-filter"] } | |||
scale = { package = "parity-scale-codec", workspace = true } | |||
subxt = { workspace = true } | |||
subxt-metadata = { workspace = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this one optional too and dependant on the drink
feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, sure, my bad; done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In standard e2e backend (i.e. node+subxt) we must have only dynamic dispatch. This is because we support running a node with an arbitrary runtime, and therefore we cannot statically generate runtime types, including RuntimeCall enum family.
Yes and no, I think it would be beneficial to define a minimal set of hardcodeed common calls that are useful and supported by both substrate-contracts-node
and drink
. Balances::transfer etc... (in fact we already do this for the pallet_contracts
calls instantiate
, call
etc.
Of course people can still target their own custom node and then if they don't satisfy that API then calls simply return an error "Not implemented for target node".
Either way this dynamic API is required and very useful.
Sure it is, I'm not attacking it. Just saying that it is not (and won't be) natively supported in drink |
Since inkdevhub/drink#36 is done, we can finally support arbitrary runtime calls in e2e tests. However, the solution is kinda tricky.
Problem
In drink! we have strongly typed runtime calls. This is because we keep the runtime directly, in-memory, and thus we have access to the
RuntimeCall
enum family. And thus there's no need of dynamic dispatch.In standard e2e backend (i.e. node+subxt) we must have only dynamic dispatch. This is because we support running a node with an arbitrary runtime, and therefore we cannot statically generate runtime types, including
RuntimeCall
enum family.But in order to support using both backends with the same test code, we have to provide a common interface...
Solution
We can't make
ChainBackend::runtime_call
statically typed, since forsubxt_client::Client
there is no way of providing correct types. Hence, we have to leave the API as it is (i.e. dynamic dispatch with pallet and call as strings and call data assubxt::Value
wrappers).For the drink! implementation we do following mumbo-jumbo:
RuntimeCall
object🤷♂️
Note
Since
drink::MinimalRuntime
andsubstrate-contracts-node
's runtime use different addressing, the testcases that relate to runtime calling are compatible with only one backend. However, notice that this is the incompatibility of used runtimes, not backends, and thus it's okay.