Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Add length check to getLisk32AddressFromPublicKey #9124

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

Phanco
Copy link
Contributor

@Phanco Phanco commented Oct 26, 2023

What was the problem?

This PR resolves #9083

How was it solved?

Added length check to getLisk32AddressFromPublicKey

How was it tested?

Unit test updated

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #9124 (8559d8f) into release/6.1.0 (185e3fa) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           release/6.1.0    #9124   +/-   ##
==============================================
  Coverage          84.34%   84.35%           
==============================================
  Files                652      652           
  Lines              23966    23970    +4     
  Branches            3489     3490    +1     
==============================================
+ Hits               20215    20219    +4     
  Misses              3751     3751           
Files Coverage Δ
elements/lisk-cryptography/src/address.ts 96.55% <100.00%> (+0.12%) ⬆️
elements/lisk-cryptography/src/constants.ts 100.00% <100.00%> (ø)

@Phanco Phanco requested review from sitetester and shuse2 October 27, 2023 08:47
@Phanco Phanco marked this pull request as ready for review October 27, 2023 08:47
elements/lisk-cryptography/src/address.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/address.spec.ts Outdated Show resolved Hide resolved
@Phanco
Copy link
Contributor Author

Phanco commented Oct 30, 2023

In lisk-sdk, lisk-cryptography is exported earlier than lisk-framework. Hence this element could not retrieve the constant from lisk-framework and I afraid it will be too risky to re-arrange the exports just for this purporse. So I defined the constant locally at lisk-cryptography

@Phanco Phanco requested a review from sitetester October 30, 2023 16:33
elements/lisk-cryptography/src/constants.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/address.spec.ts Outdated Show resolved Hide resolved
@Phanco Phanco requested a review from sitetester October 31, 2023 08:38
@sitetester
Copy link
Contributor

sitetester commented Oct 31, 2023

In lisk-sdk, lisk-cryptography is exported earlier than lisk-framework. Hence this element could not retrieve the constant from lisk-framework and I afraid it will be too risky to re-arrange the exports just for this purporse. So I defined the constant locally at lisk-cryptography

Each package should be independent of SDK, hence each package should have it's own constant.

Copy link
Contributor

@sitetester sitetester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elements/lisk-cryptography/test/address.spec.ts

Currently, there are describe blocks starting with #. For future PRs, this practice should be discouraged, as there is no such standard

@shuse2 shuse2 merged commit 36a1df9 into release/6.1.0 Oct 31, 2023
10 checks passed
@shuse2 shuse2 deleted the 9083-add-buffer-length-check branch October 31, 2023 10:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants