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

fix(transaction): Improve Numeric Json Serialization #103

Merged
merged 2 commits into from
Jun 21, 2019
Merged

fix(transaction): Improve Numeric Json Serialization #103

merged 2 commits into from
Jun 21, 2019

Conversation

sleepdefic1t
Copy link
Contributor

Current implementation sometimes fails to properly serialize numbers in the toJson() method.
This is especially an issue in some Linux builds and all Arduino IDE builds (Arduino IDE Library Manager version (Cpp-Crypto-Arduino-v.0.3.0)).

Specifically, this PR does the following:

  • moves <cinttypes> to crypto_helpers.h for non-IoT builds.
  • forces type serialization for:
    • amount (llu/uint64_t).
    • fee (llu/uint64_t).
    • network (uint8_t/int).
    • timestamp (lu/uint32_t).
    • type (uint8_t/int).
    • version (uint8_t/int).
  • updates the changelog.

This PR resolves #99.

Tests will fail until #101 is merged and this PR is rebased.

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR release a new version?

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch
  • All tests are passing
  • New/updated tests are included

Current implementation sometimes fails to properly serialize numbers in the `toJson()` method.
This is especially detrimental to some Linux builds and all Arduino IDE builds (Arduino IDE Library version (Cpp-Crypto-Arduino-v.0.3.0)).

Specifically, this PR does the following:
- moves `<cinttypes>` to `crypto_helpers.h` for non-IoT builds.
- forces type serialization for:
  - `amount` (llu/uint64_t).
  - `fee` (llu/uint64_t).
  - `network` (uint8_t/int).
  - `timestamp` (lu/uint32_t).
  - `type` (uint8_t/int).
  - `version` (uint8_t/int).
- updates the changelog.
@ghost ghost added Complexity: Low Less than 64 lines changed. Type: Bugfix The pull request fixes an incorrect functionality or behaviour. labels Jun 21, 2019
@ghost
Copy link

ghost commented Jun 21, 2019

The ci/circleci: build-linux-default job is failing as of c0a326fa0cf246c7734ad048423bb0e743c22c0d. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

1 similar comment
@ghost
Copy link

ghost commented Jun 21, 2019

The ci/circleci: build-linux-default job is failing as of c0a326fa0cf246c7734ad048423bb0e743c22c0d. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@sleepdefic1t sleepdefic1t changed the title [WIP]fix(transaction): Improve Numeric Json Serialization fix(transaction): Improve Numeric Json Serialization Jun 21, 2019
@sleepdefic1t
Copy link
Contributor Author

@faustbrian This is good to go 👍

@faustbrian faustbrian merged commit ed0ab57 into ArkEcosystemArchive:develop Jun 21, 2019
@sleepdefic1t sleepdefic1t deleted the fix(transaction)-improve-numeric-serialization branch June 21, 2019 09:21
@sleepdefic1t sleepdefic1t restored the fix(transaction)-improve-numeric-serialization branch June 21, 2019 09:22
@sleepdefic1t sleepdefic1t deleted the fix(transaction)-improve-numeric-serialization branch June 21, 2019 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Low Less than 64 lines changed. Type: Bugfix The pull request fixes an incorrect functionality or behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants