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

Hardcode mnemonics #133

Merged
merged 7 commits into from
Mar 26, 2024
Merged

Hardcode mnemonics #133

merged 7 commits into from
Mar 26, 2024

Conversation

LuqiPan
Copy link
Contributor

@LuqiPan LuqiPan commented Mar 22, 2024

Closes: #5

Testing

Not sure if I should add some tests that assert the addresses are expected, and if so, where should I add the tests?

@LuqiPan LuqiPan requested a review from turadg March 22, 2024 02:58
@LuqiPan LuqiPan marked this pull request as ready for review March 22, 2024 02:58
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

Looking good.

Couple concerns:

  • we shouldn't add any files to .agoric in the image. That's for validator state. We should actually be removing any files there that aren't in a production chain (optional to do in this PR)
  • there shouldn't be any new files in the repo root. The keys are a concern of synthetic-chain, not the repo.

We could put the files somewhere else, but I think it would be best not to make these into files. Just set put them as vars in env_setup.sh.

@LuqiPan LuqiPan merged commit 5e46218 into main Mar 26, 2024
2 checks passed
@LuqiPan LuqiPan deleted the 5-hardcode-mnemonics branch March 26, 2024 01:02
@LuqiPan
Copy link
Contributor Author

LuqiPan commented Mar 27, 2024

Well, turns out we do have places that depends on, for example, /root/.agoric/user1.key be present, namely

  • contract/Makefile in Agoric/dapp-offer-up
  • contract/Makefile in Agoric/dapp-agoric-basics
  • proposals/34:upgrade-10/upgradeHelpers.js in Agoric/agoric-3-proposals

Maybe we can add them back into the container, but not under /root/.agoric directory? What do you think, @turadg?

@turadg
Copy link
Member

turadg commented Mar 27, 2024

What do you think, @turadg?

They're hard-coded now so we could copy the keys.

Or put them somewhere else as you suggest, if we want to make it easy for downstream to look them up by name. I'd suggest a place like the upgrade-test-scripts path so that it's more clear they're not part of production

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.

hard-code governance keys in SCM
2 participants