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.

Sign up for GitHub

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

Jump to bottom

[sdk] account recovery from derive path #4374

Merged
merged 1 commit into from
Dec 4, 2022
Merged

[sdk] account recovery from derive path #4374

merged 1 commit into from
Dec 4, 2022

Conversation

stevenatcrypto
Copy link
Contributor

@stevenatcrypto stevenatcrypto commented Sep 20, 2022 •
edited by geekflyer
Loading

Description

Close #4089

Reference the previous TypeScript PR #2073 and this code of Solana SDK (and dependency crate ed25519-dalek-bip32 and tiny-bip39).

Test Plan

Add an unit-test case test_recover_account_from_derive_path.

This change is Reviewable

@jjleng
Copy link
Contributor

jjleng commented Sep 20, 2022

it would be good if you can cross-validate this with the TS SDK implementation.

@stevenatcrypto
Copy link
Contributor Author

stevenatcrypto commented Sep 20, 2022 •
edited
Loading

it would be good if you can cross-validate this with the TS SDK implementation.

@jjleng I added a test case test_recover_account_from_derive_path which uses the same validation values (mnemonics, derive path and expected address) with this test case of TypeScript.

I may misunderstand about cross-validate. Do you mean we should invoke TypeScript function to test? Could you help give some instruction? Thanks a lot.

@jjleng jjleng enabled auto-merge (rebase) October 3, 2022 05:27
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
auto-merge was automatically disabled October 17, 2022 07:09

Head branch was pushed to by a user without write access

@davidiw
Copy link
Contributor

davidiw commented Oct 28, 2022

Please rebase and we can get this landed.

@stevenatcrypto stevenatcrypto force-pushed the sdk/recover-account-from-derive-path branch 2 times, most recently from 7b17ad7 to ac06cb6 Compare November 6, 2022 12:19
@davidiw
Copy link
Contributor

davidiw commented Dec 3, 2022

Delays are hard. Please try again. Hit me up on discord davidiw#1895 if you don't get a faster response next time.

@stevenatcrypto stevenatcrypto force-pushed the sdk/recover-account-from-derive-path branch from ac06cb6 to 82379cc Compare December 4, 2022 10:38
@stevenatcrypto
Copy link
Contributor Author

@davidiw Just rebased this PR 🙏

@davidiw davidiw enabled auto-merge (squash) December 4, 2022 12:54
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

✅ Forge suite land_blocking success on 82379ccc26322f66be73ce941978b13432818490

performance benchmark with full nodes : 6904 TPS, 5757 ms latency, 9900 ms p99 latency,no expired txns
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 82379ccc26322f66be73ce941978b13432818490

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 82379ccc26322f66be73ce941978b13432818490 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7131 TPS, 5381 ms latency, 8200 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 82379ccc26322f66be73ce941978b13432818490
compatibility::simple-validator-upgrade::single-validator-upgrade : 4579 TPS, 8771 ms latency, 11400 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 82379ccc26322f66be73ce941978b13432818490
compatibility::simple-validator-upgrade::half-validator-upgrade : 5036 TPS, 8339 ms latency, 11000 ms p99 latency,no expired txns
4. upgrading second batch to new version: 82379ccc26322f66be73ce941978b13432818490
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6556 TPS, 5849 ms latency, 9200 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 82379ccc26322f66be73ce941978b13432818490 passed
Test Ok

@davidiw davidiw merged commit 071885c into aptos-labs:main Dec 4, 2022
@Markuze Markuze mentioned this pull request Dec 5, 2022
Closed
areshand pushed a commit to areshand/aptos-core-1 that referenced this pull request Dec 18, 2022
@stevenatcrypto @areshand
Implement recovering account from derive path. (aptos-labs#4374)
6e178d2
@Markuze Markuze mentioned this pull request Dec 26, 2022
Closed
@Markuze Markuze mentioned this pull request Jan 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjleng jjleng jjleng approved these changes

@davidiw davidiw davidiw approved these changes

@banool banool Awaiting requested review from banool

@gregnazario gregnazario Awaiting requested review from gregnazario

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request][Rust SDK] Create account from derive path (as typescript SDK)
3 participants
@stevenatcrypto @jjleng @davidiw