From a95ecc25123b000e77df711997d4f59872f12385 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 29 Jun 2023 13:16:51 -0400 Subject: [PATCH] Represent RCT amounts with None, not 0. Fixes #282. Does allow any v1 TXs which exist, and v2 miner-TXs, to specify Some(0). As far as I can tell, both were/are theoreitcally possible. --- coins/monero/src/transaction.rs | 61 +++++++++++++++++------- coins/monero/src/wallet/scan.rs | 10 ++-- coins/monero/src/wallet/send/mod.rs | 8 ++-- coins/monero/src/wallet/send/multisig.rs | 4 +- 4 files changed, 55 insertions(+), 28 deletions(-) diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 203edb72..c5c9d0ee 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -20,7 +20,7 @@ use crate::{ #[derive(Clone, PartialEq, Eq, Debug)] pub enum Input { Gen(u64), - ToKey { amount: u64, key_offsets: Vec, key_image: EdwardsPoint }, + ToKey { amount: Option, key_offsets: Vec, key_image: EdwardsPoint }, } impl Input { @@ -40,7 +40,7 @@ impl Input { Input::ToKey { amount, key_offsets, key_image } => { w.write_all(&[2])?; - write_varint(amount, w)?; + write_varint(&amount.unwrap_or(0), w)?; write_vec(write_varint, key_offsets, w)?; write_point(key_image, w) } @@ -53,14 +53,18 @@ impl Input { res } - pub fn read(r: &mut R) -> io::Result { + pub fn read(interpret_as_rct: bool, r: &mut R) -> io::Result { Ok(match read_byte(r)? { 255 => Input::Gen(read_varint(r)?), - 2 => Input::ToKey { - amount: read_varint(r)?, - key_offsets: read_vec(read_varint, r)?, - key_image: read_torsion_free_point(r)?, - }, + 2 => { + let amount = read_varint(r)?; + let amount = if (amount == 0) && interpret_as_rct { None } else { Some(amount) }; + Input::ToKey { + amount, + key_offsets: read_vec(read_varint, r)?, + key_image: read_torsion_free_point(r)?, + } + } _ => { Err(io::Error::new(io::ErrorKind::Other, "Tried to deserialize unknown/unused input type"))? } @@ -71,7 +75,7 @@ impl Input { // Doesn't bother moving to an enum for the unused Script classes #[derive(Clone, PartialEq, Eq, Debug)] pub struct Output { - pub amount: u64, + pub amount: Option, pub key: CompressedEdwardsY, pub view_tag: Option, } @@ -82,7 +86,7 @@ impl Output { } pub fn write(&self, w: &mut W) -> io::Result<()> { - write_varint(&self.amount, w)?; + write_varint(&self.amount.unwrap_or(0), w)?; w.write_all(&[2 + u8::from(self.view_tag.is_some())])?; w.write_all(&self.key.to_bytes())?; if let Some(view_tag) = self.view_tag { @@ -97,8 +101,17 @@ impl Output { res } - pub fn read(r: &mut R) -> io::Result { + pub fn read(interpret_as_rct: bool, r: &mut R) -> io::Result { let amount = read_varint(r)?; + let amount = if interpret_as_rct { + if amount != 0 { + Err(io::Error::new(io::ErrorKind::Other, "RCT TX output wasn't 0"))?; + } + None + } else { + Some(amount) + }; + let view_tag = match read_byte(r)? { 2 => false, 3 => true, @@ -194,11 +207,25 @@ impl TransactionPrefix { } pub fn read(r: &mut R) -> io::Result { + let version = read_varint(r)?; + // TODO: Create an enum out of version + if (version == 0) || (version > 2) { + Err(io::Error::new(io::ErrorKind::Other, "unrecognized transaction version"))?; + } + + let timelock = Timelock::from_raw(read_varint(r)?); + + let inputs = read_vec(|r| Input::read(version == 2, r), r)?; + if inputs.is_empty() { + Err(io::Error::new(io::ErrorKind::Other, "transaction had no inputs"))?; + } + let is_miner_tx = matches!(inputs[0], Input::Gen { .. }); + let mut prefix = TransactionPrefix { - version: read_varint(r)?, - timelock: Timelock::from_raw(read_varint(r)?), - inputs: read_vec(Input::read, r)?, - outputs: read_vec(Output::read, r)?, + version, + timelock, + inputs, + outputs: read_vec(|r| Output::read((!is_miner_tx) && (version == 2), r), r)?, extra: vec![], }; prefix.extra = read_vec(read_byte, r)?; @@ -263,10 +290,10 @@ impl Transaction { .iter() .map(|input| match input { Input::Gen(..) => 0, - Input::ToKey { amount, .. } => *amount, + Input::ToKey { amount, .. } => amount.unwrap(), }) .sum::() - .saturating_sub(prefix.outputs.iter().map(|output| output.amount).sum()); + .saturating_sub(prefix.outputs.iter().map(|output| output.amount.unwrap()).sum()); } else if prefix.version == 2 { rct_signatures = RctSignatures::read( prefix diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 89e9b5fb..5f217fda 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -375,8 +375,8 @@ impl Scanner { let mut commitment = Commitment::zero(); // Miner transaction - if output.amount != 0 { - commitment.amount = output.amount; + if let Some(amount) = output.amount { + commitment.amount = amount; // Regular transaction } else { let amount = match tx.rct_signatures.base.ecdh_info.get(o) { @@ -458,10 +458,10 @@ impl Scanner { tx.prefix .outputs .iter() - // Filter to miner TX outputs/0-amount outputs since we're tacking the 0-amount index - // This will fail to scan blocks containing pre-RingCT miner TXs + // Filter to v2 miner TX outputs/RCT outputs since we're tracking the RCT output index .filter(|output| { - matches!(tx.prefix.inputs.get(0), Some(Input::Gen(..))) || (output.amount == 0) + ((tx.prefix.version == 2) && matches!(tx.prefix.inputs.get(0), Some(Input::Gen(..)))) || + output.amount.is_none() }) .count(), ) diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index cecd897d..e722df2a 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -184,7 +184,7 @@ async fn prepare_inputs( )); tx.prefix.inputs.push(Input::ToKey { - amount: 0, + amount: None, key_offsets: decoys[i].offsets.clone(), key_image: signable[i].1, }); @@ -633,7 +633,7 @@ impl SignableTransaction { for output in &outputs { fee -= output.commitment.amount; tx_outputs.push(Output { - amount: 0, + amount: None, key: output.dest.compress(), view_tag: Some(output.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), }); @@ -691,7 +691,7 @@ impl SignableTransaction { uniqueness( &images .iter() - .map(|image| Input::ToKey { amount: 0, key_offsets: vec![], key_image: *image }) + .map(|image| Input::ToKey { amount: None, key_offsets: vec![], key_image: *image }) .collect::>(), ), ); @@ -750,7 +750,7 @@ impl Eventuality { for (o, (expected, actual)) in outputs.iter().zip(tx.prefix.outputs.iter()).enumerate() { // Verify the output, commitment, and encrypted amount. if (&Output { - amount: 0, + amount: None, key: expected.dest.compress(), view_tag: Some(expected.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), } != actual) || diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index bb249bc4..42c1073b 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -340,7 +340,7 @@ impl SignMachine for TransactionSignMachine { uniqueness( &sorted_images .iter() - .map(|image| Input::ToKey { amount: 0, key_offsets: vec![], key_image: *image }) + .map(|image| Input::ToKey { amount: None, key_offsets: vec![], key_image: *image }) .collect::>(), ), ) @@ -373,7 +373,7 @@ impl SignMachine for TransactionSignMachine { } tx.prefix.inputs.push(Input::ToKey { - amount: 0, + amount: None, key_offsets: value.2.offsets.clone(), key_image: value.0, });