-
Notifications
You must be signed in to change notification settings - Fork 597
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
asserts: add support for account-key constraints #12988
Conversation
and take them into account when verifying assertion signatures
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.
Looks really good, just two small suggestions
t, ok := hm["type"] | ||
if !ok { | ||
return nil, fmt.Errorf("type header constraint mandatory in asserions constraints") | ||
} | ||
tstr, ok := t.(string) | ||
if !ok { | ||
return nil, fmt.Errorf("type header constraint must be a string") | ||
} |
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.
checkNotEmptyString
?
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.
the issue doing that would be less specific errors
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.
Looks great to me, just one tiny nitpick. Thank you!
} | ||
|
||
func accountKeyFormatAnalyze(headers map[string]interface{}, body []byte) (formatnum int, err error) { | ||
formatnum = 0 |
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.
(very nitpick): s/formatnum/formatNum ?
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.
historically the code uses formatnum consistently
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.
Thank you, this looks good. But worth checking the suggestions from Philip I think.
and take them into account when verifying assertion signatures
and take them into account when verifying assertion signatures