Closed alnoki closed 1 month ago
vercel[bot]
commented
5 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| developer-docs-nextra | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Sep 13, 2024 7:14pm |
netlify[bot]
commented
5 months ago | Name | Link |
|---|---|
| Latest commit | 543aaa6836e063fb3c645aedce01b5c94fb85583 |
| Latest deploy log | https://app.netlify.com/sites/aptos-developer-docs/deploys/6687052621664a000836bcbe |
| Deploy Preview | https://deploy-preview-367--aptos-developer-docs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
alnoki
commented
5 months ago Per @gregnazario in https://github.com/aptos-labs/developer-docs/pull/367#discussion_r1623092630:
Should probably just change the existing rotate key to use this then, and then you can get rid of most of this tutorial?
I had previously tried to incorporate Ledger rotation support in the CLI https://github.com/aptos-labs/aptos-core/pull/11151 and got blocked by Ledger's inability to sign the RotationProofChallenge, which is why I was using the new rotate_authentication_key_call here. (Unfortunately, however, the method presented in this PR doesn't update the OriginatingAddress table).
However I did some digging and it now looks like it now may be possible to sign the RotationProofChallenge with Ledger.
Hence I've created https://github.com/aptos-labs/aptos-core/issues/13515 to track the CLI feature, which I agree would be more straightforward cc @gedigi
alnoki
commented
4 months ago @gregnazario thanks for the above comments. I believe I've addressed all where applicable
Just flagging here again that this PR requires https://github.com/aptos-labs/aptos-core/pull/11151, without which there is no support for ledger auth key rotation
I'd try and do everything in one PR (docs and CLI source updates), but alas there is no longer a monorepo
Seems like encouraging people to rotate back and forth between different keys is not a good idea.
Before ledger support became available, plenty of projects launched under hot keys (and many still do, unfortunately, due to the >20kb issue on ledger), and without the CLI updates proposed in the linked PR, there is no way to secure an upgradeable package with anything other than a hot key
I agree that key rotation is advanced and should be reserved for specific use cases, but securing Move packages with accounts that don't use hot keys is important for ecosystem OpSec
alnoki
commented
2 months ago @hariria I believe I've addressed all your recent comments and that I had previously addressed all comments from @gregnazario
I've updated the PR description for more clarity on the associated aptos-core PRs
igor-p
commented
1 month ago Discussed with @gregnazario, ok to merge.
Background
@davidiw @gedigi @hariria @hardsetting @xbtmatt
Per in-person discussions re: authentication key rotation and Ledger.
This relies on the new features from the
aptos-corePR https://github.com/aptos-labs/aptos-core/pull/11151 which was subsequently broken into 3 smaller PRs:Changes
bashtotextto avoid syntax highlighting issues in IDE.Testing
From in
apps/nextra:aptos-coreCLI from source using https://github.com/aptos-labs/aptos-core/pull/14309 (ormain, once it merges)Checklist
pnpm spellcheck? (This is failing due to issues with content I have not modified)pnpm fmt?pnpm lint? (This is failing due to issues with content I have not modified)