Introduce capabilities filtering for off-chain runtime calls.#3454
Introduce capabilities filtering for off-chain runtime calls.#3454
Conversation
tomusdrw
changed the title
tomusdrw
changed the title
| /// | ||
| /// This should include all capabilities of `offchain::Externalities`. | ||
| OffchainWorker(Box<dyn offchain::Externalities>), | ||
| /// Context used for other calls. |
There was a problem hiding this comment.
Are all of these exclusive?
| let crate_ = self.crate_; | ||
| let context_other = quote!( #crate_::runtime_api::ExecutionContext::Other ); | ||
| let context_other = quote!( #crate_::runtime_api::ExecutionContext::OffchainCall ); | ||
| let fn_impl = self.create_method_runtime_api_impl(method.clone()); |
There was a problem hiding this comment.
Maybe rename context_other.
bkchr
left a comment
bkchr
left a comment
There was a problem hiding this comment.
In general looking good, but I really think that we should panic when something tries to access a function without the required capabilities. Otherwise I see us at some point debugging a problem that will have its root cause in such a function.
That's why we print a warning if such function is being used. I'm really scared of
Any mistake in a code like this will cause the entire network to brick forever. |
|
For that exists testing, if you deploy code that panics it is your own fault. You should not use these functions from within the chain. |
Offchain workers are much better separate from other parts of the runtime code IMHO (it's one special function that is being called). Regular
IMHO we've experienced multiple times that testing is never enough, and just saying: "It's your fault to brick a $1M network" is a lame excuse to introduce security measures to prevent that. That said, I'm fine to bend to pressure as I'm also torn apart between having a clean code / easy debugging experience and catering for possible future (possibly unlikely) mistakes. Will update the code shortly. |
That is correct, but the code will fail anyway when you try to call it, as the |
|
We can also return a |
4 participants
If we call into the runtime, we can pass various
ExecutionContexts to identify where is it coming from.Initially this was only affecting how we run the call (a.k.a execution strategy = native / wasm), with introduction of
OffchainWorkersthis also controls the APIs that are available for that part of the runtime.Recently in #3296 we've introduced additional keystore APIs that must not be available for consensus-critical code, but are very useful for off-chain calls, like rotating the keys - they are only valid in
ExecutionContext::Other.This PR introduces a possibility to further control what APIs (
Capabilties) are available for particular off-chain call and tries to make it more explicit. This will come handy to implement equivocation reporting for Grandpa/Babe, since we will be able to call into the runtime, produce, sign and submit transaction to the pool in one call. In the future we even want the off-chain call to be able to read (And possibly) write the offchain worker DB, for instance to retrieve the merkle proofs for session membership.