From 139dcde69c1e7c4415e50e6b2e3f5c8d521a1655 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 30 Aug 2022 15:42:23 -0400 Subject: [PATCH] Support including arbitrary data in TXs and return it with outputs Fixes a bug where all payments identified as being to (0, 0) instead of their actual subaddress. --- coins/monero/src/wallet/extra.rs | 62 ++++++++++++++++------------- coins/monero/src/wallet/scan.rs | 31 ++++++++++++--- coins/monero/src/wallet/send/mod.rs | 24 ++++++++--- coins/monero/tests/send.rs | 1 + processor/src/coin/monero.rs | 2 + 5 files changed, 82 insertions(+), 38 deletions(-) diff --git a/coins/monero/src/wallet/extra.rs b/coins/monero/src/wallet/extra.rs index 97ca033a..fdc2ef9e 100644 --- a/coins/monero/src/wallet/extra.rs +++ b/coins/monero/src/wallet/extra.rs @@ -1,13 +1,13 @@ use core::ops::BitXor; -use std::io::{self, Read, Write}; +use std::io::{self, Read, Write, Cursor}; use zeroize::Zeroize; use curve25519_dalek::edwards::EdwardsPoint; use crate::serialize::{ - read_byte, read_bytes, read_varint, read_point, read_vec, write_byte, write_varint, write_point, - write_vec, + varint_len, read_byte, read_bytes, read_varint, read_point, read_vec, write_byte, write_varint, + write_point, write_vec, }; #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] @@ -30,7 +30,7 @@ impl BitXor<[u8; 8]> for PaymentId { } impl PaymentId { - fn serialize(&self, w: &mut W) -> io::Result<()> { + pub(crate) fn serialize(&self, w: &mut W) -> io::Result<()> { match self { PaymentId::Unencrypted(id) => { w.write_all(&[0])?; @@ -53,11 +53,11 @@ impl PaymentId { } } +// Doesn't bother with padding nor MinerGate #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub(crate) enum ExtraField { - Padding(Vec), PublicKey(EdwardsPoint), - PaymentId(PaymentId), // Technically Nonce, an arbitrary data field, yet solely used as PaymentId + Nonce(Vec), MergeMining(usize, [u8; 32]), PublicKeys(Vec), } @@ -65,19 +65,13 @@ pub(crate) enum ExtraField { impl ExtraField { fn serialize(&self, w: &mut W) -> io::Result<()> { match self { - ExtraField::Padding(data) => { - w.write_all(&[0])?; - write_vec(write_byte, data, w)?; - } ExtraField::PublicKey(key) => { w.write_all(&[1])?; w.write_all(&key.compress().to_bytes())?; } - ExtraField::PaymentId(id) => { + ExtraField::Nonce(data) => { w.write_all(&[2])?; - let mut buf = Vec::with_capacity(1 + 8); - id.serialize(&mut buf)?; - write_vec(write_byte, &buf, w)?; + write_vec(write_byte, data, w)?; } ExtraField::MergeMining(height, merkle) => { w.write_all(&[3])?; @@ -94,15 +88,14 @@ impl ExtraField { fn deserialize(r: &mut R) -> io::Result { Ok(match read_byte(r)? { - 0 => { - let res = read_vec(read_byte, r)?; - if res.len() > 255 { - Err(io::Error::new(io::ErrorKind::Other, "too long padding"))?; - } - ExtraField::Padding(res) - } 1 => ExtraField::PublicKey(read_point(r)?), - 2 => ExtraField::PaymentId(PaymentId::deserialize(r)?), + 2 => ExtraField::Nonce({ + let nonce = read_vec(read_byte, r)?; + if nonce.len() > 255 { + Err(io::Error::new(io::ErrorKind::Other, "too long nonce"))?; + } + nonce + }), 3 => ExtraField::MergeMining( usize::try_from(read_varint(r)?) .map_err(|_| io::Error::new(io::ErrorKind::Other, "varint for height exceeds usize"))?, @@ -131,16 +124,22 @@ impl Extra { pub(crate) fn payment_id(&self) -> Option { for field in &self.0 { - if let ExtraField::PaymentId(id) = field { - return Some(*id); + if let ExtraField::Nonce(data) = field { + return PaymentId::deserialize(&mut Cursor::new(data)).ok(); } } None } pub(crate) fn data(&self) -> Option> { + let mut first = true; for field in &self.0 { - if let ExtraField::Padding(data) = field { + if let ExtraField::Nonce(data) = field { + // Skip the first Nonce, which should be the payment ID + if first { + first = false; + continue; + } return Some(data.clone()); } } @@ -162,9 +161,16 @@ impl Extra { self.0.push(field); } - pub(crate) fn fee_weight(outputs: usize) -> usize { - // PublicKey, key, PublicKeys, length, additional keys, PaymentId, length, encrypted, ID - 33 + 2 + (outputs.saturating_sub(1) * 32) + 11 + #[rustfmt::skip] + pub(crate) fn fee_weight(outputs: usize, data: Option<&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) } 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 427853b6..587c827b 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -6,7 +6,7 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwar use crate::{ Commitment, - serialize::{read_byte, read_u32, read_u64, read_bytes, read_scalar, read_point}, + serialize::{read_byte, read_u32, read_u64, read_bytes, read_scalar, read_point, read_raw_vec}, transaction::{Timelock, Transaction}, block::Block, rpc::{Rpc, RpcError}, @@ -69,19 +69,38 @@ pub struct Metadata { // 0xff was chosen as it'd be distinct from [0; 8], enabling atomically incrementing IDs (though // they should be randomly generated) pub payment_id: [u8; 8], + // Arbitrary data + pub arbitrary_data: Option>, } impl Metadata { pub fn serialize(&self) -> Vec { - let mut res = Vec::with_capacity(4 + 4 + 8); + let mut res = Vec::with_capacity(4 + 4 + 8 + 1); 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 } pub fn deserialize(r: &mut R) -> std::io::Result { - Ok(Metadata { subaddress: (read_u32(r)?, read_u32(r)?), payment_id: read_bytes(r)? }) + Ok(Metadata { + subaddress: (read_u32(r)?, read_u32(r)?), + payment_id: read_bytes(r)?, + arbitrary_data: { + if read_byte(r)? == 1 { + let len = read_byte(r)?; + Some(read_raw_vec(read_byte, usize::from(len), r)?) + } else { + None + } + }, + }) } } @@ -251,13 +270,15 @@ impl Scanner { if subaddress.is_none() { continue; } + let subaddress = *subaddress.unwrap(); + // If it has torsion, it'll substract the non-torsioned shared key to a torsioned key // We will not have a torsioned key in our HashMap of keys, so we wouldn't identify it as // ours // If we did though, it'd enable bypassing the included burning bug protection debug_assert!(output_key.is_torsion_free()); - let key_offset = shared_key + self.pair.subaddress(*subaddress.unwrap()); + let key_offset = shared_key + self.pair.subaddress(subaddress); // Since we've found an output to us, get its amount let mut commitment = Commitment::zero(); @@ -288,7 +309,7 @@ impl Scanner { data: OutputData { key: output_key, key_offset, commitment }, - metadata: Metadata { subaddress: (0, 0), payment_id }, + metadata: Metadata { subaddress: subaddress, payment_id, arbitrary_data: extra.data() }, }); if let Some(burning_bug) = self.burning_bug.as_mut() { diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index 5f798fd6..0eff16cd 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -88,6 +88,8 @@ pub enum TransactionError { NoChange, #[error("too many outputs")] TooManyOutputs, + #[error("too much data")] + TooMuchData, #[error("not enough funds (in {0}, out {1})")] NotEnoughFunds(u64, u64), #[error("wrong spend private key")] @@ -171,6 +173,7 @@ pub struct SignableTransaction { protocol: Protocol, inputs: Vec, payments: Vec<(Address, u64)>, + data: Option>, fee: u64, } @@ -180,6 +183,7 @@ impl SignableTransaction { inputs: Vec, mut payments: Vec<(Address, u64)>, change_address: Option
, + data: Option>, fee_rate: Fee, ) -> Result { // Make sure there's only one payment ID @@ -208,6 +212,10 @@ impl SignableTransaction { Err(TransactionError::NoOutputs)?; } + if data.as_ref().map(|v| v.len()).unwrap_or(0) > 255 { + Err(TransactionError::TooMuchData)?; + } + // TODO TX MAX SIZE // If we don't have two outputs, as required by Monero, add a second @@ -218,7 +226,7 @@ impl SignableTransaction { let outputs = payments.len() + (if change { 1 } else { 0 }); // Calculate the extra length - let extra = Extra::fee_weight(outputs); + let extra = Extra::fee_weight(outputs, data.as_ref()); // Calculate the fee. let mut fee = @@ -252,7 +260,7 @@ impl SignableTransaction { Err(TransactionError::TooManyOutputs)?; } - Ok(SignableTransaction { protocol, inputs, payments, fee }) + Ok(SignableTransaction { protocol, inputs, payments, data, fee }) } fn prepare_transaction( @@ -292,10 +300,16 @@ impl SignableTransaction { let extra = { let mut extra = Extra::new(outputs.iter().map(|output| output.R).collect()); - // Additionally include a random payment ID - extra.push(ExtraField::PaymentId(PaymentId::Encrypted(id))); + let mut id_vec = Vec::with_capacity(1 + 8); + PaymentId::Encrypted(id).serialize(&mut id_vec).unwrap(); + extra.push(ExtraField::Nonce(id_vec)); - let mut serialized = Vec::with_capacity(Extra::fee_weight(outputs.len())); + // Include data if present + if let Some(data) = self.data.take() { + extra.push(ExtraField::Nonce(data)); + } + + let mut serialized = Vec::with_capacity(Extra::fee_weight(outputs.len(), self.data.as_ref())); extra.serialize(&mut serialized).unwrap(); serialized }; diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index 18bcd3e5..2950298a 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -134,6 +134,7 @@ async fn send_core(test: usize, multisig: bool) { outputs, vec![(addr, amount - 10000000000)], Some(addr), + None, fee, ) .unwrap(); diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index 64c600c3..436caa12 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -166,6 +166,7 @@ impl Coin for Monero { inputs.drain(..).map(|input| input.0).collect(), payments.to_vec(), Some(self.address(spend)), + None, fee, ) .map_err(|_| CoinError::ConnectionError)?, @@ -245,6 +246,7 @@ impl Coin for Monero { outputs, vec![(address, amount - fee)], Some(Self::empty_address()), + None, self.rpc.get_fee().await.unwrap(), ) .unwrap()