Skip to content
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

Validation of addresses #183

Open
youkaicountry opened this issue Feb 13, 2023 · 2 comments
Open

Validation of addresses #183

youkaicountry opened this issue Feb 13, 2023 · 2 comments
Labels

Comments

@youkaicountry
Copy link
Contributor

CLI currently allows non-address values to be passed in as address arguments.

Better validation in the parser should be added, by improving the regex. Additionally, the IsValidAddress function from util should be invoked by the parser.

Checksum should be ensured:
https://medium.com/coinmonks/how-to-generate-a-bitcoin-address-step-by-step-9d7fcbf1ad0b

The self keyword should also be allowed. The regex should ensure self is recognized as an address, and validation should not be done on it. Addresses that are "self" should resolve to the currently opened wallet address, if any.

PR #151 adds better validation via regex

@mvandeberg
Copy link
Member

mvandeberg commented Mar 3, 2023

From #131

From the CLI

transfer 0.1 15UWU3BYRRJTA5I2U9BR19MDGFQBC7Z2RN
Note the casing of the address. The receiving targeted address was 15uWu3bYRrJta5i2U9br19mDGFqBc7z2rn.
The transaction was successful, but the receiving address was unexpected. 15UWU3BYRRJTA5

https://koinosblocks.com/tx/0x1220108592bf3ec2e86f5748791e3774db0d34933ceb4d1ab994e554227c2c45f7ef

It looks like there are a few issues. Please split and reassign if they pass triage.

The operation in the call contract uses the bad receiving address. I didn't verify the transaction args, but perhaps the CLI should validate and reject invalid addresses.
The transfer event shows that 15UWU3BYRRJTA5 should have received TKOIN. The sending address was decremented correctly, but the receiving account only received MANA? It difficult to tell if this is a UI bug in the explorer or if it's an actual error where MANA was recorded but the TKOIN was not.
https://koinosblocks.com/address/15UWU3BYRRJTA5
It's odd that the blockchain even accepts an address with this format. Do accounts not use a well known algorithm and checkum? It seems like you can transfer TKOIN to a bunch of odd address string.
transfer 0.3 somewhere
https://koinosblocks.com/tx/0x1220419647e106ab20332d37629674dae3745911902a43a773276ae6611da276dc02
https://koinosblocks.com/address/somewhere

@koinos-ci
Copy link

This issue is stale because it has been open for 30 days with no activity.

@koinos-ci koinos-ci added the stale label Apr 3, 2023
@sgerbino sgerbino added story and removed stale labels Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants