This repository has been archived by the owner on Jun 17, 2021. It is now read-only.
Enforce hex prefixing for address strings #241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request introduces 1 alert when merging 5670bdd into 3bbc948 - view on LGTM.com new alerts:
|
cgewecke
force-pushed
the
enforce-hex-value-format
branch
from
5670bdd to
ed7eb36
Compare
cgewecke
commented
Apr 23, 2020
cgewecke
commented
Apr 23, 2020
src/account.ts
Outdated
| if (!ethjsUtil.isHexString(input)) { | ||
| throw new Error(msg) | ||
| } | ||
| } |
There was a problem hiding this comment.
Using isHexString instead of isHexPrefixed here because the latter throws its own error if given a non-string input.
cgewecke
force-pushed
the
enforce-hex-value-format
branch
from
303668a to
cf60742
Compare
cgewecke
force-pushed
the
enforce-hex-value-format
branch
from
cf60742 to
11fc1c1
Compare
cgewecke
force-pushed
the
enforce-hex-value-format
branch
from
11fc1c1 to
78626a4
Compare
|
@cgewecke Great, thanks for picking this up! 😄 Did I mention that you should really question what's written up in the API Change Table and give this also your own thoughts respectively reflect if you see these as useful/adequate API changes. I've written this up quite some time ago and there actually was not yet much discussion around this yet, so I would encourage all others to give this some thought as well. |
holgerd77
approved these changes
Apr 24, 2020
| isValidAddress('x2f015c60e0be116b1f0cd534704db9c92118fb6a') | ||
| }) | ||
| assert.throws(function() { | ||
| isValidAddress('0X52908400098527886E0F7030069857D2E4169EE7') |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #172 Breaking API Changes
The proposal to make hex param checking more strict in #172 comes out of review discussions in #170. There @holger suggested:
PR adds logic to enforce hex-prefixing for all address string inputs. Affected methods are:
isPrecompileis excluded because it's targeted for removal in #242.