Skip to content

op-proposer, op-batcher: Support private keys in addition to mnemonics#3155

Merged
mslipper merged 1 commit intodevelopfrom
feat/op-proposer-private-key
Aug 1, 2022
Merged

op-proposer, op-batcher: Support private keys in addition to mnemonics#3155
mslipper merged 1 commit intodevelopfrom
feat/op-proposer-private-key

Conversation

@mslipper
Copy link
Collaborator

@mslipper mslipper commented Aug 1, 2022

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Aug 1, 2022

⚠️ No Changeset found

Latest commit: 0befcf2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Aug 1, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@mslipper mslipper force-pushed the feat/op-proposer-private-key branch from 39a95c3 to e1a3609 Compare August 1, 2022 19:39
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch 5 times, most recently from bc14ea2 to 4f832f3 Compare August 1, 2022 19:46
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch from 4f832f3 to e8647cb Compare August 1, 2022 19:55
@mslipper mslipper requested a review from trianglesphere August 1, 2022 19:57
@mslipper mslipper changed the title op-proposer: Support private keys in addition to mnemonics op-proposer, op-batcher: Support private keys in addition to mnemonics Aug 1, 2022
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch from e8647cb to 9f4a0c7 Compare August 1, 2022 20:09
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch 2 times, most recently from b4b587f to f57eee5 Compare August 1, 2022 20:18
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch from f57eee5 to b6e4ca7 Compare August 1, 2022 20:27
@mslipper mslipper force-pushed the feat/op-proposer-private-key branch from b6e4ca7 to 0befcf2 Compare August 1, 2022 20:30
@tynes
Copy link
Contributor

tynes commented Aug 1, 2022

I think @protolambda wanted to move to using encrypted keystores
https://github.com/ethereum/go-ethereum/blob/a0b88ce869d6be243c6b1900d69b9401f2d2c092/cmd/ethkey/generate.go#L109
https://github.com/ethereum/go-ethereum/blob/a0b88ce869d6be243c6b1900d69b9401f2d2c092/accounts/keystore/key.go#L66

I think this is fine because it won't require any changes in tooling or the way we handle secrets

@protolambda
Copy link
Contributor

Maybe ideally we define one go module that can support the loading of private keys, in various ways:

  • mnemonic + path
  • private key
  • encrypted keystore + password

That way we don't duplicate security-critical code, make it easy to audit, and easy to use.

Also ideally with discouraged use of mnemonic and private key. And warnings if you put the mnemonic, private key, or password in CLI instead of env var. (Since shell history / log streaming may log the CLI command and leak it)

I've been thinking we should have an op-service go module, for exactly this type of thing. As well as logging configuration, metrics, and pprof.

I'm fine with merging this PR as-is, but it would be nice if we can refactor it all into a common op-service module later.

@mslipper
Copy link
Collaborator Author

mslipper commented Aug 1, 2022

I agree with the op-service module. We have a lot of duplication among our various Go projects and it would be great to clean it up. We'll also need a separate "Signer" function to support things like Cloud HSMs in prod.

I'll merge this as-is, but will ticket the op-service work so we can get started on it.

@mslipper mslipper merged commit 357c770 into develop Aug 1, 2022
@mslipper mslipper deleted the feat/op-proposer-private-key branch August 1, 2022 22:37
@mslipper mslipper mentioned this pull request Aug 4, 2022
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.

4 participants