aionnetwork / AVM

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

[CLOSED] Fully partition kernel and avm execution responsibilities #301

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by aionick (on Friday Nov 02, 2018 at 17:48 GMT)

A lot of the responsibilities relating to processing a transaction are currently performed on both sides of the border. In most cases the kernel just does all the work and ignores what the avm does. We now need to fully separate these responsibilities so there's no duplication and so that we can help optimize or meet the needs of the avm's asynchronous workload.

This also needs to be finished before we take advantage of the avm's asynchronous processing in #259 , so that we can ensure things are working correctly in the easier sequential situation.

The responsibilities we need to move out of the kernel into the avm are:

  1. transaction energy limit checks.
  2. sender balance checks.
  3. sender nonce checks.
  4. balance transfers (#260).
  5. only putCode upon SUCCESS.
  6. create contract account address
  7. check address existence at contract creation time
  8. be able to handle contract creation when the provided data field is null or an empty array.
  9. be able to handle contract calls when the recipient has no code (this is a strict balance transfer).
  10. energy deduction and refunds.
  11. Avm: do not commit any state changes on rejection.
  12. coinbase fees should be paid inside Avm.
aionbot commented 5 years ago

Comment by aionick (on Monday Nov 05, 2018 at 16:01 GMT)

Responsibilities 1, 2 and 3 were removed from the kernel in commit 5c72bdb61bbc129f2f9dad2972ecfba14eae254d

aionbot commented 5 years ago

Comment by aionick (on Monday Nov 05, 2018 at 16:18 GMT)

Responsibilities 8 and 9 were removed from the kernel in commit e342e717fa893e8aa9871479e2777f6d341d1e41

aionbot commented 5 years ago

Comment by aionick (on Thursday Nov 08, 2018 at 16:12 GMT)

Note that we are currently going to keep the following responsibilities inside the kernel (I don't foresee these causing troubles with the concurrent model. If so, they are each fairly small changes):

The middle two could arguably be moved into the Avm, and maybe that is best. But the first and last on this list seem to be outside the scope of the Avm, dealing with concepts it shouldn't know even exist.

Edit: speaking with @JunhanHu-aion, it seems the Avm does do these middle two. I will try and remove these responsibilities out of the kernel as well.

aionbot commented 5 years ago

Comment by aionick (on Friday Nov 09, 2018 at 20:08 GMT)

Responsibility 6 was removed from the kernel in commit 3ade91b4fa262c70650a128ed93a919752273b26

aionbot commented 5 years ago

Comment by aionick (on Monday Nov 12, 2018 at 19:36 GMT)

Responsibility 5 on the kernel side is done in commit b3fa25405b9e7971428790ac76771481639d4845

aionbot commented 5 years ago

Comment by aionick (on Tuesday Nov 13, 2018 at 16:05 GMT)

Responsibility 11 on the avm side is done in commit 165f54f

aionbot commented 5 years ago

Comment by aionick (on Tuesday Nov 13, 2018 at 20:05 GMT)

Regarding my previous comment in which I listed out 4 responsibilities that are all being done in the kernel, here is the final verdict on these 4 responsibilities and their current status:

  1. The nonce update is always done in Avm, the conditional aspect of this update is controlled by the kernel. Basically the KernelInterface that is sent into the Avm is configured to block this update if required. This way the Avm does not need to know about this, it is certainly beyond its scope and just a kernel "hack" to make the transaction pooling work.
  2. & 3. Now are both done in the Avm only.
  3. Currently is done in the kernel, but this will have to be moved into the Avm since it technically should be sequential and when a miner is paid its fees can determine the validity of a transaction if that transaction was sent by the miner. Otherwise we can create an odd situation where the transactions fail for the miner itself but not for other miners. It's a very rare situation and even harder to rig up intentionally but it is possible and we should be consistent in our billing times anyway.
aionbot commented 5 years ago

Comment by aionick (on Wednesday Nov 14, 2018 at 16:44 GMT)

This issue is complete insofar as the concurrent executor for the kernel is concerned. The remaining task, 7, is being taken care of in issue #307. I will close this issue since its scope is now complete, and I will let #307 deal with the remaining item, which is beyond this scope.