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

Check Address is valid before sending #445

Closed
MSFTserver opened this Issue Aug 6, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@MSFTserver
Contributor

MSFTserver commented Aug 6, 2017

The Issue

issue comes about when trying to send funds to a non legit account like "aaaaaaaa"

Steps to reproduce

1.enter address aaaaaaaaaa
2.enter amount
3.send

Expected behaviour

i would expect this to do an api call to see if this address is existence and has funds linked to it or if it does not have a public or private key don't send the funds, and have error pop up if it doesnt work

Actual behaviour

it claims it sent the funds to the non existence account

System Configuration

  • LBRY Daemon version: 0.14.2
  • LBRY App version: 0.14.3
  • LBRY Installation ID: 9JKT6nrdGYoFKshVLmjnbfUdqNvHRkzWmsLXk5xNdbXzGEwpraZ6MVg1KoD5jrBrf2
  • Operating system: Windows 10

Anything Else

Screenshots

@tzarebczan

This comment has been minimized.

Show comment
Hide comment
@tzarebczan

tzarebczan Aug 6, 2017

Member

We may be able to do some semantic checks on the address (length, etc) but checking if the address exists or if it has public / private keys is impossible.

Member

tzarebczan commented Aug 6, 2017

We may be able to do some semantic checks on the address (length, etc) but checking if the address exists or if it has public / private keys is impossible.

@MSFTserver

This comment has been minimized.

Show comment
Hide comment
@MSFTserver

MSFTserver Aug 6, 2017

Contributor

even just an error saying it didn't send would be nice at this point even if it doesn't exist and it doesn't send it says it sent and its in my queue

Contributor

MSFTserver commented Aug 6, 2017

even just an error saying it didn't send would be nice at this point even if it doesn't exist and it doesn't send it says it sent and its in my queue

@MSFTserver

This comment has been minimized.

Show comment
Hide comment
@MSFTserver

MSFTserver Aug 6, 2017

Contributor

image

Contributor

MSFTserver commented Aug 6, 2017

image

@kauffj

This comment has been minimized.

Show comment
Hide comment
@kauffj

kauffj Aug 6, 2017

Member

This issue exists in the daemon as well, and there are likely changes required for both. Here's the daemon ticket: lbryio/lbry#828

Member

kauffj commented Aug 6, 2017

This issue exists in the daemon as well, and there are likely changes required for both. Here's the daemon ticket: lbryio/lbry#828

@kauffj

This comment has been minimized.

Show comment
Hide comment
@kauffj

kauffj Aug 6, 2017

Member

On the app side of this, the proper solution is probably one of:

  • Create a component, FormFieldWalletAddress, that verifies the value is a valid address on blur/submit
  • Modify FormField to take a regexp parameter, that matches on blur/submit, and submit the proper regexp as defined in lbryuri.
Member

kauffj commented Aug 6, 2017

On the app side of this, the proper solution is probably one of:

  • Create a component, FormFieldWalletAddress, that verifies the value is a valid address on blur/submit
  • Modify FormField to take a regexp parameter, that matches on blur/submit, and submit the proper regexp as defined in lbryuri.
@akinwale

This comment has been minimized.

Show comment
Hide comment
@akinwale

akinwale Aug 8, 2017

Member

I think we can run a base58 address validation check at the very least. Perhaps, something like https://github.com/wzbg/base58check/blob/master/index.js#L30-L47 which would require the use of an external module.

Member

akinwale commented Aug 8, 2017

I think we can run a base58 address validation check at the very least. Perhaps, something like https://github.com/wzbg/base58check/blob/master/index.js#L30-L47 which would require the use of an external module.

@kauffj kauffj changed the title from Check Address is existing before sending to Check Address is valid before sending Aug 14, 2017

@kauffj

This comment has been minimized.

Show comment
Hide comment
@kauffj

kauffj Aug 30, 2017

Member

Per discussion in meeting, the correct resolution for this is still #445 (comment)

Member

kauffj commented Aug 30, 2017

Per discussion in meeting, the correct resolution for this is still #445 (comment)

@kauffj

This comment has been minimized.

Show comment
Hide comment
@kauffj

kauffj Sep 6, 2017

Member

Merged, will be released in 0.15.1.

Member

kauffj commented Sep 6, 2017

Merged, will be released in 0.15.1.

@kauffj kauffj added this to the September 11 milestone Sep 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment