kredits/kredits-web
9
0
Fork 0
Code Issues 22 Pull Requests 1 Actions Releases 18 Activity

Validate Ethereum addresses in input fields #144

Merged
raucao merged 2 commits from feature/validate_eth_addresses into feature/signup 2019-07-24 15:30:52 +00:00
Conversation 6 Commits 2 Files Changed 4
+247 -8
raucao commented 2019-07-22 15:31:07 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Adds address validation to the address inputs.

Importing the function from web3-utils, because ethers.js doesn't have any such utility function. 🙁

Adds address validation to the address inputs. Importing the function from `web3-utils`, because `ethers.js` doesn't have any such utility function. :slightly_frowning_face:
👍 1
bumi commented 2019-07-24 09:42:45 +00:00 (Migrated from github.com)
Copy Link
Copy Source

ethers has that getchcksumaddress function that does some check: https://github.com/ethers-io/ethers.js/blob/061b0eae1d4c570aedd9bee1971afa43fcdae1a6/src.ts/utils/address.ts#L22

I guess that duplication of isValidAccount can not be prevented as we need that method in both files?

ethers has that getchcksumaddress function that does some check: https://github.com/ethers-io/ethers.js/blob/061b0eae1d4c570aedd9bee1971afa43fcdae1a6/src.ts/utils/address.ts#L22 I guess that duplication of `isValidAccount` can not be prevented as we need that method in both files?
bumi (Migrated from github.com) reviewed 2019-07-24 09:44:31 +00:00
app/components/add-contributor/component.js
@@ -1,7 +1,9 @@
import Component from '@ember/component';
bumi (Migrated from github.com) commented 2019-07-24 09:44:31 +00:00
Copy Link
Copy Source

why is this now ethAddress and was account before?

why is this now `ethAddress` and was `account` before?
raucao commented 2019-07-24 09:50:20 +00:00 (Migrated from github.com)
Copy Link
Copy Source

ethers has that getchcksumaddress function that does some check: https://github.com/ethers-io/ethers.js/blob/061b0eae1d4c570aedd9bee1971afa43fcdae1a6/src.ts/utils/address.ts#L22

That's not a check, that's actually creating a checksum address from a non-checksum one.

> ethers has that getchcksumaddress function that does some check: https://github.com/ethers-io/ethers.js/blob/061b0eae1d4c570aedd9bee1971afa43fcdae1a6/src.ts/utils/address.ts#L22 That's not a check, that's actually creating a checksum address from a non-checksum one.
raucao (Migrated from github.com) reviewed 2019-07-24 09:55:47 +00:00
app/components/add-contributor/component.js
@@ -1,7 +1,9 @@
import Component from '@ember/component';
raucao (Migrated from github.com) commented 2019-07-24 09:55:47 +00:00
Copy Link
Copy Source

Typo. Thanks.

Typo. Thanks.
raucao commented 2019-07-24 09:58:13 +00:00 (Migrated from github.com)
Copy Link
Copy Source

I guess that duplication of isValidAccount can not be prevented as we need that method in both files?

It can, but that would be way more complex and require more code than the de-duplication is worth. In essence, isAddress is what's already de-duplicated here, and using it on this or that property is just a declarative action.

> I guess that duplication of `isValidAccount` can not be prevented as we need that method in both files? It can, but that would be way more complex and require more code than the de-duplication is worth. In essence, `isAddress` is what's already de-duplicated here, and using it on this or that property is just a declarative action.
raucao commented 2019-07-24 09:59:00 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Good to merge.

Good to merge.
Sign in to join this conversation.
Reviewers
No Reviewers
bumi
raucao
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#144
Delete Branch "feature/validate_eth_addresses"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?