Skip to content

refactor(identities): improve implementation#120

Merged
faustbrian merged 14 commits intoArkEcosystemArchive:developfrom
sleepdefic1t:refactor(identities)/improve-implementation
Aug 8, 2019
Merged

refactor(identities): improve implementation#120
faustbrian merged 14 commits intoArkEcosystemArchive:developfrom
sleepdefic1t:refactor(identities)/improve-implementation

Conversation

@sleepdefic1t
Copy link
Contributor

This PR is mostly centered around improving the Identities implementation.

Use of std::array replaces std::vector in many instances to avoid dynamic allocation. The result is a ~4x-6x gain in execution speed of relevant functions, and a reduced memory footprint overall.

Specifically, this PR does the following:

  • refactors Identities classes.
  • adds hash class.
  • adds curve class.
  • adds str helpers.
  • adds base58 class.
  • moves hex helpers.
  • updates other classes to match changes.
  • adds relevant tests.
  • updates scrips, examples, documentation, and changelog.

What kind of change does this PR introduce?

  • Refactor

Does this PR introduce a breaking change?

  • Yes
  • No

WIF class is now Wif, and Identities namespace is now identities per C++ code style guidelines.

Does this PR release a new version?

  • Yes
  • No

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

faustbrian and others added 7 commits July 16, 2019 19:50
This PR is mostly centered around improving the Identities implementation.

Use of std::array replaces std::vector in many instances to avoid dynamic allocation. The result is a ~4x-6x gain in execution speed of relevant functions, and a reduced memory footprint overall.

Specifically, this PR does the following:
- refactors Identities classes.
- adds hash class.
- adds curve class.
- adds str helpers.
- adds base58 class.
- moves hex helpers.
- updates other classes to match changes.
- adds relevant tests.
- updates scrips, examples, documentation, and changelog.
codacy incorrectly analyzing header struct member as unused.
Not at all the case.
@ghost ghost added Complexity: Undetermined Needs specialized, in-depth review. Type: Refactor The pull request improves or enhances an existing implementation. Status: In Progress The issue or pull request is being worked on. labels Jul 31, 2019
@codecov-io
Copy link

codecov-io commented Jul 31, 2019

Codecov Report

Merging #120 into develop will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #120      +/-   ##
===========================================
- Coverage    91.52%   91.49%   -0.03%     
===========================================
  Files           30       34       +4     
  Lines          861      858       -3     
===========================================
- Hits           788      785       -3     
  Misses          73       73
Impacted Files Coverage Δ
src/include/cpp-crypto/transactions/transaction.h 66.66% <ø> (ø) ⬆️
src/helpers/crypto.cpp 100% <100%> (ø) ⬆️
src/transactions/transaction.cpp 88.82% <100%> (-0.12%) ⬇️
src/crypto/curve.cpp 100% <100%> (ø)
src/identities/privatekey.cpp 100% <100%> (ø) ⬆️
src/identities/keys.cpp 100% <100%> (ø)
src/utils/base58.cpp 100% <100%> (ø)
src/identities/wif.cpp 100% <100%> (ø) ⬆️
src/transactions/builder.cpp 76.78% <100%> (+0.42%) ⬆️
src/transactions/deserializer.cpp 88.38% <100%> (+0.38%) ⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e361e3...025323b. Read the comment docs.

@sleepdefic1t
Copy link
Contributor Author

Codacy not config'd to watch develop.

You can view the current PR at codacy.com/project/sleepdefic1t/cpp-crypto.

The majority of the issues are documentation-style related.
Those and others will be resolved in a subsequent PR.

@sleepdefic1t sleepdefic1t changed the title [WIP]refactor(identities): improve implementation refactor(identities): improve implementation Jul 31, 2019
@sleepdefic1t sleepdefic1t marked this pull request as ready for review July 31, 2019 21:02
trailing return, comments, naming, etc
remove redundant strlen check.
@sleepdefic1t
Copy link
Contributor Author

this is ready, @faustbrian

@sleepdefic1t
Copy link
Contributor Author

@faustbrian, good to go 👍

@faustbrian faustbrian merged commit de42ef9 into ArkEcosystemArchive:develop Aug 8, 2019
@ghost ghost removed the Status: In Progress The issue or pull request is being worked on. label Aug 8, 2019
@sleepdefic1t sleepdefic1t deleted the refactor(identities)/improve-implementation branch August 11, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complexity: Undetermined Needs specialized, in-depth review. Type: Refactor The pull request improves or enhances an existing implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants