Check for valid address when sending credits on Wallet page #489

Closed
alexliebowitz wants to merge 2 commits from validate-address-send into master
alexliebowitz commented 2017-08-22 10:44:03 +02:00 (Migrated from github.com)

Fixes #445. Ready to go, but https://github.com/lbryio/lbry/pull/867 needs to be merged first

Fixes #445. Ready to go, but https://github.com/lbryio/lbry/pull/867 needs to be merged first
kauffj (Migrated from github.com) reviewed 2017-08-22 10:44:03 +02:00
kauffj commented 2017-08-23 00:57:07 +02:00 (Migrated from github.com)

is_address will not be an API call. See discussion elsewhere.

More importantly, this is not solved as suggested by #445. See my comment here.

Please solve this by simply adding a regexp parameter to FormField or a descendant of FormField, and a regexp constant to either lbry or lbryuri. Then an invalid address can be detected and shown to the user entirely in widget-land -- no need for a modal.

I'm closing this PR as I think very little of it will apply to the appropriate solution.

(Btw, if you think the proposed approach is incorrect, I am, as always, open to discussion. I suspect in this base it simply wasn't seen.)

`is_address` will not be an API call. See discussion elsewhere. More importantly, this is not solved as suggested by #445. See my comment [here](https://github.com/lbryio/lbry-app/issues/445#issuecomment-320525524). Please solve this by simply adding a regexp parameter to `FormField` or a descendant of `FormField`, and a regexp constant to either `lbry` or `lbryuri`. Then an invalid address can be detected and shown to the user entirely in widget-land -- no need for a modal. I'm closing this PR as I think very little of it will apply to the appropriate solution. (Btw, if you think the proposed approach is incorrect, I am, as always, open to discussion. I suspect in this base it simply wasn't seen.)
alexliebowitz commented 2017-08-23 01:31:36 +02:00 (Migrated from github.com)

@kauffj

Ugh, yes, you're right, I missed it. I skimmed the whole thread and somehow took away that your idea was about implementation details (not UX issues like modal vs. inside the widget). Then proceeded to forget you even left a comment. Should have gone over the thread again before I started work.

Is it really just a regex check to confirm a valid address, though? The JS libraries seem to do some base58 voodoo to check: https://github.com/cryptocoinjs/coinstring/blob/master/lib/coinstring.js#L53-L61 -> https://github.com/cryptocoinjs/coinstring/blob/master/lib/coinstring.js#L24-L39

In which case we could use the validator function from one of libraries (or write our own small validator function) and add a validator function param to form fields instead of a regex param.

@kauffj Ugh, yes, you're right, I missed it. I skimmed the whole thread and somehow took away that your idea was about implementation details (not UX issues like modal vs. inside the widget). Then proceeded to forget you even left a comment. Should have gone over the thread again before I started work. Is it really just a regex check to confirm a valid address, though? The JS libraries seem to do some base58 voodoo to check: https://github.com/cryptocoinjs/coinstring/blob/master/lib/coinstring.js#L53-L61 -> https://github.com/cryptocoinjs/coinstring/blob/master/lib/coinstring.js#L24-L39 In which case we could use the validator function from one of libraries (or write our own small validator function) and add a validator function param to form fields instead of a regex param.
kauffj commented 2017-08-23 01:40:22 +02:00 (Migrated from github.com)

@alexliebowitz I think it is just /^b[HJK][1-9A-Za-z][^OIl]{33}/ or something close to that...

@alexliebowitz I think it is just `/^b[HJK][1-9A-Za-z][^OIl]{33}/` or something close to that...
alexliebowitz commented 2017-08-23 02:39:01 +02:00 (Migrated from github.com)

@kauffj: @kaykurokawa says "regex can tell you if its base 58 encoded, but it won't validate it because there 's a checksum and a header byte". Checksum seems important, since it will notice typos where 1 character is off but it's still base58.

@kauffj: @kaykurokawa says "regex can tell you if its base 58 encoded, but it won't validate it because there 's a checksum and a header byte". Checksum seems important, since it will notice typos where 1 character is off but it's still base58.

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#489
No description provided.