Description
ℹ️ NOTE 2022-11-07:
This is a classical issue which I wrote with my contracts hat on. However, it is not specific to contracts.
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:
- We introduce a new host function, called
ext_try
. It takes the name or the pointer to the function. - It creates a new
Ext
that is linked with the parent's one. - A new instance of the runtime is initialized and the specified function is called.
- All changes are collected in the nested
Ext
. - If the nested call succeeds, then we commit all the changes to the parent's
Ext
. If the runtime traps (e.g. panics, overflows its stack or so on), the changes are just discarded. - The status is conveyed to the caller of
ext_try
.
Having this functionality:
- We could express
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. - We could simplify smart-contracts to just execute code directly without involving abstractions like
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. - It might open-up other patterns that can leverage this mechanism, where it is really hard to ensure all the invariants up-front (or at least without reingereering the whole runtime).
Metadata
Metadata
Assignees
Type
Projects
Status
Activity
xlc commentedon Jul 1, 2019
This will make paritytech/substrate#1791 possible
shawntabrizi commentedon Jul 2, 2019
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
check_can_transfer
andmake_transfer
. However, this fails to work when the actions of one call could affect the ability to successfully check another. For module function which does two transfers might naively be written like this:check_can_transfer(10 units to Alice)
check_can_transfer(10 units to Bob)
make_transfer(10 units to Alice)
make_transfer(10 units to Bob)
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
ERC721.sol
and then multiple "extension" contracts likeERC721Enumerable.sol
. These extension contracts work because users are able to add additional check/write patterns on top of an existing base, without having to get too dirty into someone else's code.vjoke commentedon Sep 23, 2019
when will this feature be available?
pepyakin commentedon Sep 23, 2019
One part of it, namely transactional storage, is being worked on in paritytech/substrate#3263.
pepyakin commentedon Jun 20, 2020
↑ that was subsumed by paritytech/substrate#6269 and was merged.
ggwpez commentedon May 30, 2022
I think this can be closed as we now have default transactional behaviour for extrinsics.
shawntabrizi commentedon May 31, 2022
Closed with: paritytech/substrate#10806
Would be interested to see if there is any panic catching thing we can do in the runtime too.
pepyakin commentedon May 31, 2022
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.
18 remaining items