[cli] Allow for AIP-80 compliant strings while importing #15039

GhostWalker562 · 2024-10-21T23:14:09Z

Description

Add support for AIP-80 formatted private keys. With this formatting, also restrict secp256k1 private keys.

https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-80.md

How Has This Been Tested?

Built locally and imported private keys starting with ed25519-priv using the CLI argument --private-key, manual input, and configs.

Key Areas to Review

Should cover all of the private key import cases in the CLI.

Importing through --private-key
Importing through config
Importing through manual input

Private key variants

works:
0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19a41003f
ed25519-priv-0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19a41003f

fails:
0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19aINVALID
secp256k1-priv-0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19a41003f
INVALID-priv-0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19a41003f
ed25519-priv-0x03a1bfe72a61bec320a89c6e9e031cda663a825e7af0f1ac153bd5b19aINVALID

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-10-21T23:14:13Z

⏱️ 20m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-cargo-deny	6m	🟩 🟩 ⬜ 🟩
check-dynamic-deps	3m	🟩 🟩 🟩 🟩
rust-move-tests	2m	🟩
general-lints	2m	🟩 🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩
rust-move-tests	2m	🟩
rust-move-tests	2m	🟩
rust-move-tests	52s	⬜
file_change_determinator	45s	🟩 🟩 🟩 🟩
permission-check	15s	🟩 🟩 🟩 🟩
permission-check	10s	🟩 🟩 🟩 🟩

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

gregnazario

Looks fine, we'll have to of course put saving to the new format in a future release. Probably will need eventually to have a proper feature flag system.

Let's add an optional prefix to check for, when stripping, and set it by default on the CLI to ed25519

crates/aptos/src/common/utils.rs

banool

Looks good to me!

github-actions · 2024-11-13T01:41:06Z

✅ Forge suite `realistic_env_max_load` success on `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`

two traffics test: inner traffic : committed: 14298.29 txn/s, latency: 2784.06 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 5400 ms), latency samples: 5436540
two traffics test : committed: 100.01 txn/s, latency: 1929.32 ms, (p50: 1400 ms, p70: 1600, p90: 3800 ms, p99: 6300 ms), latency samples: 1760
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.018, avg: 1.431", "ConsensusProposalToOrdered: max: 0.323, avg: 0.294", "ConsensusOrderedToCommit: max: 0.363, avg: 0.351", "ConsensusProposalToCommit: max: 0.652, avg: 0.645"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.12s no progress at version 38191 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.42s no progress at version 1935870 (avg 7.77s) [limit 15].
Test Ok

github-actions · 2024-11-13T01:42:49Z

✅ Forge suite `framework_upgrade` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`

Compatibility test results for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16 (PR)
Upgrade the nodes to version: fe89305c3d75a84fcdfdc27be08ee65904dbcb16
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1423.49 txn/s, submitted: 1427.07 txn/s, failed submission: 3.58 txn/s, expired: 3.58 txn/s, latency: 2194.21 ms, (p50: 2100 ms, p70: 2200, p90: 3300 ms, p99: 4500 ms), latency samples: 127280
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1396.26 txn/s, submitted: 1398.94 txn/s, failed submission: 2.68 txn/s, expired: 2.68 txn/s, latency: 2161.78 ms, (p50: 2100 ms, p70: 2300, p90: 3300 ms, p99: 4900 ms), latency samples: 125220
5. check swarm health
Compatibility test for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16 passed
Upgrade the remaining nodes to version: fe89305c3d75a84fcdfdc27be08ee65904dbcb16
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1404.84 txn/s, submitted: 1408.44 txn/s, failed submission: 3.60 txn/s, expired: 3.60 txn/s, latency: 2295.72 ms, (p50: 2100 ms, p70: 2400, p90: 4200 ms, p99: 5900 ms), latency samples: 125000
Test Ok

github-actions · 2024-11-13T01:43:51Z

✅ Forge suite `compat` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`

