Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step 2: kernel integration (migrate functionality to fvm) #778

Merged
merged 5 commits into from
Jan 18, 2019

Conversation

aionick
Copy link
Contributor

@aionick aionick commented Jan 17, 2019

Step 2 in the kernel integration.

This PR mostly moves classes from the kernel into the fast vm.

A consensus test has been added to check some basic hard-coded hash values that we must also produce to not break consensus. This test will be updated as we go on.

@aionick aionick changed the title Incremental pr2 Step 2: kernel integration Jan 17, 2019
@aionick aionick added this to the 0.3.3 milestone Jan 17, 2019
@aionick aionick changed the title Step 2: kernel integration Step 2: kernel integration (migrate functionality to fvm) Jan 17, 2019
* Zcash project team.
* Bitcoinj team.
*/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

license header should be removed

Copy link
Contributor

@aion-kelvin aion-kelvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than the headers.

*/
ImportResult tryToConnectInternal(final AionBlock block, long currTimeSeconds) {
// TEMPORARY: here to support the ConsensusTest
public Pair<ImportResult, AionBlockSummary> tryToConnectAndFetchSummary(AionBlock block, long currTimeSeconds) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make a note/Issue/Jira to re-visit this once the critical code has been merged in

@@ -0,0 +1,39 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary headr

@aionick aionick added the wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions label Jan 18, 2019
- The following classes were moved: TransactionExecutor,
  AbstractExecutor, FastVmResultCode and FastVmTransactionResult.
- This is a step on the way towards achieving the correct dependency
  relationship between modVM, modFastVM and modPrecompiled.
- As well as its two subclasses - these all existed as a means of
  injecting the fastvm & precompiled depencies into modVM to get
  around the dependency inversion going on here. This is now done
  with.
- IPrecompiledContract is now PrecompiledContract and is now in
  modPrecompiled. IContractFactory is removed. ExecutionContext and
  SideEffects have been moved from modVM into the fvm.
- Though it is far from its final place. It is merely here so that
  the modVM, FastVM, modPrecompiled modules all have their dependencies
  solved.
- Now BulkExecutor in modVM sees FastVM instead of the other way round.
- The fastVM and modPrecompiled modules no longer depend on modVM.
- This test checks against only a very basic case, that none of the
  fundamental results required for network consensus are broken by
  any changes we are doing.
@aionick aionick changed the base branch from incremental-pr to master-pre-merge January 18, 2019 18:01
@aionick aionick removed the wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions label Jan 18, 2019
@AlexandraRoatis AlexandraRoatis merged commit cfdc802 into master-pre-merge Jan 18, 2019
@AionJayT AionJayT deleted the incremental-pr2 branch January 23, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants