-
Notifications
You must be signed in to change notification settings - Fork 653
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
refactor(InMemorySigner) factory methods to return Signer interface #12472
Conversation
8407e4f
to
d8ad1e2
Compare
d8ad1e2
to
908484b
Compare
Following the issue near#11531, the Signer/ValidatorSigner wrapper interfaces should be used. The commit implements migration to Signer wrapper for following InMemorySigner factory functions: - InMemorySigner::from_seed - InMemorySigner::from_file Additionally: - The Signer interface is extented by get_acount_id method - Added From<Signer> for KeyFile implementation
908484b
to
b6dcadc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12472 +/- ##
==========================================
- Coverage 70.13% 70.12% -0.02%
==========================================
Files 841 841
Lines 169899 169898 -1
Branches 169899 169898 -1
==========================================
- Hits 119156 119135 -21
- Misses 45605 45624 +19
- Partials 5138 5139 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
assert!(k1.public_key == k2.public_key && k1.secret_key == k2.secret_key); | ||
let data: &[u8] = b"Example data for signature test"; | ||
assert!(k1.public_key() == k2.public_key() && k1.sign(&data) == k2.sign(&data)); |
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.
Just curious, why we changed from k1.secret_key == k2.secret_key
to k1.sign(&data) == k2.sign(&data)
?
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 test here checked if given the same seeds we end-up with the same public and secret keys. Since here we changed from InMemorySigner to Signer we do not have a public members now and we need public getters.
So I wasn't sure if giving a public access method to "secret_key" is the best way to go (especially when the only attempts to retrieve it were in tests) so other way to compare SecretKeys would be to sing something with them (as there is no random and one-shot nonce generated inside the SecretKey while signing we can expect the same result).
@staffik |
Following the issue #11531, the Signer/ValidatorSigner wrapper interfaces should be used. The commit implements migration to Signer wrapper for following InMemorySigner factory functions:
Additionally: