search

aionnetwork / AVM

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

[CLOSED] Validate and fix the energy deduction with kernel integration #310

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by nancyaion (on Friday Nov 09, 2018 at 16:32 GMT)

Currently, there are issues on the energy deductions with the kernel and AVM interaction.

  1. energy is deducted twice / duplicated, in the kernel and also AVM calling into the kernel to deduct it. It should be done by the kernel who deducts the maximum amount prior to AVM calls and then refund after the AVM returns the energy consumption result.

  2. some calculation issues.

aionbot commented 5 years ago

Comment by nancyaion (on Friday Nov 09, 2018 at 18:38 GMT)

Found this is also part of the discussion in #301

aionbot commented 5 years ago

Comment by nancyaion (on Friday Nov 09, 2018 at 19:04 GMT)

Commit on the Kernel side 32be3b1920412890220884aa92e73f4ea624c5fe

Add another unit test of balance transfer between 2 accounts

which also validates that the energy is not double billed any longer after the fix.

aionbot commented 5 years ago

Comment by nancyaion (on Friday Nov 09, 2018 at 21:32 GMT)

Next is to investigate the energy calculations.

The kernel side shows an energy consumption much higher than AVM reports, shown by 'AvmImplDeployAndRunTest' (the failed part is currently commented out and the investigation will start from there).

aionbot commented 5 years ago

Comment by aionick (on Monday Nov 12, 2018 at 22:30 GMT)

I think there may have been a little mis-communication here, my bad. In #301 I was leaning towards removing this from the kernel and keeping it in the avm, though I see now that was not 100% clear.

This will have to be done (I can do it, should be a pretty quick fix) though because the kernel can't do this. The kernel will send off n transactions into the avm in bulk, and the only way it can deduct energies is to deduct from all n before submitting them. But this creates an unfaithful image of the truth, someone may actually have no coins left after all these deductions but, if the transactions were played out in order, may have had sufficient balance all along, for example (consider a balance transfer in some middle transaction)

aionbot commented 5 years ago

Comment by nancyaion (on Tuesday Nov 13, 2018 at 22:34 GMT)

Thanks, Nick. It is nice to see that finally everything looks correct on AVM and kernel about the energy calculation and reporting.

Kernel commit 5ad9608fc60c16f07018ed3f4743a734a1a3aeb1 remove the energy scaling for AVM

Note that the contract creation takes higher energy on AVM, so the energy limit of that is set to 50_000_000L, instead of 5_000_000L on FastVM.

Close the issue.