Skip to content

Alloy signing integration - #970

Closed
musitdev wants to merge 21 commits into
mainfrom
musitdev/alloy-integration
Closed

Alloy signing integration#970
musitdev wants to merge 21 commits into
mainfrom
musitdev/alloy-integration

Conversation

@musitdev

@musitdev musitdev commented Dec 19, 2024

Copy link
Copy Markdown
Contributor

Summary

  • RFCs: Link to RFC, Link to RFC, or $\emptyset$.
  • Categories: any of protocol-units, networks, scripts, util, cicd, or misc.

First version of Alloy crate integration for signing with the API. Inspired from the Alloy AWS implementation alloy_signer_aws::AwsSigner

Changelog

Testing

Provide an integration test to test AWS KMS integration. The test can't be used in CI because it needs AWS KMS auth env variable to set to run (AWS_ACCESS_KEY and AWS_SECRET_KEY).
The test start a local Anvil eth node and execute a transfer Tx using an AWS KMS key defined with the AWS_KEY_ID env var.

AWS_KEY_ID=<>AW key id> AWS_ACCESS_KEY=<access key> AWS_SECRET_KEY=<secret key> cargo test -p movement-signing-alloy test_aws_kms_send_tx -- --nocapture

Outstanding issues

}

#[async_trait::async_trait]
impl alloy_network::TxSigner<AlloySignature> for HsmSigner {

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.

Have you already worked on the code for how this gets integrated? I think it's still probably easier for us to change the usage s.t. we are producing a raw transaction and signing that rather than tightly coupling with a target API.

/// Instantiate a new signer from an existing `Client` and key ID.
///
/// Retrieves the public key from HMS and calculates the Ethereum address.
pub async fn new(

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.

Because this returns a result, I would call it try_new

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.

Changed

@musitdev
musitdev changed the base branch from l-monninger/signing-api to main December 24, 2024 10:19
@0xmovses 0xmovses mentioned this pull request Jan 6, 2025
15 tasks
@musitdev

Copy link
Copy Markdown
Contributor Author

Integrated in another PR.

@musitdev musitdev closed this Feb 25, 2025
@musitdev
musitdev deleted the musitdev/alloy-integration branch April 7, 2025 13:55
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.

3 participants