Skip to content

perf(crypto): make address network byte check part of serde#3000

Merged
spkjp merged 7 commits intodevelopfrom
perf/crypto/address-check
Oct 4, 2019
Merged

perf(crypto): make address network byte check part of serde#3000
spkjp merged 7 commits intodevelopfrom
perf/crypto/address-check

Conversation

@spkjp
Copy link
Copy Markdown
Contributor

@spkjp spkjp commented Oct 1, 2019

Summary

The ajv address schema calls Base58.decodeCheck to validate the network byte. But during transaction serialization, the address is also decoded, so we end up doing it again. By performing the network byte validation during transaction serialization we can get rid of the decode call during schema validation. This makes multipayment processing approx. 25% faster.

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

spkjp added 2 commits October 1, 2019 06:05
The ajv address schema calls `Base58.decodeCheck` to validate the network
byte. But during transaction serialization, the address is also decoded,
so we end up doing it again. By performing the network byte validation
during transaction serialization we can get rid of the decode call during
schema validation. This makes multipayment processing approx. 25% faster.
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Oct 4, 2019

This pull request introduces 1 alert when merging b90dc92 into 575234e - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Brian Faust and others added 2 commits October 4, 2019 07:38
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 4, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@575234e). Click here to learn what that means.
The diff coverage is 76.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #3000   +/-   ##
==========================================
  Coverage           ?   29.79%           
==========================================
  Files              ?      424           
  Lines              ?    10168           
  Branches           ?      538           
==========================================
  Hits               ?     3030           
  Misses             ?     7093           
  Partials           ?       45
Impacted Files Coverage Δ
...atchers/src/transactions/valid-second-signature.ts 0% <ø> (ø)
packages/crypto/src/validation/schemas.ts 100% <ø> (ø)
packages/crypto/src/transactions/types/transfer.ts 100% <100%> (ø)
...ges/crypto/src/transactions/types/multi-payment.ts 50% <100%> (ø)
packages/crypto/src/transactions/serializer.ts 54.31% <100%> (ø)
packages/crypto/src/validation/keywords.ts 57.14% <100%> (ø)
...ackages/crypto/src/transactions/types/htlc-lock.ts 42.42% <33.33%> (ø)
packages/crypto/src/errors.ts 47.16% <50%> (ø)
packages/crypto/src/identities/address.ts 66.66% <85.71%> (ø)

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 575234e...47af879. Read the comment docs.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 4, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@575234e). Click here to learn what that means.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #3000   +/-   ##
==========================================
  Coverage           ?   65.83%           
==========================================
  Files              ?      424           
  Lines              ?    10168           
  Branches           ?      538           
==========================================
  Hits               ?     6694           
  Misses             ?     3429           
  Partials           ?       45
Impacted Files Coverage Δ
...atchers/src/transactions/valid-second-signature.ts 100% <ø> (ø)
packages/crypto/src/validation/schemas.ts 100% <ø> (ø)
packages/crypto/src/transactions/types/transfer.ts 100% <100%> (ø)
...ges/crypto/src/transactions/types/multi-payment.ts 100% <100%> (ø)
packages/crypto/src/transactions/serializer.ts 87.93% <100%> (ø)
packages/crypto/src/validation/keywords.ts 87.5% <100%> (ø)
...ackages/crypto/src/transactions/types/htlc-lock.ts 100% <100%> (ø)
packages/crypto/src/errors.ts 79.24% <50%> (ø)
packages/crypto/src/identities/address.ts 95.83% <85.71%> (ø)

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 575234e...47af879. Read the comment docs.

@spkjp spkjp merged commit 90a2f85 into develop Oct 4, 2019
@ghost ghost deleted the perf/crypto/address-check branch October 4, 2019 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants