monero: only mask user features on new polyseed, not on decode (#503)

* monero: only mask user features on new polyseed, not on decode

- This commit ensures a polyseed string that has unsupported features correctly errors on decode (rather than panic in debug build or return an incorrect successful response in prod build)
- Also avoids panicking when checksum calculation is unexpectedly wrong

Polyseed reference impl for feature masking:
- polyseed_create: b7c35bb3c6/src/polyseed.c (L61)
- polyseed_decode: b7c35bb3c6/src/polyseed.c (L212)

* PR comments

* Make from_internal a member of Polyseed

* Add accidentally removed newline

---------

Co-authored-by: Luke Parker <lukeparker5132@gmail.com>

coins/monero/src/tests/seed.rs

@@ -7,7 +7,7 @@ use curve25519_dalek::scalar::Scalar;
 use crate::{
   hash,
   wallet::seed::{
-    Seed, SeedType,
+    Seed, SeedType, SeedError,
     classic::{self, trim_by_lang},
     polyseed,
   },
@@ -469,3 +469,13 @@ fn test_polyseed() {
     }
   }
 }
+
+#[test]
+fn test_invalid_polyseed() {
+  // This seed includes unsupported features bits and should error on decode
+  let seed = "include domain claim resemble urban hire lunch bird \
+    crucial fire best wife ring warm ignore model"
+    .into();
+  let res = Seed::from_string(Zeroizing::new(seed));
+  assert_eq!(res, Err(SeedError::UnsupportedFeatures));
+}

coins/monero/src/wallet/seed/polyseed.rs

@@ -217,6 +217,31 @@ impl Polyseed {
     poly
   }
 
+  fn from_internal(
+    language: Language,
+    masked_features: u8,
+    encoded_birthday: u16,
+    entropy: Zeroizing<[u8; 32]>,
+  ) -> Result<Polyseed, SeedError> {
+    if !polyseed_features_supported(masked_features) {
+      Err(SeedError::UnsupportedFeatures)?;
+    }
+
+    if !valid_entropy(&entropy) {
+      Err(SeedError::InvalidEntropy)?;
+    }
+
+    let mut res = Polyseed {
+      language,
+      birthday: encoded_birthday,
+      features: masked_features,
+      entropy,
+      checksum: 0,
+    };
+    res.checksum = poly_eval(&res.to_poly());
+    Ok(res)
+  }
+
   /// Create a new `Polyseed` with specific internals.
   ///
   /// `birthday` is defined in seconds since the Unix epoch.
@@ -226,20 +251,7 @@ impl Polyseed {
     birthday: u64,
     entropy: Zeroizing<[u8; 32]>,
   ) -> Result<Polyseed, SeedError> {
-    let features = user_features(features);
-    if !polyseed_features_supported(features) {
-      Err(SeedError::UnsupportedFeatures)?;
-    }
-
-    let birthday = birthday_encode(birthday);
-
-    if !valid_entropy(&entropy) {
-      Err(SeedError::InvalidEntropy)?;
-    }
-
-    let mut res = Polyseed { language, birthday, features, entropy, checksum: 0 };
-    res.checksum = poly_eval(&res.to_poly());
-    Ok(res)
+    Self::from_internal(language, user_features(features), birthday_encode(birthday), entropy)
   }
 
   /// Create a new `Polyseed`.
@@ -370,7 +382,7 @@ impl Polyseed {
     let features =
       u8::try_from(extra >> DATE_BITS).expect("couldn't convert extra >> DATE_BITS to u8");
 
-    let res = Polyseed::from(lang, features, birthday_decode(birthday), entropy);
+    let res = Self::from_internal(lang, features, birthday, entropy);
     if let Ok(res) = res.as_ref() {
       debug_assert_eq!(res.checksum, checksum);
     }