diff --git a/coins/monero/rpc/src/lib.rs b/coins/monero/rpc/src/lib.rs index 8b03b7061..14e5ed7e3 100644 --- a/coins/monero/rpc/src/lib.rs +++ b/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 } diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index e5e117926..a1e921f45 100644 --- a/coins/monero/src/transaction.rs +++ b/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. - // TODO: Rename to additional timelock? - pub timelock: Timelock, + /// The timelock this transaction is additionally constrained by. + /// + /// 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, /// 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(&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: &mut R, version: u64) -> io::Result { - 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![], diff --git a/coins/monero/wallet/address/src/lib.rs b/coins/monero/wallet/address/src/lib.rs index 67269c15c..3b50940c6 100644 --- a/coins/monero/wallet/address/src/lib.rs +++ b/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 { @@ -404,16 +395,8 @@ impl fmt::Display for Address { impl Address { /// Create a new address. - pub fn new( - network: Network, - kind: AddressType, - spend: EdwardsPoint, - view: EdwardsPoint, - ) -> Result { - if kind.is_guaranteed() && view.mul_by_cofactor().is_identity() { - Err(AddressCreationError::SmallOrderView)?; - } - Ok(Address { network, kind, spend, view }) + pub fn new(network: Network, kind: AddressType, spend: EdwardsPoint, view: EdwardsPoint) -> Self { + Address { network, kind, spend, view } } /// Parse an address from a String, accepting any network it is. @@ -455,11 +438,6 @@ impl Address { 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 }) } diff --git a/coins/monero/wallet/address/src/tests.rs b/coins/monero/wallet/address/src/tests.rs index d9a1e27a6..2804832ab 100644 --- a/coins/monero/wallet/address/src/tests.rs +++ b/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 ); diff --git a/coins/monero/wallet/src/scan.rs b/coins/monero/wallet/src/scan.rs index 6b9b073ff..2453f94d2 100644 --- a/coins/monero/wallet/src/scan.rs +++ b/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() }, }); diff --git a/coins/monero/wallet/src/send/eventuality.rs b/coins/monero/wallet/src/send/eventuality.rs index 5ea8129f5..031de1e1e 100644 --- a/coins/monero/wallet/src/send/eventuality.rs +++ b/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; } diff --git a/coins/monero/wallet/src/send/tx.rs b/coins/monero/wallet/src/send/tx.rs index b86cd3b7f..7d50c1982 100644 --- a/coins/monero/wallet/src/send/tx.rs +++ b/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(), diff --git a/coins/monero/wallet/src/view_pair.rs b/coins/monero/wallet/src/view_pair.rs index 0ade97a63..fc9a3c541 100644 --- a/coins/monero/wallet/src/view_pair.rs +++ b/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()) { - Ok(addr) => addr, - Err(AddressCreationError::SmallOrderView) => { - panic!("small-order view key error despite not making a guaranteed address") - } - } + MoneroAddress::new(network, AddressType::Legacy, self.spend, self.view()) } /// 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( - 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") - } - } + MoneroAddress::new(network, AddressType::LegacyIntegrated(payment_id), self.spend, self.view()) } /// 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) { - Ok(addr) => addr, - Err(AddressCreationError::SmallOrderView) => { - panic!("small-order view key error despite not making a guaranteed address") - } - } + MoneroAddress::new(network, AddressType::Subaddress, spend, view) } } @@ -106,14 +86,8 @@ pub struct GuaranteedViewPair(pub(crate) ViewPair); impl GuaranteedViewPair { /// Create a new GuaranteedViewPair. - /// - /// This will return None if the view key is of small order (if it's zero). - // Internal doc comment: These scalars are of prime order so 0 is the only small order Scalar - pub fn new(spend: EdwardsPoint, view: Zeroizing) -> Option { - if view.deref() == &Scalar::ZERO { - None?; - } - Some(GuaranteedViewPair(ViewPair::new(spend, view))) + pub fn new(spend: EdwardsPoint, view: Zeroizing) -> Self { + 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") - } - } + ) } } diff --git a/coins/monero/wallet/tests/eventuality.rs b/coins/monero/wallet/tests/eventuality.rs index 466e0e789..ac6def57b 100644 --- a/coins/monero/wallet/tests/eventuality.rs +++ b/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(); diff --git a/coins/monero/wallet/tests/runner/mod.rs b/coins/monero/wallet/tests/runner/mod.rs index 91ec67246..abb904024 100644 --- a/coins/monero/wallet/tests/runner/mod.rs +++ b/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 diff --git a/processor/src/networks/monero.rs b/processor/src/networks/monero.rs index f03ab75c1..a316a8002 100644 --- a/processor/src/networks/monero.rs +++ b/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::(0).0)).unwrap() + GuaranteedViewPair::new(spend.0, Zeroizing::new(additional_key::(0).0)) } fn address_internal(spend: EdwardsPoint, subaddress: Option) -> Address { diff --git a/substrate/client/src/networks/monero.rs b/substrate/client/src/networks/monero.rs index c5954e193..bd5e0a15c 100644 --- a/substrate/client/src/networks/monero.rs +++ b/substrate/client/src/networks/monero.rs @@ -51,27 +51,24 @@ impl TryFrom> for Address { // Decode as SCALE let addr = EncodedAddress::decode(&mut data.as_ref()).map_err(|_| ())?; // Convert over - Ok(Address( - MoneroAddress::new( - Network::Mainnet, - match addr.kind { - EncodedAddressType::Legacy => AddressType::Legacy, - EncodedAddressType::Subaddress => AddressType::Subaddress, - EncodedAddressType::Featured(flags) => { - let subaddress = (flags & 1) != 0; - let integrated = (flags & (1 << 1)) != 0; - let guaranteed = (flags & (1 << 2)) != 0; - if integrated { - Err(())?; - } - AddressType::Featured { subaddress, payment_id: None, guaranteed } + Ok(Address(MoneroAddress::new( + Network::Mainnet, + match addr.kind { + EncodedAddressType::Legacy => AddressType::Legacy, + EncodedAddressType::Subaddress => AddressType::Subaddress, + EncodedAddressType::Featured(flags) => { + let subaddress = (flags & 1) != 0; + let integrated = (flags & (1 << 1)) != 0; + let guaranteed = (flags & (1 << 2)) != 0; + if integrated { + Err(())?; } - }, - 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(|_| ())?, - )) + AddressType::Featured { subaddress, payment_id: None, guaranteed } + } + }, + Ed25519::read_G::<&[u8]>(&mut addr.spend.as_ref()).map_err(|_| ())?.0, + Ed25519::read_G::<&[u8]>(&mut addr.view.as_ref()).map_err(|_| ())?.0, + ))) } } diff --git a/tests/full-stack/src/tests/mint_and_burn.rs b/tests/full-stack/src/tests/mint_and_burn.rs index 84b54a193..0a9da0115 100644 --- a/tests/full-stack/src/tests/mint_and_burn.rs +++ b/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::(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) }; diff --git a/tests/processor/src/networks.rs b/tests/processor/src/networks.rs index dd5d23b81..3902042fb 100644 --- a/tests/processor/src/networks.rs +++ b/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;