aionnetwork / AVM

Enabling Java code to run in a blockchain environment
https://theoan.com/
MIT License
49 stars 25 forks source link

[CLOSED] Self-destruct must transfer balance and render code immediately uncallable #299

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by jeff-aion (on Friday Oct 26, 2018 at 18:27 GMT)

Our BlockchainRuntime.selfDestruct() currently transfers balance and deletes itself, inline with the call.

This doesn't entirely make sense since the function will return and the contract will continue running. This continued running also means that the balance it has after the run can't be known mid-run (it may spend more energy or transfer to other accounts).

Additionally, even the Ethereum SELFDESTRUCT opcode is documented as "Halt execution and register account for later deletion". This means that we need to stop execution when handling this API.

We should be able to do this by using our usual asynchronous termination exception, in the Helper (much like out of energy or abort for concurrent task).

aionbot commented 5 years ago

Comment by jeff-aion (on Friday Nov 02, 2018 at 21:39 GMT)

My current thinking is that we can use a specific sub-class of AvmException in the Helper.forceExitState to handle this case, as well. We can pass the beneficiary address in the exception so that the catch point can request the balance transfer of whatever is left, after the so far consumed energy has been subtracted, plus the refund of the operation (since this usually has a negative cost), once we exit the running transaction.

Note that this kind of exception should only propagate to the entry-point of the current transaction, NOT the top-level external one.

aionbot commented 5 years ago

Comment by jeff-aion (on Monday Nov 05, 2018 at 20:37 GMT)

After some discussion within the team, this write-up describes the proposed direction we want to use: https://aionnetwork.atlassian.net/wiki/spaces/AVM/pages/141066279/Self-Destruct+Behaviour

This may evolve, yet.

aionbot commented 5 years ago

Comment by jeff-aion (on Wednesday Nov 07, 2018 at 15:38 GMT)

Based on @yulongaion's comments in the above page, we will NOT immediately terminate on this call but will issue the balance transfer and make the contract code uncallable (attempt to contact the contract will only transfer balance to an empty "new" account), immediately when called. These side-effects will obey all normal transactional rules.

aionbot commented 5 years ago

Comment by jeff-aion (on Wednesday Nov 21, 2018 at 21:15 GMT)

While building some tests for this situation, I found that our current storage abstraction actually does fail to load objects for a DApp, once it requests delete. This is because of TransactionalKernel, since it filters such IO requests. I will experiment with removing the filtering on these read operations, since we always read data from within the contract, so we already know how to account for this post-delete situation. That said, this does represent a change to how we are handling the existing kernel's storage abstraction which is worth keeping in mind, for the future.