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(marketmaker): encrypt eth private key and cryptowatch api key #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

0xrookie
Copy link

@0xrookie 0xrookie commented Jun 3, 2022

use crypto-js to encrypt the private key and api key in the config.json

@0xtrooper
Copy link
Collaborator

Hey, thanks for makeing this PR. But this would make it impossible to restart bots automatically. Also this cant be used on services like heroku. We could merge it, if you change it to a optional feature.

@0xrookie
Copy link
Author

0xrookie commented Jun 6, 2022

Hey, thanks for makeing this PR. But this would make it impossible to restart bots automatically. Also this cant be used on services like heroku. We could merge it, if you change it to a optional feature.

Hi Trooper, I have submitted another commit which change this feature to an optional choice

Copy link
Collaborator

@0xtrooper 0xtrooper left a comment

Choose a reason for hiding this comment

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

Hey, sorry for the late reply. Pleas see the comment and also add a entry to the README. Thanks :)

@@ -22,6 +25,13 @@ let FEE_TOKEN = null;
let uniswap_error_counter = 0;
let chainlink_error_counter = 0;

// Encrypt the eth private key and cryptowatch api key if you want
let args = minimist(process.argv.slice(2), {boolean: 'encrypt-key'});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should use the config.json as well. I would sugest adding a entry like: "encryptKey": true

Then do the if check like: if(MM_CONFIG?.encryptKey)

Dont think it is a good idea to change the usage for this function. We controlle verything else using the config as well.

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