Skip to content

Extracting AvmContractDetails into separate class#1112

Merged
AlexandraRoatis merged 21 commits intomasterfrom
AKI-435
Feb 12, 2020
Merged

Extracting AvmContractDetails into separate class#1112
AlexandraRoatis merged 21 commits intomasterfrom
AKI-435

Conversation

@AlexandraRoatis
Copy link
Copy Markdown
Contributor

No description provided.

@AlexandraRoatis AlexandraRoatis added the wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions label Jan 30, 2020
@AlexandraRoatis AlexandraRoatis added this to the 1.3 milestone Jan 30, 2020
@AlexandraRoatis AlexandraRoatis self-assigned this Jan 30, 2020
@AlexandraRoatis AlexandraRoatis force-pushed the AKI-435 branch 7 times, most recently from f929a37 to 904adbb Compare February 3, 2020 21:00
Reading contract details from disk involves:
 1. splitting the RLP encoding into meaningful components, and
 2. interpreting the components according to the vm type.

To make the code more understanable and increase its maintainability the
first part of the decoding is done using the new RLPContractDetails class.
The previous commit started the separation of the decoding process
into (a) reading the data from disk and (b) interpreting the data.
It also introduced part (a) as the process of creating the
RLPContractDetails object.

This commit completes the separation by doing part (b). As a result
only the interpretation of data remains inside the decode method of
AionContractDetailsImpl.

The unit tests for RLPContractDetails cover incorrect encoding size,
testEncodingIncorrectSize was no longer useful.
The VM type is required to interpret the contract details correctly.
It is now provided directly to the decode method instead of being
specified externally and requested via documentation.
DetailsDataStore has access to the physical databases where contract
data is stored. Since connecting the contract details object to the
data source must go though the DetailsDataStore it is best to perform
this operation when the instance is created.

Additionaly, this refactoring minimizes direct access to the databases
used by all contracts and reduces the mutability of the contract
details object.
Any use of this internal variable is misleading and can cause serious
errors. The current concatenated storage hash must be computed and is
retrieved by calls to the getStorageHash method.
The new AvmContractDetails is identical to AionContractDetailsImpl,
other than the class name. This choice was made to allow easily tracking
the evolution of the class towards supporting only AVM contracts.
Similarly, AionContractDetailsImpl will evolve to support only the FVM.

The DetailsDataStore is responsible for instantiating the correct version
based on the given VM type. To avoid incorrect cross-class casting, it
does not create a ContractDetails instance when the VM type does not
correspond to a supported VM.
@AlexandraRoatis AlexandraRoatis force-pushed the AKI-435 branch 3 times, most recently from 848f0ad to 3509a79 Compare February 4, 2020 21:01
@AlexandraRoatis AlexandraRoatis changed the title WIP: AKI 435 Extracting AvmContractDetails into separate class Feb 4, 2020
@AlexandraRoatis AlexandraRoatis added enhancement New feature or request and removed wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions labels Feb 4, 2020
The storage transition from in-line the details database to the external
storage database existed as a way to accomodate small FVM contracts that
could function mainly in the limited trie size that allows the in-line
storage. The AVM uses its object graph for tracking internal variables,
while the external storage is specifically meant for large collections
of data. Transitioning in this case is typically imminent and not worth
the additional cost demonstrated by the benchmark test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant