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

Aptos HSM Signer Testing #974

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Aptos HSM Signer Testing #974

wants to merge 7 commits into from

Conversation

mzabaluev
Copy link
Collaborator

@mzabaluev mzabaluev commented Dec 20, 2024

Summary

  • Categories: util.

Add a movement-signer-test crate, providing a local TestSigner, initially for the Ed25519 curve.

Changelog

  • movement-signer-test: local test support for the signing API defined in movement-signing.

Testing

Added a test verifying basic functionality of TestSigner in the movement-signer-test crate.

@l-monninger l-monninger changed the base branch from l-monninger/signing-api to main December 20, 2024 18:07
@l-monninger l-monninger changed the title Test signer Aptos HSM Signer Testing Dec 20, 2024
Provide From conversion impls to construct
PrivateKey and Signature from byte arrays.
The conversions into PrivateKey are fallible, while
a Signature is allowed to be constructed from any
64 bytes.
Add TestSigner providing an in-process Signing
implementation for tests.
TryFromBytes is not needed, neither is the public key
as a member of HashiCorpVault state.
@mzabaluev mzabaluev marked this pull request as ready for review December 23, 2024 11:07
/// Errors that occur when parsing signature or key material from byte sequences.
#[derive(Debug, thiserror::Error)]
#[error(transparent)]
pub struct CryptoMaterialError(Box<dyn Error + Send + Sync>);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be easier to have one error type for the crate. It avoids adding map_err every time when you need to extract the publickey from an u8 array, for example, and it eases the error management for external crate.
There's a reason you didn't add a new variant to the SignerError. Perhaps we need to rename it in a more generic name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like using the "fat" main error type for the much slimmer use case of TryFrom conversions.

@0xmovses 0xmovses mentioned this pull request Jan 6, 2025
14 tasks
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.

2 participants