-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat: full std.Address
validation
#1799
feat: full std.Address
validation
#1799
Conversation
1a3a235
to
db58dad
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1799 +/- ##
==========================================
- Coverage 47.49% 47.48% -0.01%
==========================================
Files 388 388
Lines 61311 61329 +18
==========================================
+ Hits 29117 29124 +7
- Misses 29756 29767 +11
Partials 2438 2438 ☔ View full report in Codecov by Sentry. |
Hey @r3v4s, thanks for the contribution. We initially thought about doing this, but then went with a simple length check because it would cover most human errors (ie inputting nothing for an address, or accidentally deleting a letter). I'll leave the reference for the PR/issue that introduced the initial address validation: #1298 (comment). We had an issue as the comment states, but now since #859 has been merged, your PR can work. cc @thehowl Please see why the CI is failing 🙏 |
Hello @leohhhn, if my memory serves me right, this issue was covered in Miami and I forgot to tag #1298 As you said, checking length does cover most human errors. However chain returns different result when used address has length of 40, but invalid character
I think second transaction should fail too. P.S I'll fix cause of CI Failing |
@leohhhn Fixed test case causing CI to fail. Not sure If I have to assign you as reviewer or not.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you make a sound argument for addition. implementation looks good, testing can be improved. thanks!
9fa5ff9
to
5e8e935
Compare
std.Address
validationstd.Address
validation
Length of
std.Address
was only condition to check whether address is valid or not.Fixing it by checking whether address can be decoded
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description