Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Atomic Transaction and Batch Transaction #1791

Closed
xlc opened this issue Feb 14, 2019 · 6 comments
Closed

Atomic Transaction and Batch Transaction #1791

xlc opened this issue Feb 14, 2019 · 6 comments
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.
Milestone

Comments

@xlc
Copy link
Contributor

xlc commented Feb 14, 2019

Currently transactions are not applied atomically that if it failed halfway through, the previously applied storage updates are kept and not reverted. Therefore we have to perform all the checks before first storage write and cannot return error after first storage write happened.

That means if a runtime module wants make multiple fallible calls, it have to first check all of them won't fail upfront. This can be hard to implement. Image I want to implement a runtime call to transfer token to 3 different accounts, which may or may not be already created (hence different transaction fee will be deduced). It is hard to calculate the exact total balances required to make this action without failure.

Another feature I would like to implement in future is atomic batch transaction (Thanks to Schnorr, only one signature will be needed). It needs to ensure either all or none of the transactions success. That means it needs ability to revert previously applied transaction if the last one failed.

@gavofyork gavofyork added this to the As-and-when milestone Feb 15, 2019
@gavofyork
Copy link
Member

It is hard to calculate the exact total balances required to make this action without failure.

Is it? It's relatively easy to check for account existence (Currency::balance(id).is_zero()) and it's not hard to add together three numbers...

That said, I'm not against the ability to set "revert points" in runtime execution, it's just quite a bit of work to introduce it without pessimising the average case and in most instances really isn't required.

I fear batch transactions would require fairly substantial reworking of the transaction queue and introducing such mutual dependencies at the lowest level would inevitably risk introducing numerous attack vectors.

I think a better solution would be to keep transactions simple but introduce a simple module that allowed for multiple Calls to be dispatched at once according to what was signed.

e.g. Account A wants to call X, Account B wants to call Y, but neither should succeed unless both succeed. A sends a transaction: commit(hash([(A, X), (B, Y)])), B sends a transaction: commit(hash([(A, X), (B, Y)])); then either A or B send a transaction execute([(A, X), (B, Y)]) which would check that hash([(A, X), (B, Y)]) was apprroved by all parties involved and then dispatch X and dispatch Y.

@gavofyork gavofyork added the J1-meta A specific issue for grouping tasks or bugs of a specific category. label Feb 15, 2019
@gavofyork gavofyork added this to To Do in Runtime via automation Feb 15, 2019
@gavofyork gavofyork modified the milestones: As-and-when, The Distant Future Feb 15, 2019
@xlc
Copy link
Contributor Author

xlc commented Feb 15, 2019

Even for simple case like transfer, it can be tricky to handle all the edge cases. e.g.
C did not exist
A transfer to C
B transfer to C

For this case, the pre-checks may include account creation fee twice because C did not exist without checking both transfer goes to a same account. It is very easy to make mistake.

A much harder example is make two smart contract calls, you simply cannot calculate the gas fee required upfront.

Thanks for the suggestion of using a module to handle the multiple Calls, it is definitely cleaner than modify the extrinsic format and executive module. But it does have drawback of having more overheads, but that's acceptable.
The real issue is that it still won't be able to perform all the checks upfront and then apply the changes knowing all of them won't fail.

Also this issue does not only exists in the multi-Calls scenario. Image the case someone wants to implement a new runtime call, which needs to call make_transfer multiple times

pub fn make_transfer(transactor: &T::AccountId, dest: &T::AccountId, value: T::Balance) -> Result {

Or call any methods that have side-effect and returns Result multiple times.
It is not easy to write the pre-conditios to ensure all the has-side-effect fallible methods won't fail.

In summary, I think this limitation increase the barrier of module re-use, increase the complexity to implement complicate logics and allow developers to easily make mistakes. It doesn't scales.

@xlc
Copy link
Contributor Author

xlc commented Feb 20, 2019

Another benefit of be able to return error anytime without unwanted side effects is that it will be easy to always use checked arithmetic without worry if an operation could overflow or not.
One example is that currently balances module is mixed with checked_add and +. Careful code audit is required to ensure all + usage are safe. If all of the overflowing arithmetic are replaced with checked one, we will have full confidence no overflow could happen. This helps parachain developers, whom may not have the full expertise on secure blockchain development, to write safer code and have one less thing to worry about.

@gavofyork
Copy link
Member

A much harder example is make two smart contract calls, you simply cannot calculate the gas fee required upfront.

If you require to know whether gas costs are sufficient for a arbitrary code to execute before making state changes or not, then you have just created yourself a trivially attackable chain.

Now I'm not saying that being able to return errors at any point is without its benefits. But the complexity and potential for pessimisation it would introduce on the client side is too high a cost to bear for us right now. It also introduces a programming style that makes it way easier to create code that is economically insecure, as you have huge potential for introducing substantial computation that results in an Error with no state change and thus no fees charged.

The smart contract environment is designed to provide this higher level functionality as the speed
and complexity costs are already paid and the economic security guaranteed through metering.

I'm afraid my position is that parachain developers will just have to be mindful for now.

@xlc
Copy link
Contributor Author

xlc commented Feb 25, 2019

@gavofyork Thanks for the explanation.

Is it possible to design the runtime such that the transaction fee are still charged even the transaction failed? So they have similar property to smart contract.
Runtime developers still have to remember to charge a fee before any signification operations (which may fail due to out of fee error), but at least they can be confident that an error will not result partially applied transaction.

There is already developed solution for contract, is it possible to generalize this solution so it works for all the runtime modules? The fees module needs some special case handling so its writes are not revertible, but we have a single place to do the modification.

@xlc
Copy link
Contributor Author

xlc commented Jun 26, 2020

Fixed by #6269

@xlc xlc closed this as completed Jun 26, 2020
Runtime automation moved this from To Do to Done Jun 26, 2020
@xlc xlc added this to Done ✔️ in SDK Node (deprecated) via automation Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.
Projects
No open projects
SDK Node (deprecated)
  
Done ✔️
Development

No branches or pull requests

3 participants