Skip to content

Aptos HSM Signer Testing - #974

Open
mzabaluev wants to merge 7 commits into
mainfrom
mikhail/test-signer
Open

Aptos HSM Signer Testing#974
mzabaluev wants to merge 7 commits into
mainfrom
mikhail/test-signer

Conversation

@mzabaluev

@mzabaluev mzabaluev commented Dec 20, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor 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
15 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