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

Removing the AVM prefix #851

Merged
merged 5 commits into from
Mar 19, 2019
Merged

Removing the AVM prefix #851

merged 5 commits into from
Mar 19, 2019

Conversation

AlexandraRoatis
Copy link
Contributor

@AlexandraRoatis AlexandraRoatis commented Mar 15, 2019

Description

  1. Introduced functionality for storing additional contract information like the block when the contract was created and VM used for deployment.
  2. Fully removed the use of AVM specific contract prefix.
  3. Corrected the address generation inside AionCapabilities.java
  4. Expanded repository interface with a getter for the VM deployment code.
  5. Updated the VM specs with two supported VM codes for contract deployment (previously there were 3 allowed values).
  6. Removed the prefix checks from tests and expanded tests to ensure the receipt output matches the contract address.

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

  • Bug fix.
  • New feature.
  • Enhancement.
  • Unit test.
  • Breaking change (a fix or feature that causes existing functionality to not work as expected).
  • Requires documentation update.

Testing

Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.

  • the existing test suite and newly added checks

Verification

Insert x into the following checkboxes to confirm (eg. [x]):

  • I have self-reviewed my own code and conformed to the style guidelines of this project.
  • New and existing tests pass locally with my changes.
  • I have added tests for my fix or feature.
  • I have made appropriate changes to the corresponding documentation.
  • My code generates no new warnings.
  • Any dependent changes have been made.

@AlexandraRoatis AlexandraRoatis added the wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions label Mar 15, 2019
@AlexandraRoatis AlexandraRoatis added this to the 0.4.0 milestone Mar 15, 2019
@AlexandraRoatis AlexandraRoatis self-assigned this Mar 15, 2019
@AlexandraRoatis AlexandraRoatis requested a review from aionick March 15, 2019 20:43
@AlexandraRoatis AlexandraRoatis force-pushed the rm-avm-prefix branch 2 times, most recently from fdc5c94 to ef3e446 Compare March 18, 2019 17:11
@AlexandraRoatis AlexandraRoatis added bug Something isn't working enhancement New feature or request feature and removed wip Indicates that a PR or issue is Work In Progress, issuer should be notified before any actions labels Mar 18, 2019
@AlexandraRoatis AlexandraRoatis removed the request for review from aionick March 18, 2019 17:23
@AlexandraRoatis
Copy link
Contributor Author

Note: Once the changes are approved, I will upload the new code to Maven and change the vm_api library dependencies.

@AlexandraRoatis
Copy link
Contributor Author

based on my discussion with @jeff-aion the VirtualMachineSpecs will be moved out of the interfaces repository

Copy link
Collaborator

@AionJayT AionJayT left a comment

Choose a reason for hiding this comment

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

LGTM, might consider using the detail data store instead of creating new duplicate contract info.

@AlexandraRoatis AlexandraRoatis force-pushed the rm-avm-prefix branch 10 times, most recently from 7100c87 to 62d5247 Compare March 19, 2019 15:53
 - corrected address generation inside AionCapabilities.java
 - expanded repository interface with check for VM deploy code
 - updated the VM specs with two supported VM codes for contract deploy
 - removed prefix checks from tests
 - expanded tests to ensure the receipt output matches the contract address
…on type

 - using the local version of the vm_api since more changes are expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants