-
Notifications
You must be signed in to change notification settings - Fork 97
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
types: Reject non canonical addresses in DecodeAddress #595
Conversation
@@ -94,6 +95,13 @@ func DecodeAddress(addr string) (a Address, err error) { | |||
|
|||
// Checksum is good, copy address bytes into output | |||
copy(a[:], addressBytes) | |||
|
|||
// Check if address is canonical |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as this looks very good.
Minor nit: consider modding "correct, and ..." to "correct and canonical, and ..." in the function comment.
@@ -65,7 +65,8 @@ func (a *Address) UnmarshalText(text []byte) error { | |||
} | |||
|
|||
// DecodeAddress turns a checksum address string into an Address object. It | |||
// checks that the checksum is correct, and returns an error if it's not. | |||
// checks that the checksum is correct and whether the address is canonical, | |||
// and returns an error if it's not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Checks that the address to decode is a canonical representation (least significant bit is 00, i.e. the ending character is one of "AEIMQUY4"). Adds a test to check that non-canonical addresses are rejected.
Note that the server (and most goal commands) will reject non-canonical addresses, but our SDK clients don't seem to check this.
Closes #274