Remove non-small-order view key bound

Guaranteed addresses are in fact guaranteed even with this due to prefixing key
images causing zeroing the ECDH to not zero the shared key.

coins/monero/rpc/src/lib.rs

@@ -751,7 +751,7 @@ pub trait Rpc: Sync + Clone + Debug {
         };
         Ok(Some([key, rpc_point(&out.mask)?]).filter(|_| {
           if fingerprintable_canonical {
-            Timelock::Block(height) >= txs[i].prefix().timelock
+            Timelock::Block(height) >= txs[i].prefix().additional_timelock
           } else {
             out.unlocked
           }

coins/monero/src/transaction.rs

@@ -141,11 +141,11 @@ impl Output {
 /// longer of the two will be the timelock used.
 #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
 pub enum Timelock {
-  /// No timelock.
+  /// No additional timelock.
   None,
-  /// Locked until this block.
+  /// Additionally locked until this block.
   Block(usize),
-  /// Locked until this many seconds since the epoch.
+  /// Additionally locked until this many seconds since the epoch.
   Time(u64),
 }
 
@@ -199,9 +199,11 @@ impl PartialOrd for Timelock {
 /// handle it. It excludes any proofs.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct TransactionPrefix {
-  /// The timelock this transaction uses.
+  /// The timelock this transaction is additionally constrained by.
-  // TODO: Rename to additional timelock?
+  ///
-  pub timelock: Timelock,
+  /// All transactions on the blockchain are subject to a 10-block lock. This adds a further
+  /// constraint.
+  pub additional_timelock: Timelock,
   /// The inputs for this transaction.
   pub inputs: Vec<Input>,
   /// The outputs for this transaction.
@@ -218,7 +220,7 @@ impl TransactionPrefix {
   ///
   /// This is distinct from Monero in that it won't write any version.
   fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
-    self.timelock.write(w)?;
+    self.additional_timelock.write(w)?;
     write_vec(Input::write, &self.inputs, w)?;
     write_vec(Output::write, &self.outputs, w)?;
     write_varint(&self.extra.len(), w)?;
@@ -230,7 +232,7 @@ impl TransactionPrefix {
   /// This is distinct from Monero in that it won't read the version. The version must be passed
   /// in.
   pub fn read<R: Read>(r: &mut R, version: u64) -> io::Result<TransactionPrefix> {
-    let timelock = Timelock::read(r)?;
+    let additional_timelock = Timelock::read(r)?;
 
     let inputs = read_vec(|r| Input::read(r), r)?;
     if inputs.is_empty() {
@@ -239,7 +241,7 @@ impl TransactionPrefix {
     let is_miner_tx = matches!(inputs[0], Input::Gen { .. });
 
     let mut prefix = TransactionPrefix {
-      timelock,
+      additional_timelock,
       inputs,
       outputs: read_vec(|r| Output::read((!is_miner_tx) && (version == 2), r), r)?,
       extra: vec![],

coins/monero/wallet/address/src/lib.rs

@@ -8,7 +8,7 @@ use std_shims::string::ToString;
 
 use zeroize::Zeroize;
 
-use curve25519_dalek::{traits::IsIdentity, EdwardsPoint};
+use curve25519_dalek::EdwardsPoint;
 
 use monero_io::*;
 
@@ -341,15 +341,6 @@ pub const MONERO_BYTES: NetworkedAddressBytes = match NetworkedAddressBytes::new
   None => panic!("Monero network byte constants conflicted"),
 };
 
-/// Errors when creating an address.
-#[derive(Clone, Copy, PartialEq, Eq, Debug)]
-#[cfg_attr(feature = "std", derive(thiserror::Error))]
-pub enum AddressCreationError {
-  /// The view key was of small order despite being in a guaranteed address.
-  #[cfg_attr(feature = "std", error("small-order view key in guaranteed address"))]
-  SmallOrderView,
-}
-
 /// A Monero address.
 #[derive(Clone, Copy, PartialEq, Eq, Zeroize)]
 pub struct Address<const ADDRESS_BYTES: u128> {
@@ -404,16 +395,8 @@ impl<const ADDRESS_BYTES: u128> fmt::Display for Address<ADDRESS_BYTES> {
 
 impl<const ADDRESS_BYTES: u128> Address<ADDRESS_BYTES> {
   /// Create a new address.
-  pub fn new(
+  pub fn new(network: Network, kind: AddressType, spend: EdwardsPoint, view: EdwardsPoint) -> Self {
-    network: Network,
+    Address { network, kind, spend, view }
-    kind: AddressType,
-    spend: EdwardsPoint,
-    view: EdwardsPoint,
-  ) -> Result<Self, AddressCreationError> {
-    if kind.is_guaranteed() && view.mul_by_cofactor().is_identity() {
-      Err(AddressCreationError::SmallOrderView)?;
-    }
-    Ok(Address { network, kind, spend, view })
   }
 
   /// Parse an address from a String, accepting any network it is.
@@ -455,11 +438,6 @@ impl<const ADDRESS_BYTES: u128> Address<ADDRESS_BYTES> {
       Err(AddressError::InvalidLength)?;
     }
 
-    // If this is a guaranteed address, reject small-order view keys
-    if kind.is_guaranteed() && view.mul_by_cofactor().is_identity() {
-      Err(AddressError::SmallOrderView)?;
-    }
-
     Ok(Address { network, kind, spend, view })
   }
 

coins/monero/wallet/address/src/tests.rs

@@ -125,7 +125,7 @@ fn featured() {
         let guaranteed = (features & GUARANTEED_FEATURE_BIT) == GUARANTEED_FEATURE_BIT;
 
         let kind = AddressType::Featured { subaddress, payment_id, guaranteed };
-        let addr = MoneroAddress::new(network, kind, spend, view).unwrap();
+        let addr = MoneroAddress::new(network, kind, spend, view);
 
         assert_eq!(addr.to_string().chars().next().unwrap(), first);
         assert_eq!(MoneroAddress::from_str(network, &addr.to_string()).unwrap(), addr);
@@ -198,7 +198,6 @@ fn featured_vectors() {
         spend,
         view
       )
-      .unwrap()
       .to_string(),
       vector.address
     );

coins/monero/wallet/src/scan.rs

@@ -231,7 +231,7 @@ impl InternalScanner {
             key: output_key,
             key_offset,
             commitment,
-            additional_timelock: tx.prefix().timelock,
+            additional_timelock: tx.prefix().additional_timelock,
           },
           metadata: Metadata { subaddress, payment_id, arbitrary_data: extra.data() },
         });

coins/monero/wallet/src/send/eventuality.rs

@@ -62,7 +62,7 @@ impl Eventuality {
     }
 
     // Also ensure no timelock was set
-    if tx.prefix().timelock != Timelock::None {
+    if tx.prefix().additional_timelock != Timelock::None {
       return false;
     }
 

coins/monero/wallet/src/send/tx.rs

@@ -151,7 +151,7 @@ impl SignableTransaction {
       // `- 1` to remove the one byte for the 0 fee
       Transaction::V2 {
         prefix: TransactionPrefix {
-          timelock: Timelock::None,
+          additional_timelock: Timelock::None,
           inputs: self.inputs(&key_images),
           outputs: self.outputs(&key_images),
           extra: self.extra(),
@@ -239,7 +239,7 @@ impl SignableTransactionWithKeyImages {
 
     Transaction::V2 {
       prefix: TransactionPrefix {
-        timelock: Timelock::None,
+        additional_timelock: Timelock::None,
         inputs: self.intent.inputs(&self.key_images),
         outputs: self.intent.outputs(&self.key_images),
         extra: self.intent.extra(),

coins/monero/wallet/src/view_pair.rs

@@ -6,7 +6,7 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, Scalar, EdwardsPoint}
 
 use crate::{
   primitives::keccak256_to_scalar,
-  address::{Network, AddressType, SubaddressIndex, AddressCreationError, MoneroAddress},
+  address::{Network, AddressType, SubaddressIndex, MoneroAddress},
 };
 
 /// The pair of keys necessary to scan transactions.
@@ -57,40 +57,20 @@ impl ViewPair {
   ///
   /// Subaddresses SHOULD be used instead.
   pub fn legacy_address(&self, network: Network) -> MoneroAddress {
-    match MoneroAddress::new(network, AddressType::Legacy, self.spend, self.view()) {
+    MoneroAddress::new(network, AddressType::Legacy, self.spend, self.view())
-      Ok(addr) => addr,
-      Err(AddressCreationError::SmallOrderView) => {
-        panic!("small-order view key error despite not making a guaranteed address")
-      }
-    }
   }
 
   /// Derive a legacy integrated address from this ViewPair.
   ///
   /// Subaddresses SHOULD be used instead.
   pub fn legacy_integrated_address(&self, network: Network, payment_id: [u8; 8]) -> MoneroAddress {
-    match MoneroAddress::new(
+    MoneroAddress::new(network, AddressType::LegacyIntegrated(payment_id), self.spend, self.view())
-      network,
-      AddressType::LegacyIntegrated(payment_id),
-      self.spend,
-      self.view(),
-    ) {
-      Ok(addr) => addr,
-      Err(AddressCreationError::SmallOrderView) => {
-        panic!("small-order view key error despite not making a guaranteed address")
-      }
-    }
   }
 
   /// Derive a subaddress from this ViewPair.
   pub fn subaddress(&self, network: Network, subaddress: SubaddressIndex) -> MoneroAddress {
     let (spend, view) = self.subaddress_keys(subaddress);
-    match MoneroAddress::new(network, AddressType::Subaddress, spend, view) {
+    MoneroAddress::new(network, AddressType::Subaddress, spend, view)
-      Ok(addr) => addr,
-      Err(AddressCreationError::SmallOrderView) => {
-        panic!("small-order view key error despite not making a guaranteed address")
-      }
-    }
   }
 }
 
@@ -106,14 +86,8 @@ pub struct GuaranteedViewPair(pub(crate) ViewPair);
 
 impl GuaranteedViewPair {
   /// Create a new GuaranteedViewPair.
-  ///
+  pub fn new(spend: EdwardsPoint, view: Zeroizing<Scalar>) -> Self {
-  /// This will return None if the view key is of small order (if it's zero).
+    GuaranteedViewPair(ViewPair::new(spend, view))
-  // Internal doc comment: These scalars are of prime order so 0 is the only small order Scalar
-  pub fn new(spend: EdwardsPoint, view: Zeroizing<Scalar>) -> Option<Self> {
-    if view.deref() == &Scalar::ZERO {
-      None?;
-    }
-    Some(GuaranteedViewPair(ViewPair::new(spend, view)))
   }
 
   /// The public spend key for this GuaranteedViewPair.
@@ -142,16 +116,11 @@ impl GuaranteedViewPair {
       (self.spend(), self.view())
     };
 
-    match MoneroAddress::new(
+    MoneroAddress::new(
       network,
       AddressType::Featured { subaddress: subaddress.is_some(), payment_id, guaranteed: true },
       spend,
       view,
-    ) {
+    )
-      Ok(addr) => addr,
-      Err(AddressCreationError::SmallOrderView) => {
-        panic!("created a ViewPair with identity as the view key")
-      }
-    }
   }
 }

coins/monero/wallet/tests/eventuality.rs

@@ -21,8 +21,7 @@ test!(
           AddressType::Legacy,
           ED25519_BASEPOINT_POINT,
           ED25519_BASEPOINT_POINT,
-        )
+        ),
-        .unwrap(),
         1,
       );
       builder.add_payment(
@@ -31,8 +30,7 @@ test!(
           AddressType::LegacyIntegrated([0xaa; 8]),
           ED25519_BASEPOINT_POINT,
           ED25519_BASEPOINT_POINT,
-        )
+        ),
-        .unwrap(),
         2,
       );
       builder.add_payment(
@@ -41,8 +39,7 @@ test!(
           AddressType::Subaddress,
           ED25519_BASEPOINT_POINT,
           ED25519_BASEPOINT_POINT,
-        )
+        ),
-        .unwrap(),
         3,
       );
       builder.add_payment(
@@ -51,8 +48,7 @@ test!(
           AddressType::Featured { subaddress: false, payment_id: None, guaranteed: true },
           ED25519_BASEPOINT_POINT,
           ED25519_BASEPOINT_POINT,
-        )
+        ),
-        .unwrap(),
         4,
       );
       let tx = builder.build().unwrap();

coins/monero/wallet/tests/runner/mod.rs

@@ -41,8 +41,7 @@ pub fn random_address() -> (Scalar, ViewPair, MoneroAddress) {
       AddressType::Legacy,
       spend_pub,
       view.deref() * ED25519_BASEPOINT_TABLE,
-    )
+    ),
-    .unwrap(),
   )
 }
 
@@ -53,14 +52,13 @@ pub fn random_guaranteed_address() -> (Scalar, GuaranteedViewPair, MoneroAddress
   let view = Zeroizing::new(Scalar::random(&mut OsRng));
   (
     spend,
-    GuaranteedViewPair::new(spend_pub, view.clone()).unwrap(),
+    GuaranteedViewPair::new(spend_pub, view.clone()),
     MoneroAddress::new(
       Network::Mainnet,
       AddressType::Legacy,
       spend_pub,
       view.deref() * ED25519_BASEPOINT_TABLE,
-    )
+    ),
-    .unwrap(),
   )
 }
 
@@ -141,7 +139,6 @@ pub async fn rpc() -> SimpleRequestRpc {
     &Scalar::random(&mut OsRng) * ED25519_BASEPOINT_TABLE,
     &Scalar::random(&mut OsRng) * ED25519_BASEPOINT_TABLE,
   )
-  .unwrap()
   .to_string();
 
   // Mine 40 blocks to ensure decoy availability

processor/src/networks/monero.rs

@@ -256,7 +256,7 @@ impl Monero {
   }
 
   fn view_pair(spend: EdwardsPoint) -> GuaranteedViewPair {
-    GuaranteedViewPair::new(spend.0, Zeroizing::new(additional_key::<Monero>(0).0)).unwrap()
+    GuaranteedViewPair::new(spend.0, Zeroizing::new(additional_key::<Monero>(0).0))
   }
 
   fn address_internal(spend: EdwardsPoint, subaddress: Option<SubaddressIndex>) -> Address {

substrate/client/src/networks/monero.rs

@@ -51,8 +51,7 @@ impl TryFrom<Vec<u8>> for Address {
     // Decode as SCALE
     let addr = EncodedAddress::decode(&mut data.as_ref()).map_err(|_| ())?;
     // Convert over
-    Ok(Address(
+    Ok(Address(MoneroAddress::new(
-      MoneroAddress::new(
       Network::Mainnet,
       match addr.kind {
         EncodedAddressType::Legacy => AddressType::Legacy,
@@ -69,9 +68,7 @@ impl TryFrom<Vec<u8>> for Address {
       },
       Ed25519::read_G::<&[u8]>(&mut addr.spend.as_ref()).map_err(|_| ())?.0,
       Ed25519::read_G::<&[u8]>(&mut addr.view.as_ref()).map_err(|_| ())?.0,
-      )
+    )))
-      .map_err(|_| ())?,
-    ))
   }
 }
 

tests/full-stack/src/tests/mint_and_burn.rs

@@ -387,8 +387,7 @@ async fn mint_and_burn_test() {
             decompress_point(monero_key_pair.1.to_vec().try_into().unwrap()).unwrap(),
             ED25519_BASEPOINT_POINT *
               processor::additional_key::<processor::networks::monero::Monero>(0).0,
-          )
+          ),
-          .unwrap(),
           1_100_000_000_000,
         )],
         Change::new(&view_pair),
@@ -475,8 +474,7 @@ async fn mint_and_burn_test() {
         AddressType::Legacy,
         spend,
         ED25519_BASEPOINT_TABLE * &view,
-      )
+      );
-      .unwrap();
 
       (spend, view, addr)
     };

tests/processor/src/networks.rs

@@ -463,8 +463,7 @@ impl Wallet {
           AddressType::Featured { subaddress: false, payment_id: None, guaranteed: true },
           to_spend_key,
           ED25519_BASEPOINT_POINT * to_view_key.0,
-        )
+        );
-        .unwrap();
 
         // Create and sign the TX
         const AMOUNT: u64 = 1_000_000_000_000;