From d5a5704ba437db8880613948aa8d19d98a6f0527 Mon Sep 17 00:00:00 2001 From: akildemir <34187742+akildemir@users.noreply.github.com> Date: Fri, 9 Dec 2022 18:58:11 +0300 Subject: [PATCH] support add/read multiple arbitrary tx data (#189) * support add/read multiple arbitrary tx data * fix clippy errors * resolve pr issues --- coins/monero/src/wallet/extra.rs | 17 +++--- coins/monero/src/wallet/scan.rs | 24 ++++---- coins/monero/src/wallet/send/builder.rs | 22 ++++--- coins/monero/src/wallet/send/mod.rs | 16 ++--- coins/monero/tests/add_data.rs | 79 +++++++++++++++++++++++++ processor/src/coin/monero.rs | 4 +- 6 files changed, 128 insertions(+), 34 deletions(-) create mode 100644 coins/monero/tests/add_data.rs diff --git a/coins/monero/src/wallet/extra.rs b/coins/monero/src/wallet/extra.rs index fdc2ef9e..ac8924a4 100644 --- a/coins/monero/src/wallet/extra.rs +++ b/coins/monero/src/wallet/extra.rs @@ -10,6 +10,8 @@ use crate::serialize::{ write_point, write_vec, }; +pub const MAX_TX_EXTRA_NONCE_SIZE: usize = 255; + #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub(crate) enum PaymentId { Unencrypted([u8; 32]), @@ -91,7 +93,7 @@ impl ExtraField { 1 => ExtraField::PublicKey(read_point(r)?), 2 => ExtraField::Nonce({ let nonce = read_vec(read_byte, r)?; - if nonce.len() > 255 { + if nonce.len() > MAX_TX_EXTRA_NONCE_SIZE { Err(io::Error::new(io::ErrorKind::Other, "too long nonce"))?; } nonce @@ -131,8 +133,9 @@ impl Extra { None } - pub(crate) fn data(&self) -> Option> { + pub(crate) fn data(&self) -> Vec> { let mut first = true; + let mut res = vec![]; for field in &self.0 { if let ExtraField::Nonce(data) = field { // Skip the first Nonce, which should be the payment ID @@ -140,10 +143,10 @@ impl Extra { first = false; continue; } - return Some(data.clone()); + res.push(data.clone()); } } - None + res } pub(crate) fn new(mut keys: Vec) -> Extra { @@ -162,15 +165,15 @@ impl Extra { } #[rustfmt::skip] - pub(crate) fn fee_weight(outputs: usize, data: Option<&Vec>) -> usize { + pub(crate) fn fee_weight(outputs: usize, data: &[Vec]) -> usize { // PublicKey, key (1 + 32) + // PublicKeys, length, additional keys (1 + 1 + (outputs.saturating_sub(1) * 32)) + // PaymentId (Nonce), length, encrypted, ID (1 + 1 + 1 + 8) + - // Nonce, length, data (if existant) - data.map(|v| 1 + varint_len(v.len()) + v.len()).unwrap_or(0) + // Nonce, length, data (if existent) + data.iter().map(|v| 1 + varint_len(v.len()) + v.len()).sum::() } pub(crate) fn serialize(&self, w: &mut W) -> io::Result<()> { diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index bd790693..b01a9305 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -73,7 +73,7 @@ pub struct Metadata { // have this making it simplest for it to be as-is. pub payment_id: [u8; 8], /// Arbitrary data encoded in TX extra. - pub arbitrary_data: Option>, + pub arbitrary_data: Vec>, } impl Metadata { @@ -82,11 +82,11 @@ impl Metadata { res.extend(self.subaddress.0.to_le_bytes()); res.extend(self.subaddress.1.to_le_bytes()); res.extend(self.payment_id); - if let Some(data) = self.arbitrary_data.as_ref() { - res.extend([1, u8::try_from(data.len()).unwrap()]); - res.extend(data); - } else { - res.extend([0]); + + res.extend(u32::try_from(self.arbitrary_data.len()).unwrap().to_le_bytes()); + for part in &self.arbitrary_data { + res.extend([u8::try_from(part.len()).unwrap()]); + res.extend(part); } res } @@ -96,12 +96,12 @@ impl Metadata { subaddress: (read_u32(r)?, read_u32(r)?), payment_id: read_bytes(r)?, arbitrary_data: { - if read_byte(r)? == 1 { + let mut data = vec![]; + for _ in 0 .. read_u32(r)? { let len = read_byte(r)?; - Some(read_raw_vec(read_byte, usize::from(len), r)?) - } else { - None + data.push(read_raw_vec(read_byte, usize::from(len), r)?); } + data }, }) } @@ -128,6 +128,10 @@ impl ReceivedOutput { self.data.commitment.clone() } + pub fn arbitrary_data(&self) -> &[Vec] { + &self.metadata.arbitrary_data + } + pub fn serialize(&self) -> Vec { let mut serialized = self.absolute.serialize(); serialized.extend(&self.data.serialize()); diff --git a/coins/monero/src/wallet/send/builder.rs b/coins/monero/src/wallet/send/builder.rs index 140c6089..b137dcbf 100644 --- a/coins/monero/src/wallet/send/builder.rs +++ b/coins/monero/src/wallet/send/builder.rs @@ -4,7 +4,10 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::{ Protocol, - wallet::{address::MoneroAddress, Fee, SpendableOutput, SignableTransaction, TransactionError}, + wallet::{ + address::MoneroAddress, Fee, SpendableOutput, SignableTransaction, TransactionError, + extra::MAX_TX_EXTRA_NONCE_SIZE, + }, }; #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] @@ -15,14 +18,14 @@ struct SignableTransactionBuilderInternal { inputs: Vec, payments: Vec<(MoneroAddress, u64)>, change_address: Option, - data: Option>, + data: Vec>, } impl SignableTransactionBuilderInternal { // Takes in the change address so users don't miss that they have to manually set one // If they don't, all leftover funds will become part of the fee fn new(protocol: Protocol, fee: Fee, change_address: Option) -> Self { - Self { protocol, fee, inputs: vec![], payments: vec![], change_address, data: None } + Self { protocol, fee, inputs: vec![], payments: vec![], change_address, data: vec![] } } fn add_input(&mut self, input: SpendableOutput) { @@ -39,8 +42,8 @@ impl SignableTransactionBuilderInternal { self.payments.extend(payments); } - fn set_data(&mut self, data: Vec) { - self.data = Some(data); + fn add_data(&mut self, data: Vec) { + self.data.push(data); } } @@ -100,9 +103,12 @@ impl SignableTransactionBuilder { self.shallow_copy() } - pub fn set_data(&mut self, data: Vec) -> Self { - self.0.write().unwrap().set_data(data); - self.shallow_copy() + pub fn add_data(&mut self, data: Vec) -> Result { + if data.len() > MAX_TX_EXTRA_NONCE_SIZE { + Err(TransactionError::TooMuchData)?; + } + self.0.write().unwrap().add_data(data); + Ok(self.shallow_copy()) } pub fn build(self) -> Result { diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index b4b6d6e6..9732c051 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -24,7 +24,7 @@ use crate::{ rpc::{Rpc, RpcError}, wallet::{ address::MoneroAddress, SpendableOutput, Decoys, PaymentId, ExtraField, Extra, key_image_sort, - uniqueness, shared_key, commitment_mask, amount_encryption, + uniqueness, shared_key, commitment_mask, amount_encryption, extra::MAX_TX_EXTRA_NONCE_SIZE, }, }; @@ -177,7 +177,7 @@ pub struct SignableTransaction { protocol: Protocol, inputs: Vec, payments: Vec<(MoneroAddress, u64)>, - data: Option>, + data: Vec>, fee: u64, } @@ -191,7 +191,7 @@ impl SignableTransaction { inputs: Vec, mut payments: Vec<(MoneroAddress, u64)>, change_address: Option, - data: Option>, + data: Vec>, fee_rate: Fee, ) -> Result { // Make sure there's only one payment ID @@ -220,8 +220,10 @@ impl SignableTransaction { Err(TransactionError::NoOutputs)?; } - if data.as_ref().map(|v| v.len()).unwrap_or(0) > 255 { - Err(TransactionError::TooMuchData)?; + for part in &data { + if part.len() > MAX_TX_EXTRA_NONCE_SIZE { + Err(TransactionError::TooMuchData)?; + } } // TODO TX MAX SIZE @@ -313,8 +315,8 @@ impl SignableTransaction { extra.push(ExtraField::Nonce(id_vec)); // Include data if present - if let Some(data) = self.data.take() { - extra.push(ExtraField::Nonce(data)); + for part in self.data.drain(..) { + extra.push(ExtraField::Nonce(part)); } let mut serialized = Vec::with_capacity(Extra::fee_weight(outputs.len(), self.data.as_ref())); diff --git a/coins/monero/tests/add_data.rs b/coins/monero/tests/add_data.rs new file mode 100644 index 00000000..280f80fd --- /dev/null +++ b/coins/monero/tests/add_data.rs @@ -0,0 +1,79 @@ +use monero_serai::{rpc::Rpc, wallet::TransactionError, transaction::Transaction}; + +mod runner; + +test!( + add_single_data_less_than_255, + ( + |_, mut builder: Builder, addr| async move { + // make a data that is less than 255 bytes + let arbitrary_data = Vec::from("this is an arbitrary data less than 255 bytes"); + + // make sure we can add to tx + let result = builder.add_data(arbitrary_data.clone()); + assert!(result.is_ok()); + + builder.add_payment(addr, 5); + (builder.build().unwrap(), (arbitrary_data,)) + }, + |rpc: Rpc, signed: Transaction, mut scanner: Scanner, state: (Vec,)| async move { + let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + assert_eq!(output.arbitrary_data()[0], state.0); + }, + ), +); + +test!( + add_multiple_data_less_than_255, + ( + |_, mut builder: Builder, addr| async move { + // make a data that is less than 255 bytes + let arbitrary_data = Vec::from("this is an arbitrary data less than 255 bytes"); + + // add tx multiple times + for _ in 0 .. 5 { + let result = builder.add_data(arbitrary_data.clone()); + assert!(result.is_ok()); + } + + builder.add_payment(addr, 5); + (builder.build().unwrap(), (arbitrary_data,)) + }, + |rpc: Rpc, signed: Transaction, mut scanner: Scanner, state: (Vec,)| async move { + let tx = rpc.get_transaction(signed.hash()).await.unwrap(); + let output = scanner.scan_transaction(&tx).not_locked().swap_remove(0); + assert_eq!(output.commitment().amount, 5); + let data = output.arbitrary_data(); + for i in 0 .. 5 { + assert_eq!(data[i], state.0); + } + }, + ), +); + +test!( + add_single_data_more_than_255, + ( + |_, mut builder: Builder, addr| async move { + // make a data that is bigger than 255 bytes + let mut arbitrary_data = vec![]; + for _ in 0 .. 256 { + arbitrary_data.push(b'a'); + } + + // make sure we get an error if we try to add it to tx + let mut result = builder.add_payment(addr, 5).add_data(arbitrary_data.clone()); + assert_eq!(result, Err(TransactionError::TooMuchData)); + + // reduce data size and re-try + arbitrary_data.swap_remove(0); + result = builder.add_data(arbitrary_data); + + assert!(result.is_ok()); + (builder.build().unwrap(), ()) + }, + |_, _, _, _| async move {}, + ), +); diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index 32841179..af2e28e4 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -178,7 +178,7 @@ impl Coin for Monero { inputs.drain(..).map(|input| input.0).collect(), payments.to_vec(), Some(self.address(spend)), - None, + vec![], fee, ) .map_err(|_| CoinError::ConnectionError)?, @@ -261,7 +261,7 @@ impl Coin for Monero { outputs, vec![(address, amount - fee)], Some(Self::empty_address()), - None, + vec![], self.rpc.get_fee().await.unwrap(), ) .unwrap()