-
Notifications
You must be signed in to change notification settings - Fork 80
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
Signing e2e tests #981
base: musitdev/alloy-integration
Are you sure you want to change the base?
Signing e2e tests #981
Conversation
|
||
#[derive(Clone, Debug, Serialize, Deserialize, Default)] | ||
pub enum KeyProvider { | ||
#[default] |
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.
Enum variants should not be all caps: https://rust-lang.github.io/api-guidelines/naming.html#casing-conforms-to-rfc-430-c-case
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.
Yes, I thought it was UpperCamelCase when the variant contains fields and all upper case when it's a constant. I'll update.
|
||
fn main() { | ||
env_default!(get_aws_key_id, "AWS_KEY_ID", String); | ||
let awskms_key_id = get_aws_key_id().expect("AWS_KEY_ID not defined in env."); |
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.
This doesn't really show me a lot to do with how you integrate this with the mcr_settlement_client
crate which is where the e2e testing would take place. That's what I would like to see earlier than later in this next sprint.
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.
mcr_settlement_client/test_client_settlement test
is more an integration test because it doesn't need all the node setup process. It only uses an anvil process. It can be updated the same way it's done here: https://github.com/movementlabsxyz/movement/blob/musitdev/alloy-integration/util/signing/integrations/eth/tests/aws_test.rs
In this PR, I propose a way to manage how the setup/config process can be handled to integrate the signing API in the node execution process (setup->config->execution). To do that, we need a way to define the key during the setup and a way to get the key when the node is executed. For example, the setup key definition need to handle when the execution connects to AWS KMS with auth token (local on a PC for example) or a local signer provider when use in the github CI. The why I define 2 setup and one test.
If we agree on this conception, the integration in the Suzuka node / bridge will be only to update the corresponding setup to define the key the same way as the setup example and update how the Alloy provider is loaded, as done in the test.
Summary
protocol-units
,networks
,scripts
,util
,cicd
, ormisc
.e2e tests must follow the normal deployment and execution process of Movement products that use signing.
This process is:
The Key Manager will be used to allow to define a key/provider during the setup and to provide the key when the application is executed.
The e2e tests are divided the same way: a setup binary that defines the key used by the test, and the test (see
movement-signer-e2e-test
crate).This PR adds the following crates to integrate this process:
Another point is that to test AWS KMS or HashiCorp Vault the authentication data must be provided to run the test. It can be acceptable on the developer PC but not in the CI because they will be visible to everyone. As a workaround, a local signer provider will be developed to allow to sign with a newly generated key during the test. This local provided will be used for CI test.
Changelog
Testing
One test is defined to illustrate the process. It signs a transfer tx but depending on the executed setup it doesn't use the same key/provider.
Two setups are defined: a local to use the local provider and an AWS KMS to use AWS one.
I've started to add some process compose script to run the test. It is still a work in progress.
Outstanding issues
Key Manager API must be updated to follow this model and the
Signing
that is not object safe can be an issue to get dynamic Signer depending on the executed setup. Some work must be done here to see if it can be use dynamically using a box or an enum for example.