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

Clean up transaction result code #720

Merged
merged 2 commits into from
Nov 23, 2018
Merged

Clean up transaction result code #720

merged 2 commits into from
Nov 23, 2018

Conversation

iamyulong
Copy link
Contributor

Description

This PR intends to clean up the transaction result code:

  • Aligned the ReasonCode with the error code defined in the C interface;
  • Corrected the inappropriate usage of INTERNAL_ERROR;
  • Fixed the STATIC_MODE_ERROR misinterpretation.

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.

Copy link
Contributor

@AlexandraRoatis AlexandraRoatis left a comment

Choose a reason for hiding this comment

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

Does anything need to be updated in org.aion.mcf.blockchain.TxResponse and org.aion.api.server.ApiTxResponse as a result of these changes?

INSUFFICIENT_BALANCE(10),
CONTRACT_ALREADY_EXISTS(11),
INTERNAL_ERROR(-1);
STATIC_MODE_ERROR(8),
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment explaining the STATIC_MODE_ERROR as well

@@ -38,19 +38,36 @@
byte[] output;

public enum ResultCode {
/*
* Indicates a successful transaction.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

minor formatting issue: missing the plugin

@AlexandraRoatis AlexandraRoatis added bug Something isn't working enhancement New feature or request labels Nov 22, 2018
@AlexandraRoatis AlexandraRoatis added this to the 0.3.2 milestone Nov 22, 2018
@AionJayT AionJayT merged commit c600334 into yulong Nov 23, 2018
@AlexandraRoatis
Copy link
Contributor

I think this PR was supposed to be updated to go to the master-pre-merge branch

@AionJayT AionJayT deleted the yulong_tx_code branch December 11, 2018 16:34
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants