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

add aws secret manager support #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gingfrederik
Copy link

add support for aws secret manager for storing the eth private key and cryptowatch key

marketmaker.js Outdated Show resolved Hide resolved
config.json.EXAMPLE Show resolved Hide resolved
marketmaker.js Outdated Show resolved Hide resolved
@0xtrooper
Copy link
Collaborator

Hey, thanks for this PR.

  1. Can you explain what the advantage this PR brings, as it would just switch from stroing the private key directly to storing the secret to fetch the private key.
  2. Please dont use prettier for PR for other projects. Its really hard to tell what has changed.

@gingfrederik
Copy link
Author

Hi Trooper,
Appreciate for taking the time to review the PR.

  1. It benefits the user using the AWS service or local machine. AWS IAM can handle the credentials for fetching private keys. AWS Secrets Manager will safely store the key for users, also it will have the ability to know who and where the data has been fetched. They will never need to type their private key on the machine, not even in env. There will be NO information be stored in the machine
  2. Apology for that, it is my first time trying to contribute.

@gingfrederik gingfrederik force-pushed the feat/aws_secret_manager branch from 39c1583 to 3d7000c Compare June 6, 2022 04:23
@gingfrederik
Copy link
Author

Sorry for the force-push. I stop the prettier.
The part mentioned above has been fixed

Appreciate for taking the time to review the PR again.

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