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

feat(signers): adds alloy-signer-gcp #94

Merged
merged 9 commits into from
Jan 18, 2024

Conversation

georgewhewell
Copy link
Contributor

@georgewhewell georgewhewell commented Dec 27, 2023

Motivation

Closes #71

Solution

upstreams https://github.com/georgewhewell/ethers-gcp-kms-signer

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Cargo.toml Outdated Show resolved Hide resolved
crates/signer-gcp/Cargo.toml Outdated Show resolved Hide resolved

/// Reference to a GCP KeyRing.
#[derive(Clone, Debug)]
pub struct GcpKeyRingRef {
Copy link
Member

Choose a reason for hiding this comment

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

check my understanding:

  • the GcpKeyRingRef specifies a keyring by project, location, and name
  • it is extended with a key_id and a version to form a full key specifier
  • that key specifier is passed in the requests

we may want to clean it up with something like this?

struct KeySpecifier { keyring: GcpKeyRingRef, key_id: String, version: u64 }

impl KeySpecifier { 
    fn to_key_version_ref(&self) -> String {
        // current behavior of `GcpKeyRingRef::to_key_version_ref`
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that matches my understanding- eventually all these parameters are just used to construct a single string so have implemented KeySpecifier(String) since the inputs are never used individually

crates/signer-gcp/src/signer.rs Outdated Show resolved Hide resolved
@gakonst
Copy link
Member

gakonst commented Jan 16, 2024

@DaniPopes whenever you have time let's try to get this over the line!

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Thanks!
FYI I've edited the description to use GitHub keywords when referencing issues.

@DaniPopes DaniPopes merged commit 51a8253 into alloy-rs:main Jan 18, 2024
17 checks passed
@georgewhewell georgewhewell deleted the grw/feat/signer-gcp branch April 22, 2024 17:23
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.

[Feature] Add GCP KMS signer
4 participants