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(crypto): de/serialization #2175

Merged
merged 20 commits into from Mar 1, 2019

Conversation

Projects
None yet
4 participants
@supaiku0
Copy link
Contributor

supaiku0 commented Feb 28, 2019

Proposed changes

Cleans up some AIP29 stuff and also enforces the block schema in the Block constructor. This is in preparation for the switch to serialized payloads.

I still need to fix a few failing tests (mainly core-database/core-blockchain) due to the schema change.

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)

@supaiku0 supaiku0 requested a review from faustbrian Feb 28, 2019

@supaiku0 supaiku0 requested a review from kristjank as a code owner Feb 28, 2019

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@air1one @faustbrian - please review this in the next few days. Be sure to explicitly select labels so I know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 28, 2019

@supaiku0 The ci/circleci: test-node10-1 job is failing as of 35ddfdd92b047d77a3a8fd7d1b4b9ef7e062b37c. Please review the logs for more information.

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

faustbrian and others added some commits Feb 28, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 1, 2019

Codecov Report

Merging #2175 into 2.3 will increase coverage by 7.35%.
The diff coverage is 89.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##              2.3    #2175      +/-   ##
==========================================
+ Coverage   70.61%   77.96%   +7.35%     
==========================================
  Files         391      308      -83     
  Lines        9630     8314    -1316     
  Branches     1250     1154      -96     
==========================================
- Hits         6800     6482     -318     
+ Misses       2795     1797     -998     
  Partials       35       35
Impacted Files Coverage Δ
packages/crypto/src/validation/schemas.ts 100% <ø> (ø) ⬆️
...s/core-database-postgres/src/models/transaction.ts 100% <ø> (ø) ⬆️
packages/crypto/src/transactions/types/schemas.ts 100% <ø> (ø) ⬆️
packages/crypto/src/transactions/types/transfer.ts 100% <100%> (ø) ⬆️
packages/crypto/src/validation/keywords.ts 97.95% <100%> (+0.52%) ⬆️
...ypto/src/transactions/deserializers/transaction.ts 89.18% <100%> (-0.43%) ⬇️
...crypto/src/transactions/serializers/transaction.ts 96.21% <100%> (+0.05%) ⬆️
...ges/crypto/src/transactions/deserializers/block.ts 100% <100%> (ø) ⬆️
...kages/crypto/src/transactions/serializers/block.ts 100% <100%> (ø) ⬆️
packages/crypto/src/errors.ts 83.56% <50%> (-0.95%) ⬇️
... and 86 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 bdd07d3...2264bf1. Read the comment docs.

@faustbrian faustbrian merged commit a986cb8 into ArkEcosystem:2.3 Mar 1, 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 1, 2019

Merge remote-tracking branch 'ArkEcosystem/core/2.3' into blockid
* ArkEcosystem/core/2.3:
  refactor(crypto): de/serialization (#2175)
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.