-
Notifications
You must be signed in to change notification settings - Fork 707
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
VISION: Try..catch for substrate runtimes #370
Comments
This will make paritytech/substrate#1791 possible |
To me, a feature like this is a huge gain in terms of simplicity to hack and build on top of other existing modules, and could add to the virality of the Substrate platform. Ultimately, the pattern that is challenging for runtimes is chaining multiple different pieces of logic together in a single module function. Let's say for example buying a kitty in a custom runtime module. There may be two distinct steps which are involved here:
With the current module system, you must write the function like so:
Users cannot in this case consolidate the check/write pattern into a single functional component which then gets serially called by the runtime function. This would often come up when you want to make a function which combines your own logic with multiple downstream runtime module calls. For example imagine a runtime function which calls 3 other modules to:
All of the functions in this example would be dispatchable functions defined in different modules. As a developer, I would have no easy way to execute all 3 by calling the implementations in their respective runtime module. I would need to copy their implementations, separate it into check and write components, and reorder the logic. At this point, the user has just written a new module which re-implements all of this code in other places... I had thought about recommending to users to write their module functions in two halves, a check function and a write function. So something like
But if the caller only has 10 units to begin with, then both checks will pass (since state is not updated between checks) but fail later after the transfer to Alice already happened, and cannot be reverted. The ability to do patterns like this made Ethereum smart contracts and ethereum standards work so well. If we look at the way that contracts like ERC-721 were written by teams like OpenZepplin, you will see there is a base contract |
when will this feature be available? |
One part of it, namely transactional storage, is being worked on in paritytech/substrate#3263. |
↑ that was subsumed by paritytech/substrate#6269 and was merged. |
I think this can be closed as we now have default transactional behaviour for extrinsics. |
Closed with: paritytech/substrate#10806 Would be interested to see if there is any panic catching thing we can do in the runtime too. |
The issue is meant to be tracking general try catch where storage transactions are a part of. Let's keep it reopened until we get there. |
Yeah storage transactions won't help with panics. Without panic safety we cannot ensure that we always collect fees. |
This issue has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/future-of-webassembly-and-ink-smart-contracts/428/5 |
This issue has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/the-xpost-bot-is-awesome-but-can-be-make-it-prettier/448/1 |
This issue has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/eliminating-pre-dispatch-weight/400/14 |
Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2. - [Release notes](https://github.com/npm/ssri/releases) - [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md) - [Commits](npm/ssri@v6.0.1...v6.0.2) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
At the moment, a runtime call modifies storage eagerly. I.e. the runtime performed some storage changes there is no way of rolling back these changes. Because of that, substrate modules leverage the first-check-then-modify approach. This means that all conditions should be checked before making any side-effects. This also has a serious ramification for the runtime, the runtime MUST NOT panic under any circumstances. If it were to do so, then it opens up a DoS vector, since all changes including the charging fees for a transaction are rolled back.
In the srml-contracts module we implement a system that emulates checkpoint functionality. It does this by storing all changes performed by a smart-contract in memory, committing them to the main storage if and only if the smart-contract successfully finishes the execution. This however is not ideal, since the rest of the runtime logic acts eagerly and changes the storage as soon as that logic is called and there is no way of rolling these changes back. This really poses a problem when interaction between the smart-contract module and the rest of the runtime comes into a play: the smart-contract should revert all changes in the case of failure, so that means we can't call the runtime directly and we need to delay all calls after the smart-contract finished successfully. That also implies that the smart-contracts can't easily get the result of the execution of such a runtime call.
This makes me think that it might be beneficial to lift the checkpointing logic to substrate itself. If we had substrate-wide support of such pattern, then we could fix the issue of interaction between smart-contracts, make some patterns safer and most importantly make the runtime more tolerable to panics.
There might be different designs of such a mechanism, but here is one:
ext_try
. It takes the name or the pointer to the function.Ext
that is linked with the parent's one.Ext
.Ext
. If the runtime traps (e.g. panics, overflows its stack or so on), the changes are just discarded.ext_try
.Having this functionality:
executive
module as just charging the fee andext_try
-ing to dispatch transaction. There is a chance that this would open us a way to use more sanity checks (i.e usingassert!
s) further improving the security of a chain. It is worth noting that some of the path still have to be panic-free.AccountDb
. Transfers would be implemented as regular transfers, without trying to replicate what theT::Currency
trait does. Calling to the runtime would be as simple as dispatching the function and getting the result.The text was updated successfully, but these errors were encountered: