This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
Refactor SignerSource to expose DerivationPath to other kinds of signers #16933
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CriesofCarrots
force-pushed
the
client-url
branch
3 times, most recently
from
4b70ac0 to
2561420
Compare
CriesofCarrots
force-pushed
the
client-url
branch
from
2561420 to
fe3ba60
Compare
t-nelson
reviewed
Apr 29, 2021
t-nelson
previously approved these changes
Apr 29, 2021
| } if j == junk)); | ||
| assert!(matches!( | ||
| parse_signer_source(&junk), | ||
| Err(SignerSourceError::IoError(_)) |
CriesofCarrots
force-pushed
the
client-url
branch
from
b354dea to
838b33d
Compare
Codecov Report
@@ Coverage Diff @@
## master #16933 +/- ##
========================================
Coverage 82.7% 82.8%
========================================
Files 416 416
Lines 116629 116479 -150
========================================
- Hits 96532 96480 -52
+ Misses 20097 19999 -98 |
mergify bot
pushed a commit
that referenced
this pull request
Apr 29, 2021
…ers (#16933) * One use statement * Add stdin uri scheme * Convert parse_signer_source to return Result * A-Z deps * Convert Usb data to Locator * Pull DerivationPath out of Locator * Wrap SignerSource to share derivation_path * Review comments * Check Filepath existence, readability in parse_signer_source (cherry picked from commit d6f30b7) # Conflicts: # Cargo.lock # clap-utils/src/memo.rs # remote-wallet/src/locator.rs # sdk/Cargo.toml
mergify bot
pushed a commit
that referenced
this pull request
Apr 29, 2021
…ers (#16933) * One use statement * Add stdin uri scheme * Convert parse_signer_source to return Result * A-Z deps * Convert Usb data to Locator * Pull DerivationPath out of Locator * Wrap SignerSource to share derivation_path * Review comments * Check Filepath existence, readability in parse_signer_source (cherry picked from commit d6f30b7) # Conflicts: # sdk/Cargo.toml
mergify bot
added a commit
that referenced
this pull request
Apr 29, 2021
…ers (backport #16933) (#16941) * Refactor SignerSource to expose DerivationPath to other kinds of signers (#16933) * One use statement * Add stdin uri scheme * Convert parse_signer_source to return Result * A-Z deps * Convert Usb data to Locator * Pull DerivationPath out of Locator * Wrap SignerSource to share derivation_path * Review comments * Check Filepath existence, readability in parse_signer_source (cherry picked from commit d6f30b7) # Conflicts: # sdk/Cargo.toml * Fix conflicts Co-authored-by: Tyera Eulberg <teulberg@gmail.com> Co-authored-by: Tyera Eulberg <tyera@solana.com>
mergify bot
added a commit
that referenced
this pull request
Apr 29, 2021
…ers (backport #16933) (#16940) * Refactor SignerSource to expose DerivationPath to other kinds of signers (#16933) * One use statement * Add stdin uri scheme * Convert parse_signer_source to return Result * A-Z deps * Convert Usb data to Locator * Pull DerivationPath out of Locator * Wrap SignerSource to share derivation_path * Review comments * Check Filepath existence, readability in parse_signer_source (cherry picked from commit d6f30b7) # Conflicts: # Cargo.lock # clap-utils/src/memo.rs # remote-wallet/src/locator.rs # sdk/Cargo.toml * Fix conflicts * Fix legacy compile test Co-authored-by: Tyera Eulberg <teulberg@gmail.com> Co-authored-by: Tyera Eulberg <tyera@solana.com>
jeffwashington
pushed a commit
to jeffwashington/solana
that referenced
this pull request
Apr 30, 2021
…ers (solana-labs#16933) * One use statement * Add stdin uri scheme * Convert parse_signer_source to return Result * A-Z deps * Convert Usb data to Locator * Pull DerivationPath out of Locator * Wrap SignerSource to share derivation_path * Review comments * Check Filepath existence, readability in parse_signer_source
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
DerivationPath parsing doesn't lend itself to code sharing (only available for
SignerSource::Usb-type signers, within remote-wallet).Summary of Changes
Refactor it:
DerivationPath::from_uri()method that implements the DerivationPath parsing previously inLocator::new_from_uri(); move test cases to target derivation-path specificallySignerSourceto a struct that wraps the oldkindand includes and optionalderivation_pathToward #5246