-
Notifications
You must be signed in to change notification settings - Fork 176
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
Improve transaction verifier clarity #3771
Conversation
FVM Benchstat comparisonThis branch with compared with the base branch onflow:master commit 3745401 The command Collapsed results for better readability
|
fvm/transactionVerifier.go
Outdated
// verifyAccountSignatures() is always called after getAccountKeys() | ||
// (i.e., accountKey is always initialized by the time | ||
// verifyAccountSignatures is called). | ||
verified bool |
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.
verified=true
reads as "signature was successfully verified" to me. Which is not what this is for. This is more like: "this signature already went through the verification step". Maybe a name like verificationDone
would make that clearer.
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.
good point. i'll update the name
As per Leo's suggestions. Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com>
387bf21
to
d410289
Compare
bors merge |
As per Leo's suggestions.
Co-authored-by: Leo Zhang (zhangchiqing) zhangchiqing@gmail.com