From 5c106cecf6809cbfb612262a76f07c4e30a8d3de Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 22 Aug 2022 12:15:14 -0400 Subject: [PATCH] Fix https://github.com/serai-dex/serai/issues/105 --- coins/monero/src/rpc.rs | 98 +++------- coins/monero/src/wallet/decoys.rs | 4 +- coins/monero/src/wallet/scan.rs | 230 +++++++++++++++++------ coins/monero/src/wallet/send/mod.rs | 12 +- coins/monero/src/wallet/send/multisig.rs | 14 +- coins/monero/tests/send.rs | 16 +- processor/src/coin/mod.rs | 2 +- processor/src/coin/monero.rs | 35 +++- processor/src/wallet.rs | 2 +- 9 files changed, 258 insertions(+), 155 deletions(-) diff --git a/coins/monero/src/rpc.rs b/coins/monero/src/rpc.rs index abd6b394..68e533ba 100644 --- a/coins/monero/src/rpc.rs +++ b/coins/monero/src/rpc.rs @@ -138,12 +138,9 @@ impl Rpc { Ok(self.rpc_call::, HeightResponse>("get_height", None).await?.height) } - async fn get_transactions_core( - &self, - hashes: &[[u8; 32]], - ) -> Result<(Vec>, Vec<[u8; 32]>), RpcError> { + pub async fn get_transactions(&self, hashes: &[[u8; 32]]) -> Result, RpcError> { if hashes.is_empty() { - return Ok((vec![], vec![])); + return Ok(vec![]); } #[derive(Deserialize, Debug)] @@ -168,50 +165,34 @@ impl Rpc { ) .await?; - Ok(( - txs - .txs - .iter() - .map(|res| { - let tx = Transaction::deserialize(&mut std::io::Cursor::new( - rpc_hex(if !res.as_hex.is_empty() { &res.as_hex } else { &res.pruned_as_hex }).unwrap(), - )) - .map_err(|_| { - RpcError::InvalidTransaction(hex::decode(&res.tx_hash).unwrap().try_into().unwrap()) - })?; - - // https://github.com/monero-project/monero/issues/8311 - if res.as_hex.is_empty() { - match tx.prefix.inputs.get(0) { - Some(Input::Gen { .. }) => (), - _ => Err(RpcError::PrunedTransaction)?, - } - } - - Ok(tx) - }) - .collect(), - txs.missed_tx.iter().map(|hash| hex::decode(&hash).unwrap().try_into().unwrap()).collect(), - )) - } - - pub async fn get_transactions(&self, hashes: &[[u8; 32]]) -> Result, RpcError> { - let (txs, missed) = self.get_transactions_core(hashes).await?; - if !missed.is_empty() { - Err(RpcError::TransactionsNotFound(missed))?; + if txs.missed_tx.len() != 0 { + Err(RpcError::TransactionsNotFound( + txs.missed_tx.iter().map(|hash| hex::decode(&hash).unwrap().try_into().unwrap()).collect(), + ))?; } - // This will clone several KB and is accordingly inefficient - // TODO: Optimize - txs.iter().cloned().collect::>() - } - // TODO: Remove with https://github.com/serai-dex/serai/issues/25 - pub async fn get_transactions_possible( - &self, - hashes: &[[u8; 32]], - ) -> Result, RpcError> { - let (txs, _) = self.get_transactions_core(hashes).await?; - Ok(txs.iter().cloned().filter_map(|tx| tx.ok()).collect()) + txs + .txs + .iter() + .map(|res| { + let tx = Transaction::deserialize(&mut std::io::Cursor::new( + rpc_hex(if !res.as_hex.is_empty() { &res.as_hex } else { &res.pruned_as_hex }).unwrap(), + )) + .map_err(|_| { + RpcError::InvalidTransaction(hex::decode(&res.tx_hash).unwrap().try_into().unwrap()) + })?; + + // https://github.com/monero-project/monero/issues/8311 + if res.as_hex.is_empty() { + match tx.prefix.inputs.get(0) { + Some(Input::Gen { .. }) => (), + _ => Err(RpcError::PrunedTransaction)?, + } + } + + Ok(tx) + }) + .collect() } pub async fn get_block(&self, height: usize) -> Result { @@ -238,32 +219,13 @@ impl Rpc { ) } - async fn get_block_transactions_core( - &self, - height: usize, - possible: bool, - ) -> Result, RpcError> { + pub async fn get_block_transactions(&self, height: usize) -> Result, RpcError> { let block = self.get_block(height).await?; let mut res = vec![block.miner_tx]; - res.extend(if possible { - self.get_transactions_possible(&block.txs).await? - } else { - self.get_transactions(&block.txs).await? - }); + res.extend(self.get_transactions(&block.txs).await?); Ok(res) } - pub async fn get_block_transactions(&self, height: usize) -> Result, RpcError> { - self.get_block_transactions_core(height, false).await - } - - pub async fn get_block_transactions_possible( - &self, - height: usize, - ) -> Result, RpcError> { - self.get_block_transactions_core(height, true).await - } - pub async fn get_o_indexes(&self, hash: [u8; 32]) -> Result, RpcError> { #[derive(Serialize, Debug)] struct Request { diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index 72388d16..9debb473 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -140,8 +140,8 @@ impl Decoys { let mut real = Vec::with_capacity(inputs.len()); let mut outputs = Vec::with_capacity(inputs.len()); for input in inputs { - real.push(rpc.get_o_indexes(input.tx).await?[usize::from(input.o)]); - outputs.push((real[real.len() - 1], [input.key, input.commitment.calculate()])); + real.push(input.global_index); + outputs.push((real[real.len() - 1], [input.output.data.key, input.commitment().calculate()])); } let distribution_len = { diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 03512b46..f8de7aa2 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -8,22 +8,59 @@ use crate::{ Commitment, serialize::{read_byte, read_u32, read_u64, read_bytes, read_scalar, read_point}, transaction::{Timelock, Transaction}, + block::Block, + rpc::{Rpc, RpcError}, wallet::{PaymentId, Extra, Scanner, uniqueness, shared_key, amount_decryption, commitment_mask}, }; #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] -pub struct SpendableOutput { +pub struct AbsoluteId { pub tx: [u8; 32], pub o: u8, +} +impl AbsoluteId { + pub fn serialize(&self) -> Vec { + let mut res = Vec::with_capacity(32 + 1); + res.extend(&self.tx); + res.push(self.o); + res + } + + pub fn deserialize(r: &mut R) -> std::io::Result { + Ok(AbsoluteId { tx: read_bytes(r)?, o: read_byte(r)? }) + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] +pub struct OutputData { pub key: EdwardsPoint, - // Absolute difference between the spend key and the key in this output, inclusive of any - // subaddress offset + // Absolute difference between the spend key and the key in this output pub key_offset: Scalar, pub commitment: Commitment, +} - // Metadata to know how to process this output +impl OutputData { + pub fn serialize(&self) -> Vec { + let mut res = Vec::with_capacity(32 + 32 + 40); + res.extend(self.key.compress().to_bytes()); + res.extend(self.key_offset.to_bytes()); + res.extend(self.commitment.mask.to_bytes()); + res.extend(self.commitment.amount.to_le_bytes()); + res + } + pub fn deserialize(r: &mut R) -> std::io::Result { + Ok(OutputData { + key: read_point(r)?, + key_offset: read_scalar(r)?, + commitment: Commitment::new(read_scalar(r)?, read_u64(r)?), + }) + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] +pub struct Metadata { // Does not have to be an Option since the 0 subaddress is the main address pub subaddress: (u32, u32), // Can be an Option, as extra doesn't necessarily have a payment ID, yet all Monero TXs should @@ -35,14 +72,98 @@ pub struct SpendableOutput { pub payment_id: [u8; 8], } -#[derive(Zeroize, ZeroizeOnDrop)] -pub struct Timelocked(Timelock, Vec); -impl Timelocked { +impl Metadata { + pub fn serialize(&self) -> Vec { + let mut res = Vec::with_capacity(4 + 4 + 8); + res.extend(self.subaddress.0.to_le_bytes()); + res.extend(self.subaddress.1.to_le_bytes()); + res.extend(self.payment_id); + res + } + + pub fn deserialize(r: &mut R) -> std::io::Result { + Ok(Metadata { subaddress: (read_u32(r)?, read_u32(r)?), payment_id: read_bytes(r)? }) + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] +pub struct ReceivedOutput { + pub absolute: AbsoluteId, + pub data: OutputData, + pub metadata: Metadata, +} + +impl ReceivedOutput { + pub fn commitment(&self) -> Commitment { + self.data.commitment.clone() + } + + pub fn serialize(&self) -> Vec { + let mut serialized = self.absolute.serialize(); + serialized.extend(&self.data.serialize()); + serialized.extend(&self.metadata.serialize()); + serialized + } + + pub fn deserialize(r: &mut R) -> std::io::Result { + Ok(ReceivedOutput { + absolute: AbsoluteId::deserialize(r)?, + data: OutputData::deserialize(r)?, + metadata: Metadata::deserialize(r)?, + }) + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] +pub struct SpendableOutput { + pub output: ReceivedOutput, + pub global_index: u64, +} + +impl SpendableOutput { + pub async fn refresh_global_index(&mut self, rpc: &Rpc) -> Result<(), RpcError> { + self.global_index = + rpc.get_o_indexes(self.output.absolute.tx).await?[usize::from(self.output.absolute.o)]; + Ok(()) + } + + pub async fn from(rpc: &Rpc, output: ReceivedOutput) -> Result { + let mut output = SpendableOutput { output, global_index: 0 }; + output.refresh_global_index(rpc).await?; + Ok(output) + } + + pub fn commitment(&self) -> Commitment { + self.output.commitment() + } + + pub fn serialize(&self) -> Vec { + let mut serialized = self.output.serialize(); + serialized.extend(&self.global_index.to_le_bytes()); + serialized + } + + pub fn deserialize(r: &mut R) -> std::io::Result { + Ok(SpendableOutput { output: ReceivedOutput::deserialize(r)?, global_index: read_u64(r)? }) + } +} + +#[derive(Zeroize)] +pub struct Timelocked(Timelock, Vec); +impl Drop for Timelocked { + fn drop(&mut self) { + self.0.zeroize(); + self.1.zeroize(); + } +} +impl ZeroizeOnDrop for Timelocked {} + +impl Timelocked { pub fn timelock(&self) -> Timelock { self.0 } - pub fn not_locked(&self) -> Vec { + pub fn not_locked(&self) -> Vec { if self.0 == Timelock::None { return self.1.clone(); } @@ -50,52 +171,18 @@ impl Timelocked { } /// Returns None if the Timelocks aren't comparable. Returns Some(vec![]) if none are unlocked - pub fn unlocked(&self, timelock: Timelock) -> Option> { + pub fn unlocked(&self, timelock: Timelock) -> Option> { // If the Timelocks are comparable, return the outputs if they're now unlocked self.0.partial_cmp(&timelock).filter(|_| self.0 <= timelock).map(|_| self.1.clone()) } - pub fn ignore_timelock(&self) -> Vec { + pub fn ignore_timelock(&self) -> Vec { self.1.clone() } } -impl SpendableOutput { - pub fn serialize(&self) -> Vec { - let mut res = Vec::with_capacity(32 + 1 + 32 + 32 + 40); - - res.extend(&self.tx); - res.push(self.o); - - res.extend(self.key.compress().to_bytes()); - res.extend(self.key_offset.to_bytes()); - res.extend(self.commitment.mask.to_bytes()); - res.extend(self.commitment.amount.to_le_bytes()); - - res.extend(self.subaddress.0.to_le_bytes()); - res.extend(self.subaddress.1.to_le_bytes()); - res.extend(self.payment_id); - - res - } - - pub fn deserialize(r: &mut R) -> std::io::Result { - Ok(SpendableOutput { - tx: read_bytes(r)?, - o: read_byte(r)?, - - key: read_point(r)?, - key_offset: read_scalar(r)?, - commitment: Commitment::new(read_scalar(r)?, read_u64(r)?), - - subaddress: (read_u32(r)?, read_u32(r)?), - payment_id: read_bytes(r)?, - }) - } -} - impl Scanner { - pub fn scan(&mut self, tx: &Transaction) -> Timelocked { + pub fn scan_stateless(&mut self, tx: &Transaction) -> Timelocked { let extra = Extra::deserialize(&mut Cursor::new(&tx.prefix.extra)); let keys; let extra = if let Ok(extra) = extra { @@ -170,16 +257,16 @@ impl Scanner { } if commitment.amount != 0 { - res.push(SpendableOutput { - tx: tx.hash(), - o: o.try_into().unwrap(), + res.push(ReceivedOutput { + absolute: AbsoluteId { tx: tx.hash(), o: o.try_into().unwrap() }, - key: output.key, - key_offset: key_offset + self.pair.subaddress(*subaddress.unwrap()), - commitment, + data: OutputData { + key: output.key, + key_offset: key_offset + self.pair.subaddress(*subaddress.unwrap()), + commitment, + }, - subaddress: (0, 0), - payment_id, + metadata: Metadata { subaddress: (0, 0), payment_id }, }); if let Some(burning_bug) = self.burning_bug.as_mut() { @@ -194,4 +281,41 @@ impl Scanner { Timelocked(tx.prefix.timelock, res) } + + pub async fn scan( + &mut self, + rpc: &Rpc, + block: &Block, + ) -> Result>, RpcError> { + let mut index = rpc.get_o_indexes(block.miner_tx.hash()).await?[0]; + let mut txs = vec![block.miner_tx.clone()]; + txs.extend(rpc.get_transactions(&block.txs).await?); + + let map = |mut timelock: Timelocked, index| { + if timelock.1.is_empty() { + None + } else { + Some(Timelocked( + timelock.0, + timelock + .1 + .drain(..) + .map(|output| SpendableOutput { + global_index: index + u64::from(output.absolute.o), + output, + }) + .collect(), + )) + } + }; + + let mut res = vec![]; + for tx in txs { + if let Some(timelock) = map(self.scan_stateless(&tx), index) { + res.push(timelock); + } + index += u64::try_from(tx.prefix.outputs.len()).unwrap(); + } + Ok(res) + } } diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index 6210d430..4dee38ae 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -129,9 +129,9 @@ async fn prepare_inputs( for (i, input) in inputs.iter().enumerate() { signable.push(( - spend + input.key_offset, - generate_key_image(spend + input.key_offset), - ClsagInput::new(input.commitment.clone(), decoys[i].clone()) + spend + input.output.data.key_offset, + generate_key_image(spend + input.output.data.key_offset), + ClsagInput::new(input.commitment().clone(), decoys[i].clone()) .map_err(TransactionError::ClsagError)?, )); @@ -225,7 +225,7 @@ impl SignableTransaction { fee_rate.calculate(Transaction::fee_weight(protocol, inputs.len(), outputs, extra)); // Make sure we have enough funds - let in_amount = inputs.iter().map(|input| input.commitment.amount).sum::(); + let in_amount = inputs.iter().map(|input| input.commitment().amount).sum::(); let mut out_amount = payments.iter().map(|payment| payment.1).sum::() + fee; if in_amount < out_amount { Err(TransactionError::NotEnoughFunds(in_amount, out_amount))?; @@ -345,8 +345,8 @@ impl SignableTransaction { ) -> Result { let mut images = Vec::with_capacity(self.inputs.len()); for input in &self.inputs { - let mut offset = spend + input.key_offset; - if (&offset * &ED25519_BASEPOINT_TABLE) != input.key { + let mut offset = spend + input.output.data.key_offset; + if (&offset * &ED25519_BASEPOINT_TABLE) != input.output.data.key { Err(TransactionError::WrongPrivateKey)?; } diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index c8bd5e70..64575ce0 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -100,11 +100,11 @@ impl SignableTransaction { for input in &self.inputs { // These outputs can only be spent once. Therefore, it forces all RNGs derived from this // transcript (such as the one used to create one time keys) to be unique - transcript.append_message(b"input_hash", &input.tx); - transcript.append_message(b"input_output_index", &[input.o]); + transcript.append_message(b"input_hash", &input.output.absolute.tx); + transcript.append_message(b"input_output_index", &[input.output.absolute.o]); // Not including this, with a doxxed list of payments, would allow brute forcing the inputs // to determine RNG seeds and therefore the true spends - transcript.append_message(b"input_shared_key", &input.key_offset.to_bytes()); + transcript.append_message(b"input_shared_key", &input.output.data.key_offset.to_bytes()); } for payment in &self.payments { transcript.append_message(b"payment_address", payment.0.to_string().as_bytes()); @@ -116,14 +116,14 @@ impl SignableTransaction { for (i, input) in self.inputs.iter().enumerate() { // Check this the right set of keys - let offset = keys.offset(dalek_ff_group::Scalar(input.key_offset)); - if offset.group_key().0 != input.key { + let offset = keys.offset(dalek_ff_group::Scalar(input.output.data.key_offset)); + if offset.group_key().0 != input.output.data.key { Err(TransactionError::WrongPrivateKey)?; } clsags.push( AlgorithmMachine::new( - ClsagMultisig::new(transcript.clone(), input.key, inputs[i].clone()) + ClsagMultisig::new(transcript.clone(), input.output.data.key, inputs[i].clone()) .map_err(TransactionError::MultisigError)?, offset, &included, @@ -331,7 +331,7 @@ impl SignMachine for TransactionSignMachine { }); *value.3.write().unwrap() = Some(ClsagDetails::new( - ClsagInput::new(value.1.commitment.clone(), value.2).map_err(|_| { + ClsagInput::new(value.1.commitment().clone(), value.2).map_err(|_| { panic!("Signing an input which isn't present in the ring we created for it") })?, mask, diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index dbeff426..e24a747a 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -23,7 +23,7 @@ use frost::{ use monero_serai::{ random_scalar, - wallet::{ViewPair, Scanner, address::Network, SignableTransaction}, + wallet::{address::Network, ViewPair, Scanner, SpendableOutput, SignableTransaction}, }; mod rpc; @@ -98,13 +98,13 @@ async fn send_core(test: usize, multisig: bool) { // Grab the largest output available let output = { - let mut outputs = scanner.scan(tx.as_ref().unwrap()).ignore_timelock(); - outputs.sort_by(|x, y| x.commitment.amount.cmp(&y.commitment.amount).reverse()); + let mut outputs = scanner.scan_stateless(tx.as_ref().unwrap()).ignore_timelock(); + outputs.sort_by(|x, y| x.commitment().amount.cmp(&y.commitment().amount).reverse()); outputs.swap_remove(0) }; // Test creating a zero change output and a non-zero change output - amount = output.commitment.amount - u64::try_from(i).unwrap(); - outputs.push(output); + amount = output.commitment().amount - u64::try_from(i).unwrap(); + outputs.push(SpendableOutput::from(&rpc, output).await.unwrap()); // Test spending multiple inputs } else if test == 1 { @@ -122,9 +122,9 @@ async fn send_core(test: usize, multisig: bool) { } for i in (start + 1) .. (start + 9) { - let tx = rpc.get_block_transactions(i).await.unwrap().swap_remove(0); - let output = scanner.scan(&tx).ignore_timelock().swap_remove(0); - amount += output.commitment.amount; + let mut txs = scanner.scan(&rpc, &rpc.get_block(i).await.unwrap()).await.unwrap(); + let output = txs.swap_remove(0).ignore_timelock().swap_remove(0); + amount += output.commitment().amount; outputs.push(output); } } diff --git a/processor/src/coin/mod.rs b/processor/src/coin/mod.rs index 7a8ce22b..88d9d98c 100644 --- a/processor/src/coin/mod.rs +++ b/processor/src/coin/mod.rs @@ -53,7 +53,7 @@ pub trait Coin { &self, block: &Self::Block, key: ::G, - ) -> Vec; + ) -> Result, CoinError>; async fn prepare_send( &self, diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index 1abfa9e4..f28afad2 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -8,6 +8,7 @@ use frost::{curve::Ed25519, FrostKeys}; use monero_serai::{ transaction::Transaction, + block::Block, rpc::Rpc, wallet::{ ViewPair, Scanner, @@ -36,11 +37,11 @@ impl OutputTrait for Output { type Id = [u8; 32]; fn id(&self) -> Self::Id { - self.0.key.compress().to_bytes() + self.0.output.data.key.compress().to_bytes() } fn amount(&self) -> u64 { - self.0.commitment.amount + self.0.commitment().amount } fn serialize(&self) -> Vec { @@ -97,7 +98,7 @@ impl Coin for Monero { type Fee = Fee; type Transaction = Transaction; - type Block = Vec; + type Block = Block; type Output = Output; type SignableTransaction = SignableTransaction; @@ -125,12 +126,25 @@ impl Coin for Monero { } async fn get_block(&self, height: usize) -> Result { - self.rpc.get_block_transactions_possible(height).await.map_err(|_| CoinError::ConnectionError) + self.rpc.get_block(height).await.map_err(|_| CoinError::ConnectionError) } - async fn get_outputs(&self, block: &Self::Block, key: dfg::EdwardsPoint) -> Vec { - let mut scanner = self.scanner(key); - block.iter().flat_map(|tx| scanner.scan(tx).not_locked()).map(Output::from).collect() + async fn get_outputs( + &self, + block: &Self::Block, + key: dfg::EdwardsPoint, + ) -> Result, CoinError> { + Ok( + self + .scanner(key) + .scan(&self.rpc, block) + .await + .map_err(|_| CoinError::ConnectionError)? + .iter() + .flat_map(|outputs| outputs.not_locked()) + .map(Output::from) + .collect(), + ) } async fn prepare_send( @@ -221,10 +235,13 @@ impl Coin for Monero { } let outputs = Self::empty_scanner() - .scan(&self.rpc.get_block_transactions_possible(height).await.unwrap().swap_remove(0)) + .scan(&self.rpc, &self.rpc.get_block(height).await.unwrap()) + .await + .unwrap() + .swap_remove(0) .ignore_timelock(); - let amount = outputs[0].commitment.amount; + let amount = outputs[0].commitment().amount; let fee = 3000000000; // TODO let tx = MSignableTransaction::new( self.rpc.get_protocol().await.unwrap(), diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index 5cb5949d..72457054 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -262,7 +262,7 @@ impl Wallet { self .coin .get_outputs(&block, keys.group_key()) - .await + .await? .iter() .cloned() .filter(|output| self.db.add_output(output)),