Compatibility test results for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16 (PR)
1. Check liveness of validators at old version: 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9
compatibility::simple-validator-upgrade::liveness-check : committed: 15044.70 txn/s, latency: 2247.35 ms, (p50: 1800 ms, p70: 1900, p90: 3000 ms, p99: 9100 ms), latency samples: 488200
2. Upgrading first Validator to new version: fe89305c3d75a84fcdfdc27be08ee65904dbcb16
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7810.28 txn/s, latency: 3685.68 ms, (p50: 4100 ms, p70: 4300, p90: 4400 ms, p99: 4500 ms), latency samples: 144860
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 8001.26 txn/s, latency: 4073.34 ms, (p50: 4400 ms, p70: 4400, p90: 4500 ms, p99: 4500 ms), latency samples: 265000
3. Upgrading rest of first batch to new version: fe89305c3d75a84fcdfdc27be08ee65904dbcb16
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 5268.78 txn/s, latency: 5434.51 ms, (p50: 4200 ms, p70: 7500, p90: 8100 ms, p99: 8600 ms), latency samples: 96200
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7304.37 txn/s, latency: 4172.63 ms, (p50: 4200 ms, p70: 4200, p90: 4300 ms, p99: 6500 ms), latency samples: 275140
4. upgrading second batch to new version: fe89305c3d75a84fcdfdc27be08ee65904dbcb16
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11513.45 txn/s, latency: 2417.41 ms, (p50: 2600 ms, p70: 2700, p90: 2900 ms, p99: 3000 ms), latency samples: 203880
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10100.80 txn/s, latency: 3112.05 ms, (p50: 2700 ms, p70: 3000, p90: 5900 ms, p99: 7600 ms), latency samples: 344660
5. check swarm health
Compatibility test for 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16 passed
Test Ok

GhostWalker562 requested review from gregnazario and banool as code owners October 21, 2024 23:14

GhostWalker562 force-pushed the cli/support-aip8-imports branch 2 times, most recently from 1b37337 to 22d2c9c Compare October 28, 2024 20:35

gregnazario approved these changes Oct 28, 2024

View reviewed changes

crates/aptos/src/common/utils.rs Outdated Show resolved Hide resolved

GhostWalker562 force-pushed the cli/support-aip8-imports branch 2 times, most recently from d93e7cd to e0380d3 Compare October 28, 2024 23:43

GhostWalker562 force-pushed the cli/support-aip8-imports branch 2 times, most recently from 5943a9e to 5fcb1cc Compare November 7, 2024 00:31

banool approved these changes Nov 8, 2024

View reviewed changes

GhostWalker562 force-pushed the cli/support-aip8-imports branch 4 times, most recently from a1b1a41 to ad2ab71 Compare November 12, 2024 00:14

GhostWalker562 added 4 commits November 12, 2024 16:57

Allow for AIP-80 compliant strings in imports

febb8a4

Add ProfileConfig deserialization

9201d1c

Restrict prefixes to ed25519

618abf9

Rebase to CHANGELOG.md

fe89305

GhostWalker562 force-pushed the cli/support-aip8-imports branch from ad2ab71 to fe89305 Compare November 13, 2024 00:57

GhostWalker562 enabled auto-merge (squash) November 13, 2024 01:12

This comment has been minimized.

Sign in to view

GhostWalker562 merged commit 60bfc6b into main Nov 13, 2024
92 checks passed

GhostWalker562 deleted the cli/support-aip8-imports branch November 13, 2024 01:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] Allow for AIP-80 compliant strings while importing #15039

[cli] Allow for AIP-80 compliant strings while importing #15039

GhostWalker562 commented Oct 21, 2024 •

edited

Loading

trunk-io bot commented Oct 21, 2024 •

edited

Loading

gregnazario left a comment

banool left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 13, 2024

github-actions bot commented Nov 13, 2024

github-actions bot commented Nov 13, 2024

[cli] Allow for AIP-80 compliant strings while importing #15039

[cli] Allow for AIP-80 compliant strings while importing #15039

Conversation

GhostWalker562 commented Oct 21, 2024 • edited Loading

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Oct 21, 2024 • edited Loading

gregnazario left a comment

Choose a reason for hiding this comment

banool left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 13, 2024

✅ Forge suite realistic_env_max_load success on fe89305c3d75a84fcdfdc27be08ee65904dbcb16

github-actions bot commented Nov 13, 2024

✅ Forge suite framework_upgrade success on 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16

github-actions bot commented Nov 13, 2024

✅ Forge suite compat success on 58c5c53eaffe680eb2fc4cfbddf05a867fecadb9 ==> fe89305c3d75a84fcdfdc27be08ee65904dbcb16

GhostWalker562 commented Oct 21, 2024 •

edited

Loading

trunk-io bot commented Oct 21, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`

✅ Forge suite `framework_upgrade` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`

✅ Forge suite `compat` success on `58c5c53eaffe680eb2fc4cfbddf05a867fecadb9` ==> `fe89305c3d75a84fcdfdc27be08ee65904dbcb16`