feat: Added the ability to get a public key for an account #436

FroVolod · 2025-01-16T19:21:37Z

Also resolves #371

FroVolod · 2025-01-17T07:19:00Z

race-of-sloths · 2025-01-17T07:19:33Z

@FroVolod Thank you for your contribution! Your pull request is now a part of the Race of Sloths!
Weekly streak is on the road, smart strategy! Secure your streak with another PR!

Current status: waiting for scoring

We're waiting for maintainer to score this pull request with @race-of-sloths score [0,1,2,3,5,8,13] command. Alternatively, autoscoring [1,2] will be applied for this pull request

What is the Race of Sloths

Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow

For contributors:

Tag @race-of-sloths inside your pull requests
Wait for the maintainer to review and score your pull request
Check out your position in the Leaderboard
Keep weekly and monthly streaks to reach higher positions
Boast your contributions with a dynamic picture of your Profile

For maintainers:

Score pull requests that participate in the Race of Sloths and receive a reward
Engage contributors with fair scoring and fast responses so they keep their streaks
Promote the Race to the point where the Race starts promoting you
Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands

For contributors
- Include a PR: @race-of-sloths include to enter the Race with your PR
For maintainers:
- Invite contributor @race-of-sloths invite to invite the contributor to participate in a race or include it, if it's already a runner.
- Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
- Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
- Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

akorchyn

Good work. A few comments left to think a bit.

akorchyn · 2025-01-17T11:46:28Z

src/commands/account/get_public_key/mod.rs

+    #[strum_discriminants(strum(
+        message = "from-ledger                 - Get the public key stored on your Ledger Nano device"
+    ))]
+    /// Get the public key stored on your Ledger Nano device


Not sure if these comments have any value as they are repeating text from the message above.

--help uses these comments:

Doesn't it use strum macro above ?

strum macro processes these messages with various functions

akorchyn · 2025-01-17T11:47:07Z

src/commands/account/get_public_key/from_seed_phrase/mod.rs

+        _previous_context: crate::GlobalContext,
+        scope: &<PublicKeyFromSeedPhrase as interactive_clap::ToInteractiveClapContextScope>::InteractiveClapContextScope,
+    ) -> color_eyre::eyre::Result<Self> {
+        let seed_phrase_hd_path_default = slipped10::BIP32Path::from_str("m/44'/397'/0'").unwrap();


I believe, we could configure BIP32Path in other handlers

I think we should stay consistent here

Example. It actuallyt seems to be skipped, but we can at least have a structure similar way

akorchyn · 2025-01-17T11:53:17Z

src/commands/account/get_public_key/from_legacy_keychain/mod.rs

+                    let signer_access_key_file_path: std::path::PathBuf = {
+                        if previous_context.offline {
+                            eprintln!(
+                            "\nThe signer's public key cannot be verified and retrieved offline."


But why ? I still think we can easily process the directory and return user a key. Just with a warning that this key wasn't checked that it's active on chain

We can easily process the directory and return the key to the user. But we promised the user that they would be able to get the public key with full access.
For example, an account has 10 key files. We give the user the public key and say that we don't know if it is active or has full access.

akorchyn · 2025-01-17T11:57:04Z

src/commands/account/get_public_key/from_keychain/mod.rs

+#[derive(Debug, Clone, interactive_clap::InteractiveClap)]
+#[interactive_clap(input_context = crate::GlobalContext)]
+#[interactive_clap(output_context = PublicKeyFromKeychainContext)]
+pub struct PublicKeyFromKeychain {


It's a bit weird, to be honest. Maybe we shouldn't have that. Because to get a key from a keychain, we need to fetch a key before-hand (e.g., from the chain). So it doesn't really make sense.
Maybe we can iterate somehow over keychain entries and parse it that way....

it works:

Screen.Recording.2025-01-17.at.21.01.13.mov

start

942ebb6

FroVolod requested review from akorchyn, dj8yfo and PolyProgrammist as code owners January 16, 2025 19:21

FroVolod added 2 commits January 16, 2025 21:32

clippy

a86a54d

refactored

2c74d26

akorchyn reviewed Jan 17, 2025

View reviewed changes

FroVolod marked this pull request as draft January 17, 2025 14:45

refactored PublicKeyFromSeedPhrase

e8d49d1

FroVolod marked this pull request as ready for review January 17, 2025 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added the ability to get a public key for an account #436

feat: Added the ability to get a public key for an account #436

FroVolod commented Jan 16, 2025 •

edited

Loading

FroVolod commented Jan 17, 2025

race-of-sloths commented Jan 17, 2025

akorchyn left a comment

akorchyn Jan 17, 2025

FroVolod Jan 17, 2025

akorchyn Jan 17, 2025 •

edited

Loading

FroVolod Jan 17, 2025

akorchyn Jan 17, 2025

akorchyn Jan 17, 2025

akorchyn Jan 17, 2025

FroVolod Jan 17, 2025

akorchyn Jan 17, 2025

FroVolod Jan 17, 2025

akorchyn Jan 17, 2025

FroVolod Jan 17, 2025 •

edited

Loading

feat: Added the ability to get a public key for an account #436

Are you sure you want to change the base?

feat: Added the ability to get a public key for an account #436

Conversation

FroVolod commented Jan 16, 2025 • edited Loading

FroVolod commented Jan 17, 2025

race-of-sloths commented Jan 17, 2025

akorchyn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akorchyn Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FroVolod Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

FroVolod commented Jan 16, 2025 •

edited

Loading

akorchyn Jan 17, 2025 •

edited

Loading

FroVolod Jan 17, 2025 •

edited

Loading