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

refactor: move transaction logic out of crypto #2188

Merged
merged 16 commits into from Mar 2, 2019

Conversation

Projects
None yet
4 participants
@supaiku0
Copy link
Contributor

supaiku0 commented Mar 2, 2019

Proposed changes

crypto should only be responsible for:

  • de/serialization
  • sign/verify

The transaction logic itself (apply/revert to wallet and more) should therefore not be part of crypto. This PR introduces core-transactions and decouples the crypto transactions from the wallet logic.

Next up are the transaction specific pieces from the transaction pool and SPV (separate PR).

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build (changes that affect the build system)
  • Docs (documentation only changes)
  • Test (adding missing tests or fixing existing tests)
  • Other... Please describe:

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Mar 2, 2019

The ci/circleci: test-node10-2 job is failing as of ee3e5ba8a3cf37f4ed5d3031a2cd14f57d99e168. Please review the logs for more information.

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

supaiku0 added some commits Mar 2, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 2, 2019

Codecov Report

Merging #2188 into 2.3 will increase coverage by 0.15%.
The diff coverage is 79.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##              2.3    #2188      +/-   ##
==========================================
+ Coverage   77.96%   78.11%   +0.15%     
==========================================
  Files         308      320      +12     
  Lines        8314     8386      +72     
  Branches     1154     1115      -39     
==========================================
+ Hits         6482     6551      +69     
- Misses       1797     1800       +3     
  Partials       35       35
Impacted Files Coverage Δ
...to/src/transactions/types/delegate-registration.ts 100% <ø> (+4%) ⬆️
packages/crypto/src/transactions/types/ipfs.ts 46.66% <ø> (+4.56%) ⬆️
...ges/crypto/src/transactions/types/multi-payment.ts 34.61% <ø> (+1.28%) ⬆️
packages/crypto/src/utils.ts 100% <ø> (ø) ⬆️
...pto/src/transactions/types/delegate-resignation.ts 77.77% <ø> (+24.44%) ⬆️
packages/crypto/src/validation/ajv-wrapper.ts 96.96% <ø> (ø) ⬆️
...crypto/src/transactions/types/timelock-transfer.ts 42.85% <ø> (+2.85%) ⬆️
...s/crypto/src/transactions/types/multi-signature.ts 29.16% <ø> (+8.11%) ⬆️
.../crypto/src/transactions/types/second-signature.ts 100% <ø> (ø) ⬆️
...kages/crypto/src/transactions/types/transaction.ts 87.71% <ø> (-3.4%) ⬇️
... and 35 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 a986cb8...f7d891a. Read the comment docs.

@ArkEcosystem ArkEcosystem deleted a comment from ArkEcosystemBot Mar 2, 2019

@faustbrian faustbrian changed the title refactor: move tx logic out of crypto refactor: move transaction logic out of crypto Mar 2, 2019

@faustbrian faustbrian merged commit 317e86e into ArkEcosystem:2.3 Mar 2, 2019

6 checks passed

ci/circleci: test-node10-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-2 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-2 Your tests passed on CircleCI!
Details

vasild added a commit that referenced this pull request Mar 4, 2019

Merge remote-tracking branch 'ArkEcosystem/core/2.3' into blockid
* ArkEcosystem/core/2.3:
  feat(core): add support for forced updates (#2190)
  feat(core): add support for reinstalling the current version (#2192)
  feat(core-tester-cli): add the ability to send multiple waves (#2191)
  feat(core-tester-cli): test the vendor field (#2189)
  refactor: move transaction logic out of crypto (#2188)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.