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

fix: require that c32 addresses start with S #14

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

jcnelson
Copy link
Member

@jcnelson jcnelson commented Apr 15, 2021

Right now, c32addressDecode() will decode the address bP2CT665Q0JB7P39TZ7BST0QYCAQSMJWBZK8QT35J. Given that clients of this library are expecting that this method not decode invalid Stacks addresses, this method should be fixed to require that the leading character is S. This PR addresses this. Context: stacks-network/stacks-utils#21

@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #14 (5a9d37b) into master (5b0e305) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
+ Coverage   91.62%   91.70%   +0.08%     
==========================================
  Files           5        5              
  Lines         191      193       +2     
  Branches       32       33       +1     
==========================================
+ Hits          175      177       +2     
  Misses          5        5              
  Partials       11       11              
Impacted Files Coverage Δ
src/address.ts 95.74% <100.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b0e305...5a9d37b. Read the comment docs.

@aulneau
Copy link

aulneau commented Apr 15, 2021

@jcnelson thanks for this. I'm curious if there is a better solution than requiring that the string start with S? Or are there any number of c32 addresses and they are all "valid" regardless of what it starts with?

@jcnelson
Copy link
Member Author

So, way back in the day (Stacks 1.0), it was decided that the first letter of the address should simply be S for branding purposes. It's entirely cosmetic. But, the code never required this -- if you mistyped the first letter, you'd still have the same address (because the hash160 is encoded in the letters following the S). However, upstream clients aren't expecting this.

@jcnelson
Copy link
Member Author

Soooo.....can I merge this?

@jcnelson jcnelson merged commit 5777e44 into master Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants