-
Notifications
You must be signed in to change notification settings - Fork 374
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: add simple address validity check #1303
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1303 +/- ##
==========================================
- Coverage 47.93% 47.89% -0.05%
==========================================
Files 372 372
Lines 62990 62990
==========================================
- Hits 30192 30166 -26
- Misses 30341 30360 +19
- Partials 2457 2464 +7 ☔ View full report in Codecov by Sentry. |
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.
LGTM 💯
I just wonder if we can get rid of these in the same PR:
gno/examples/gno.land/p/demo/grc/grc20/util.gno
Lines 7 to 12 in 34d78b3
func checkIsValidAddress(addr std.Address) error { | |
if addr.String() == "" { | |
return ErrInvalidAddress | |
} | |
return nil | |
} |
gno/examples/gno.land/p/demo/grc/grc721/util.gno
Lines 9 to 14 in 34d78b3
func isValidAddress(addr std.Address) error { | |
if addr.String() == "" { | |
return ErrInvalidAddress | |
} | |
return nil | |
} |
gno/examples/gno.land/p/demo/grc/grc1155/util.gno
Lines 9 to 14 in 34d78b3
func isValidAddress(addr std.Address) bool { | |
if addr.String() == "" { | |
return false | |
} | |
return true | |
} |
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.
Looks good 💯
go.mod
Outdated
github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c | ||
github.com/btcsuite/btcd v0.22.3 | ||
github.com/btcsuite/btcd/btcutil v1.0.0 | ||
github.com/btcsuite/btcutil v1.0.2 | ||
github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce |
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.
Ehh, I'd say upgrading btcd/btcutil is something that should be done in another PR, especially considering #1064
I'd suggest reverting changes here + go.sum
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.
It was accidental, sorry. Will revert them :)
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.
Oops, my mistake. I attempted to fix the previous commit but didn't thoroughly check it.
Edit: nope, was introduced before my commit; I'll fix now.
Signed-off-by: moul <94029+moul@users.noreply.github.com>
1fc803d
to
1fb7ab6
Compare
## Description This PR adds a simple `Valid()` check to the Address type. It is there to prevent most human errors, and doesn't actually validate the math behind bech32 (for now) - it relies only on the length of the address. Running `gno test .` will run the unit test for the `Valid()` function. Closes: gnolang#1298 <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: moul <94029+moul@users.noreply.github.com>
## Description This PR adds a simple `Valid()` check to the Address type. It is there to prevent most human errors, and doesn't actually validate the math behind bech32 (for now) - it relies only on the length of the address. Running `gno test .` will run the unit test for the `Valid()` function. Closes: gnolang#1298 <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: moul <94029+moul@users.noreply.github.com>
Description
This PR adds a simple
Valid()
check to the Address type. It is there to prevent most human errors, and doesn't actually validate the math behind bech32 (for now) - it relies only on the length of the address.Running
gno test .
will run the unit test for theValid()
function.Closes: #1298
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description