Closed aionick closed 5 years ago
Some API concerns:
VirtualMachine
should probably take a transaction & KernelInterface as its input instead of a context. The KernelInterface really can account for the context, and it feels awkward to have to enclose a transaction inside of its so-called context so that the VM can use it.InternalTransaction
has a lot of methods that the Avm does not implement and which do seem to be implementation details rather than genuine API-level methods.TransactionResult
: need to figure out how much of this should remain in the VM and how much in the API. I'm kind of leaning towards moving all result codes into the API except possibly the failure codes, which may be VM-specific. Or moving some of these up and allowing VM's to define their own failure codes as well.TransactionResult
should have its fields set ONLY inside the virtual machines, and the kernel should be the consumer of this data. Therefore, the getter is all that needs to be exposed.VirtualMachine
should start itself up once you first create it, this should not be part of the API.KernelInterface
needs to document each method's full scope of functionality precisely so that everyone agrees on it. This is a dangerous place where returning null
instead of some other 'invalid' signifier (zero or an empty list, for example) can lead to bugs that internal VM tests don't catch because someone else implemented the interface differently.I'm going to go ahead and close this issue, it was mainly here to collect insights as we worked through the integration. It will be useful going forward, but we can retrieve it from the closed issues. I suspect there's not much more to add on to this anymore.
The Avm took on the API defined in the VM API project starting at commit 76fd88b53135a1e09ef1da18b1f479ff48efa928. This API is still not in its final shape and some of it has been forced somewhat awkwardly upon the Avm.
I want to use this issue to track two things:
Proposed changes to the Avm:
getEnergyRemaining()
inAvmTransactionResult
.destinationAddressIsSafeForThisVM
fromKernelInterface
to block fvm/precompiled calls.Transaction
constructor should take nonce as aBigInteger
not along
, especially now that theKernelInterface
returns aBigInteger
.SideEffects
class properly and remove the data that will be tracked here from theAvmTransactionResult
, where it no longer belongs.