Refactor remote-wallet path parsing (backport #16798) (#16894)

* SDK: More conversions for `Pubkey`

(cherry picked from commit 9b7120bf73)

* SDK: More conversion for `DerivationPath`

(cherry picked from commit 722de942ca)

* remote-wallet: Add helpers for locating remote wallets

(cherry picked from commit 64fcb792c2)

* remote-wallet: Plumb `Locator` into `RemoteWalletInfo`

(cherry picked from commit 3d12be29ec)

* remote-wallet: `derivation-path` crate doesn't like empty trailing child indexes

(cherry picked from commit 4ce4f04c58)

* remote-wallet: Move `Locator` to its own module

(cherry picked from commit cac666d035)

Co-authored-by: Trent Nelson <trent@solana.com>

Cargo.lock

@@ -3055,6 +3055,15 @@ dependencies = [
  "prost",
 ]
 
+[[package]]
+name = "qstring"
+version = "0.7.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d464fae65fff2680baf48019211ce37aaec0c78e9264c84a3e484717f965104e"
+dependencies = [
+ "percent-encoding 2.1.0",
+]
+
 [[package]]
 name = "quick-error"
 version = "1.2.3"
@@ -5058,10 +5067,11 @@ dependencies = [
  "num-derive",
  "num-traits",
  "parking_lot 0.10.2",
+ "qstring",
  "semver 0.9.0",
  "solana-sdk",
  "thiserror",
- "url 2.2.0",
+ "uriparse",
 ]
 
 [[package]]

programs/bpf/Cargo.lock

@@ -2078,6 +2078,15 @@ dependencies = [
  "unicode-xid 0.2.0",
 ]
 
+[[package]]
+name = "qstring"
+version = "0.7.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d464fae65fff2680baf48019211ce37aaec0c78e9264c84a3e484717f965104e"
+dependencies = [
+ "percent-encoding",
+]
+
 [[package]]
 name = "quote"
 version = "0.6.13"
@@ -3303,10 +3312,11 @@ dependencies = [
  "num-derive 0.3.0",
  "num-traits",
  "parking_lot 0.10.2",
+ "qstring",
  "semver 0.9.0",
  "solana-sdk",
  "thiserror",
- "url",
+ "uriparse",
 ]
 
 [[package]]

remote-wallet/Cargo.toml

@@ -18,10 +18,11 @@ log = "0.4.11"
 num-derive = { version = "0.3" }
 num-traits = { version = "0.2" }
 parking_lot = "0.10"
+qstring = "0.7.2"
 semver = "0.9"
 solana-sdk = { path = "../sdk", version = "=1.6.7" }
 thiserror = "1.0"
-url = "2.1.1"
+uriparse = "0.6.3"
 
 [features]
 default = ["linux-static-hidraw"]

remote-wallet/src/ledger.rs

@@ -1,6 +1,7 @@
 use {
     crate::{
         ledger_error::LedgerError,
+        locator::Manufacturer,
         remote_wallet::{RemoteWallet, RemoteWalletError, RemoteWalletInfo, RemoteWalletManager},
     },
     console::Emoji,
@@ -9,7 +10,7 @@ use {
     num_traits::FromPrimitive,
     semver::Version as FirmwareVersion,
     solana_sdk::{derivation_path::DerivationPath, pubkey::Pubkey, signature::Signature},
-    std::{cmp::min, fmt, sync::Arc},
+    std::{cmp::min, convert::TryFrom, fmt, sync::Arc},
 };
 
 static CHECK_MARK: Emoji = Emoji("✅ ", "");
@@ -363,9 +364,8 @@ impl RemoteWallet for LedgerWallet {
     ) -> Result<RemoteWalletInfo, RemoteWalletError> {
         let manufacturer = dev_info
             .manufacturer_string()
-            .unwrap_or("Unknown")
-            .to_lowercase()
-            .replace(" ", "-");
+            .and_then(|s| Manufacturer::try_from(s).ok())
+            .unwrap_or_default();
         let model = dev_info
             .product_string()
             .unwrap_or("Unknown")

remote-wallet/src/lib.rs

@@ -1,5 +1,6 @@
 #![allow(clippy::integer_arithmetic)]
 pub mod ledger;
 pub mod ledger_error;
+pub mod locator;
 pub mod remote_keypair;
 pub mod remote_wallet;

remote-wallet/src/locator.rs

@@ -0,0 +1,656 @@
+use {
+    solana_sdk::{
+        derivation_path::{DerivationPath, DerivationPathError},
+        pubkey::{ParsePubkeyError, Pubkey},
+    },
+    std::{
+        convert::{Infallible, TryFrom, TryInto},
+        str::FromStr,
+    },
+    thiserror::Error,
+    uriparse::{URIReference, URIReferenceBuilder, URIReferenceError},
+};
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum Manufacturer {
+    Unknown,
+    Ledger,
+}
+
+impl Default for Manufacturer {
+    fn default() -> Self {
+        Self::Unknown
+    }
+}
+
+const MANUFACTURER_UNKNOWN: &str = "unknown";
+const MANUFACTURER_LEDGER: &str = "ledger";
+
+#[derive(Clone, Debug, Error, PartialEq)]
+#[error("not a manufacturer")]
+pub struct ManufacturerError;
+
+impl From<Infallible> for ManufacturerError {
+    fn from(_: Infallible) -> Self {
+        ManufacturerError
+    }
+}
+
+impl FromStr for Manufacturer {
+    type Err = ManufacturerError;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let s = s.to_ascii_lowercase();
+        match s.as_str() {
+            MANUFACTURER_LEDGER => Ok(Self::Ledger),
+            _ => Err(ManufacturerError),
+        }
+    }
+}
+
+impl TryFrom<&str> for Manufacturer {
+    type Error = ManufacturerError;
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
+        Manufacturer::from_str(s)
+    }
+}
+
+impl AsRef<str> for Manufacturer {
+    fn as_ref(&self) -> &str {
+        match self {
+            Self::Unknown => MANUFACTURER_UNKNOWN,
+            Self::Ledger => MANUFACTURER_LEDGER,
+        }
+    }
+}
+
+impl std::fmt::Display for Manufacturer {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let s: &str = self.as_ref();
+        write!(f, "{}", s)
+    }
+}
+
+#[derive(Clone, Debug, Error, PartialEq)]
+pub enum LocatorError {
+    #[error(transparent)]
+    ManufacturerError(#[from] ManufacturerError),
+    #[error(transparent)]
+    PubkeyError(#[from] ParsePubkeyError),
+    #[error(transparent)]
+    DerivationPathError(#[from] DerivationPathError),
+    #[error(transparent)]
+    UriReferenceError(#[from] URIReferenceError),
+    #[error("unimplemented scheme")]
+    UnimplementedScheme,
+    #[error("infallible")]
+    Infallible,
+}
+
+impl From<Infallible> for LocatorError {
+    fn from(_: Infallible) -> Self {
+        Self::Infallible
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub struct Locator {
+    pub manufacturer: Manufacturer,
+    pub pubkey: Option<Pubkey>,
+    pub derivation_path: Option<DerivationPath>,
+}
+
+impl std::fmt::Display for Locator {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let maybe_path = self.pubkey.map(|p| p.to_string());
+        let path = maybe_path.as_deref().unwrap_or("/");
+        let maybe_query = self.derivation_path.as_ref().map(|d| d.get_query());
+        let maybe_query2 = maybe_query.as_ref().map(|q| &q[1..]);
+
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(self.manufacturer.as_ref()))
+            .unwrap()
+            .try_path(path)
+            .unwrap()
+            .try_query(maybe_query2)
+            .unwrap();
+
+        let uri = builder.build().unwrap();
+        write!(f, "{}", uri)
+    }
+}
+
+impl Locator {
+    pub fn new_from_path<P: AsRef<str>>(path: P) -> Result<Self, LocatorError> {
+        let path = path.as_ref();
+        let uri = URIReference::try_from(path)?;
+        Self::new_from_uri(&uri)
+    }
+
+    pub fn new_from_uri(uri: &URIReference<'_>) -> Result<Self, LocatorError> {
+        let scheme = uri.scheme().map(|s| s.as_str().to_ascii_lowercase());
+        let host = uri.host().map(|h| h.to_string());
+        match (scheme, host) {
+            (Some(scheme), Some(host)) if scheme == "usb" => {
+                let path = uri.path().segments().get(0).and_then(|s| {
+                    if !s.is_empty() {
+                        Some(s.as_str())
+                    } else {
+                        None
+                    }
+                });
+                let key = if let Some(query) = uri.query() {
+                    let query_str = query.as_str();
+                    let query = qstring::QString::from(query_str);
+                    if query.len() > 1 {
+                        return Err(DerivationPathError::InvalidDerivationPath(
+                            "invalid query string, extra fields not supported".to_string(),
+                        )
+                        .into());
+                    }
+                    let key = query.get("key");
+                    if key.is_none() {
+                        return Err(DerivationPathError::InvalidDerivationPath(format!(
+                            "invalid query string `{}`, only `key` supported",
+                            query_str,
+                        ))
+                        .into());
+                    }
+                    key.map(|v| v.to_string())
+                } else {
+                    None
+                };
+                Self::new_from_parts(host.as_str(), path, key.as_deref())
+            }
+            (Some(_scheme), Some(_host)) => Err(LocatorError::UnimplementedScheme),
+            (None, Some(_host)) => Err(LocatorError::UnimplementedScheme),
+            (_, None) => Err(LocatorError::ManufacturerError(ManufacturerError)),
+        }
+    }
+
+    pub fn new_from_parts<V, VE, P, PE, D, DE>(
+        manufacturer: V,
+        pubkey: Option<P>,
+        derivation_path: Option<D>,
+    ) -> Result<Self, LocatorError>
+    where
+        VE: Into<LocatorError>,
+        V: TryInto<Manufacturer, Error = VE>,
+        PE: Into<LocatorError>,
+        P: TryInto<Pubkey, Error = PE>,
+        DE: Into<LocatorError>,
+        D: TryInto<DerivationPath, Error = DE>,
+    {
+        let manufacturer = manufacturer.try_into().map_err(|e| e.into())?;
+        let pubkey = if let Some(pubkey) = pubkey {
+            Some(pubkey.try_into().map_err(|e| e.into())?)
+        } else {
+            None
+        };
+        let derivation_path = if let Some(derivation_path) = derivation_path {
+            Some(derivation_path.try_into().map_err(|e| e.into())?)
+        } else {
+            None
+        };
+        Ok(Self {
+            manufacturer,
+            pubkey,
+            derivation_path,
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_manufacturer() {
+        assert_eq!(MANUFACTURER_LEDGER.try_into(), Ok(Manufacturer::Ledger));
+        assert!(
+            matches!(Manufacturer::from_str(MANUFACTURER_LEDGER), Ok(v) if v == Manufacturer::Ledger)
+        );
+        assert_eq!(Manufacturer::Ledger.as_ref(), MANUFACTURER_LEDGER);
+
+        assert!(
+            matches!(Manufacturer::from_str("bad-manufacturer"), Err(e) if e == ManufacturerError)
+        );
+    }
+
+    #[test]
+    fn test_locator_new_from_parts() {
+        let manufacturer = Manufacturer::Ledger;
+        let manufacturer_str = "ledger";
+        let pubkey = Pubkey::new_unique();
+        let pubkey_str = pubkey.to_string();
+        let derivation_path = DerivationPath::new_bip44(Some(0), Some(0));
+        let derivation_path_str = "0/0";
+
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: None,
+        };
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, None::<Pubkey>, None::<DerivationPath>),
+            Ok(e) if e == expect,
+        ));
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer_str, None::<Pubkey>, None::<DerivationPath>),
+            Ok(e) if e == expect,
+        ));
+
+        let expect = Locator {
+            manufacturer,
+            pubkey: Some(pubkey),
+            derivation_path: None,
+        };
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, Some(pubkey), None::<DerivationPath>),
+            Ok(e) if e == expect,
+        ));
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer_str, Some(pubkey_str.as_str()), None::<DerivationPath>),
+            Ok(e) if e == expect,
+        ));
+
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, None::<Pubkey>, Some(derivation_path)),
+            Ok(e) if e == expect,
+        ));
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, None::<Pubkey>, Some(derivation_path_str)),
+            Ok(e) if e == expect,
+        ));
+
+        assert!(matches!(
+            Locator::new_from_parts("bad-manufacturer", None::<Pubkey>, None::<DerivationPath>),
+            Err(LocatorError::ManufacturerError(e)) if e == ManufacturerError,
+        ));
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, Some("bad-pubkey"), None::<DerivationPath>),
+            Err(LocatorError::PubkeyError(e)) if e == ParsePubkeyError::Invalid,
+        ));
+        let bad_path = "bad-derivation-path".to_string();
+        assert!(matches!(
+            Locator::new_from_parts(manufacturer, None::<Pubkey>, Some(bad_path.as_str())),
+            Err(LocatorError::DerivationPathError(
+                DerivationPathError::InvalidDerivationPath(_)
+            )),
+        ));
+    }
+
+    #[test]
+    fn test_locator_new_from_uri() {
+        let derivation_path = DerivationPath::new_bip44(Some(0), Some(0));
+        let manufacturer = Manufacturer::Ledger;
+        let pubkey = Pubkey::new_unique();
+        let pubkey_str = pubkey.to_string();
+
+        // usb://ledger/{PUBKEY}?key=0'/0'
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path(pubkey_str.as_str())
+            .unwrap()
+            .try_query(Some("key=0/0"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: Some(pubkey),
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger/{PUBKEY}
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path(pubkey_str.as_str())
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: Some(pubkey),
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger/
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("/")
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger?key=0/0
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("key=0/0"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger?key=0'/0'
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("key=0'/0'"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // usb://ledger/?key=0/0
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("/")
+            .unwrap()
+            .try_query(Some("key=0/0"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path),
+        };
+        assert_eq!(Locator::new_from_uri(&uri), Ok(expect));
+
+        // bad-scheme://ledger
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("bad-scheme"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert_eq!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::UnimplementedScheme)
+        );
+
+        // usb://bad-manufacturer
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some("bad-manufacturer"))
+            .unwrap()
+            .try_path("")
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert_eq!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::ManufacturerError(ManufacturerError))
+        );
+
+        // usb://ledger/bad-pubkey
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("bad-pubkey")
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert_eq!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::PubkeyError(ParsePubkeyError::Invalid))
+        );
+
+        // usb://ledger?bad-key=0/0
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("bad-key=0/0"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert!(matches!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key=bad-value
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("key=bad-value"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert!(matches!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key=
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("key="))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert!(matches!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key
+        let mut builder = URIReferenceBuilder::new();
+        builder
+            .try_scheme(Some("usb"))
+            .unwrap()
+            .try_authority(Some(Manufacturer::Ledger.as_ref()))
+            .unwrap()
+            .try_path("")
+            .unwrap()
+            .try_query(Some("key"))
+            .unwrap();
+        let uri = builder.build().unwrap();
+        assert!(matches!(
+            Locator::new_from_uri(&uri),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+    }
+
+    #[test]
+    fn test_locator_new_from_path() {
+        let derivation_path = DerivationPath::new_bip44(Some(0), Some(0));
+        let manufacturer = Manufacturer::Ledger;
+        let pubkey = Pubkey::new_unique();
+        let path = format!("usb://ledger/{}?key=0/0", pubkey);
+        Locator::new_from_path(path).unwrap();
+
+        // usb://ledger/{PUBKEY}?key=0'/0'
+        let path = format!("usb://ledger/{}?key=0'/0'", pubkey);
+        let expect = Locator {
+            manufacturer,
+            pubkey: Some(pubkey),
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // usb://ledger/{PUBKEY}
+        let path = format!("usb://ledger/{}", pubkey);
+        let expect = Locator {
+            manufacturer,
+            pubkey: Some(pubkey),
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // usb://ledger
+        let path = "usb://ledger";
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // usb://ledger/
+        let path = "usb://ledger/";
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: None,
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // usb://ledger?key=0'/0'
+        let path = "usb://ledger?key=0'/0'";
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path.clone()),
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // usb://ledger/?key=0'/0'
+        let path = "usb://ledger?key=0'/0'";
+        let expect = Locator {
+            manufacturer,
+            pubkey: None,
+            derivation_path: Some(derivation_path),
+        };
+        assert_eq!(Locator::new_from_path(path), Ok(expect));
+
+        // bad-scheme://ledger
+        let path = "bad-scheme://ledger";
+        assert_eq!(
+            Locator::new_from_path(path),
+            Err(LocatorError::UnimplementedScheme)
+        );
+
+        // usb://bad-manufacturer
+        let path = "usb://bad-manufacturer";
+        assert_eq!(
+            Locator::new_from_path(path),
+            Err(LocatorError::ManufacturerError(ManufacturerError))
+        );
+
+        // usb://ledger/bad-pubkey
+        let path = "usb://ledger/bad-pubkey";
+        assert_eq!(
+            Locator::new_from_path(path),
+            Err(LocatorError::PubkeyError(ParsePubkeyError::Invalid))
+        );
+
+        // usb://ledger?bad-key=0/0
+        let path = "usb://ledger?bad-key=0/0";
+        assert!(matches!(
+            Locator::new_from_path(path),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?bad-key=0'/0'
+        let path = "usb://ledger?bad-key=0'/0'";
+        assert!(matches!(
+            Locator::new_from_path(path),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key=bad-value
+        let path = format!("usb://ledger/{}?key=bad-value", pubkey);
+        assert!(matches!(
+            Locator::new_from_path(path),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key=
+        let path = format!("usb://ledger/{}?key=", pubkey);
+        assert!(matches!(
+            Locator::new_from_path(path),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+
+        // usb://ledger?key
+        let path = format!("usb://ledger/{}?key", pubkey);
+        assert!(matches!(
+            Locator::new_from_path(path),
+            Err(LocatorError::DerivationPathError(_))
+        ));
+    }
+}

remote-wallet/src/remote_keypair.rs

@@ -1,6 +1,7 @@
 use {
     crate::{
         ledger::get_ledger_from_info,
+        locator::Manufacturer,
         remote_wallet::{
             RemoteWallet, RemoteWalletError, RemoteWalletInfo, RemoteWalletManager,
             RemoteWalletType,
@@ -61,7 +62,7 @@ pub fn generate_remote_keypair(
     keypair_name: &str,
 ) -> Result<RemoteKeypair, RemoteWalletError> {
     let (remote_wallet_info, derivation_path) = RemoteWalletInfo::parse_path(path)?;
-    if remote_wallet_info.manufacturer == "ledger" {
+    if remote_wallet_info.manufacturer == Manufacturer::Ledger {
         let ledger = get_ledger_from_info(remote_wallet_info, keypair_name, wallet_manager)?;
         let path = format!("{}{}", ledger.pretty_path, derivation_path.get_query());
         Ok(RemoteKeypair::new(

remote-wallet/src/remote_wallet.rs

@@ -2,6 +2,7 @@ use {
     crate::{
         ledger::{is_valid_ledger, LedgerWallet},
         ledger_error::LedgerError,
+        locator::{Locator, LocatorError, Manufacturer},
     },
     log::*,
     parking_lot::{Mutex, RwLock},
@@ -11,12 +12,10 @@ use {
         signature::{Signature, SignerError},
     },
     std::{
-        str::FromStr,
         sync::Arc,
         time::{Duration, Instant},
     },
     thiserror::Error,
-    url::Url,
 };
 
 const HID_GLOBAL_USAGE_PAGE: u16 = 0xFF00;
@@ -57,6 +56,9 @@ pub enum RemoteWalletError {
 
     #[error("remote wallet operation rejected by the user")]
     UserCancel,
+
+    #[error(transparent)]
+    LocatorError(#[from] LocatorError),
 }
 
 impl From<hidapi::HidError> for RemoteWalletError {
@@ -239,7 +241,7 @@ pub struct RemoteWalletInfo {
     /// RemoteWallet device model
     pub model: String,
     /// RemoteWallet device manufacturer
-    pub manufacturer: String,
+    pub manufacturer: Manufacturer,
     /// RemoteWallet device serial number
     pub serial: String,
     /// RemoteWallet host device path
@@ -252,63 +254,19 @@ pub struct RemoteWalletInfo {
 
 impl RemoteWalletInfo {
     pub fn parse_path(path: String) -> Result<(Self, DerivationPath), RemoteWalletError> {
-        let wallet_path = Url::parse(&path).map_err(|e| {
-            Into::<RemoteWalletError>::into(DerivationPathError::InvalidDerivationPath(format!(
-                "parse error: {:?}",
-                e
-            )))
-        })?;
-
-        if wallet_path.host_str().is_none() {
-            return Err(DerivationPathError::InvalidDerivationPath(
-                "missing remote wallet type".to_string(),
-            )
-            .into());
-        }
-
-        let mut wallet_info = RemoteWalletInfo {
-            manufacturer: wallet_path.host_str().unwrap().to_string(),
-            ..RemoteWalletInfo::default()
-        };
-
-        if let Some(wallet_id) = wallet_path.path_segments().map(|c| c.collect::<Vec<_>>()) {
-            if !wallet_id[0].is_empty() {
-                wallet_info.pubkey = Pubkey::from_str(wallet_id[0]).map_err(|e| {
-                    Into::<RemoteWalletError>::into(DerivationPathError::InvalidDerivationPath(
-                        format!("pubkey from_str error: {:?}", e),
-                    ))
-                })?;
-            }
-        }
-
-        let mut derivation_path = DerivationPath::default();
-        let mut query_pairs = wallet_path.query_pairs();
-        if query_pairs.count() > 0 {
-            for _ in 0..query_pairs.count() {
-                if let Some(mut pair) = query_pairs.next() {
-                    if pair.0 == "key" {
-                        let key_path = pair.1.to_mut();
-                        if key_path.ends_with('/') {
-                            key_path.pop();
-                        }
-                        derivation_path = DerivationPath::from_key_str(key_path)?;
-                    } else {
-                        return Err(DerivationPathError::InvalidDerivationPath(format!(
-                            "invalid query string `{}={}`, only `key` supported",
-                            pair.0, pair.1
-                        ))
-                        .into());
-                    }
-                }
-                if query_pairs.next().is_some() {
-                    return Err(DerivationPathError::InvalidDerivationPath(
-                        "invalid query string, extra fields not supported".to_string(),
-                    )
-                    .into());
-                }
-            }
-        }
-        Ok((wallet_info, derivation_path))
+        let Locator {
+            manufacturer,
+            pubkey,
+            derivation_path,
+        } = Locator::new_from_path(path)?;
+        Ok((
+            RemoteWalletInfo {
+                manufacturer,
+                pubkey: pubkey.unwrap_or_default(),
+                ..RemoteWalletInfo::default()
+            },
+            derivation_path.unwrap_or_default(),
+        ))
     }
 
     pub fn get_pretty_path(&self) -> String {
@@ -356,7 +314,7 @@ mod tests {
             RemoteWalletInfo::parse_path(format!("usb://ledger/{:?}?key=1/2", pubkey)).unwrap();
         assert!(wallet_info.matches(&RemoteWalletInfo {
             model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey,
@@ -367,7 +325,7 @@ mod tests {
             RemoteWalletInfo::parse_path(format!("usb://ledger/{:?}?key=1'/2'", pubkey)).unwrap();
         assert!(wallet_info.matches(&RemoteWalletInfo {
             model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey,
@@ -378,42 +336,20 @@ mod tests {
             RemoteWalletInfo::parse_path(format!("usb://ledger/{:?}?key=1\'/2\'", pubkey)).unwrap();
         assert!(wallet_info.matches(&RemoteWalletInfo {
             model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey,
             error: None,
         }));
         assert_eq!(derivation_path, DerivationPath::new_bip44(Some(1), Some(2)));
-        let (wallet_info, derivation_path) =
-            RemoteWalletInfo::parse_path(format!("usb://ledger/{:?}?key=1/2/", pubkey)).unwrap();
-        assert!(wallet_info.matches(&RemoteWalletInfo {
-            model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
-            serial: "".to_string(),
-            host_device_path: "/host/device/path".to_string(),
-            pubkey,
-            error: None,
-        }));
-        assert_eq!(derivation_path, DerivationPath::new_bip44(Some(1), Some(2)));
-        let (wallet_info, derivation_path) =
-            RemoteWalletInfo::parse_path(format!("usb://ledger/{:?}?key=1/", pubkey)).unwrap();
-        assert!(wallet_info.matches(&RemoteWalletInfo {
-            model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
-            serial: "".to_string(),
-            host_device_path: "/host/device/path".to_string(),
-            pubkey,
-            error: None,
-        }));
-        assert_eq!(derivation_path, DerivationPath::new_bip44(Some(1), None));
 
         // Test that wallet id need not be complete for key derivation to work
         let (wallet_info, derivation_path) =
             RemoteWalletInfo::parse_path("usb://ledger?key=1".to_string()).unwrap();
         assert!(wallet_info.matches(&RemoteWalletInfo {
             model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey: Pubkey::default(),
@@ -424,7 +360,7 @@ mod tests {
             RemoteWalletInfo::parse_path("usb://ledger/?key=1/2".to_string()).unwrap();
         assert!(wallet_info.matches(&RemoteWalletInfo {
             model: "".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey: Pubkey::default(),
@@ -452,7 +388,7 @@ mod tests {
     fn test_remote_wallet_info_matches() {
         let pubkey = solana_sdk::pubkey::new_rand();
         let info = RemoteWalletInfo {
-            manufacturer: "Ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             model: "Nano S".to_string(),
             serial: "0001".to_string(),
             host_device_path: "/host/device/path".to_string(),
@@ -460,11 +396,11 @@ mod tests {
             error: None,
         };
         let mut test_info = RemoteWalletInfo {
-            manufacturer: "Not Ledger".to_string(),
+            manufacturer: Manufacturer::Unknown,
             ..RemoteWalletInfo::default()
         };
         assert!(!info.matches(&test_info));
-        test_info.manufacturer = "Ledger".to_string();
+        test_info.manufacturer = Manufacturer::Ledger;
         assert!(info.matches(&test_info));
         test_info.model = "Other".to_string();
         assert!(info.matches(&test_info));
@@ -485,7 +421,7 @@ mod tests {
         let pubkey_str = pubkey.to_string();
         let remote_wallet_info = RemoteWalletInfo {
             model: "nano-s".to_string(),
-            manufacturer: "ledger".to_string(),
+            manufacturer: Manufacturer::Ledger,
             serial: "".to_string(),
             host_device_path: "/host/device/path".to_string(),
             pubkey,

sdk/program/src/pubkey.rs

@@ -1,7 +1,11 @@
 use crate::{decode_error::DecodeError, hash::hashv};
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
 use num_derive::{FromPrimitive, ToPrimitive};
-use std::{convert::TryFrom, fmt, mem, str::FromStr};
+use std::{
+    convert::{Infallible, TryFrom},
+    fmt, mem,
+    str::FromStr,
+};
 use thiserror::Error;
 
 /// Number of bytes in a pubkey
@@ -63,7 +67,16 @@ pub enum ParsePubkeyError {
     WrongSize,
     #[error("Invalid Base58 string")]
     Invalid,
+    #[error("Infallible")]
+    Infallible,
 }
+
+impl From<Infallible> for ParsePubkeyError {
+    fn from(_: Infallible) -> Self {
+        Self::Infallible
+    }
+}
+
 impl<T> DecodeError<T> for ParsePubkeyError {
     fn type_of() -> &'static str {
         "ParsePubkeyError"
@@ -88,6 +101,13 @@ impl FromStr for Pubkey {
     }
 }
 
+impl TryFrom<&str> for Pubkey {
+    type Error = ParsePubkeyError;
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
+        Pubkey::from_str(s)
+    }
+}
+
 impl Pubkey {
     pub fn new(pubkey_vec: &[u8]) -> Self {
         Self(

sdk/src/derivation_path.rs

@@ -1,7 +1,11 @@
 use {
     core::{iter::IntoIterator, slice::Iter},
     derivation_path::{ChildIndex, DerivationPath as DerivationPathInner},
-    std::{fmt, str::FromStr},
+    std::{
+        convert::{Infallible, TryFrom},
+        fmt,
+        str::FromStr,
+    },
     thiserror::Error,
 };
 
@@ -9,13 +13,21 @@ const ACCOUNT_INDEX: usize = 2;
 const CHANGE_INDEX: usize = 3;
 
 /// Derivation path error.
-#[derive(Error, Debug, Clone)]
+#[derive(Error, Debug, Clone, PartialEq)]
 pub enum DerivationPathError {
     #[error("invalid derivation path: {0}")]
     InvalidDerivationPath(String),
+    #[error("infallible")]
+    Infallible,
 }
 
-#[derive(PartialEq)]
+impl From<Infallible> for DerivationPathError {
+    fn from(_: Infallible) -> Self {
+        Self::Infallible
+    }
+}
+
+#[derive(Clone, PartialEq)]
 pub struct DerivationPath(DerivationPathInner);
 
 impl Default for DerivationPath {
@@ -24,6 +36,13 @@ impl Default for DerivationPath {
     }
 }
 
+impl TryFrom<&str> for DerivationPath {
+    type Error = DerivationPathError;
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
+        Self::from_key_str(s)
+    }
+}
+
 impl DerivationPath {
     fn new<P: Into<Box<[ChildIndex]>>>(path: P) -> Self {
         Self(DerivationPathInner::new(path))