Monero: use only the first input ring length for RCT deserialization. (…

…#504)

* Use only the first input ring length for all RCT input signatures.

This is what Monero does:
https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/src/ringct/rctTypes.h#L422

https://github.com/monero-project/monero/blob/master/src/cryptonote_basic/cryptonote_basic.h#L308-L309

This isn't an issue for current transactions as from hf 12 Monero requires
all inputs to have the same number of decoys but for transactions before
that Monero would reject RCT txs with differing ring lengths. Monero would
deserialize each inputs signature using the ring length of the first so the
signatures for inputs other than the first would have a different
(wrong) number of elements for that input meaning the signature is invalid.

But as we are using the ring length of each input, which arguably is the
*correct* way, we would approve of transactions with inputs differing in
ring lengths.

* Check that there is more than one ring member for MLSAG signatures.

https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/src/ringct/rctSigs.cpp#L462

coins/monero/src/ringct/mlsag.rs

@@ -33,7 +33,10 @@ pub struct RingMatrix {
 
 impl RingMatrix {
   pub fn new(matrix: Vec<Vec<EdwardsPoint>>) -> Result<Self, MlsagError> {
-    if matrix.is_empty() {
+    // Monero requires that there is more than one ring member for MLSAG signatures:
+    // https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/
+    // src/ringct/rctSigs.cpp#L462
+    if matrix.len() < 2 {
       Err(MlsagError::InvalidRing)?;
     }
     for member in &matrix {

coins/monero/src/ringct/mod.rs

@@ -257,7 +257,8 @@ impl RctPrunable {
 
   pub fn read<R: Read>(
     rct_type: RctType,
-    decoys: &[usize],
+    ring_length: usize,
+    inputs: usize,
     outputs: usize,
     r: &mut R,
   ) -> io::Result<RctPrunable> {
@@ -268,19 +269,19 @@ impl RctPrunable {
     //   src/ringct/rctSigs.cpp#L609
     // And then for RctNull, that's only allowed for miner TXs which require one input of
     // Input::Gen
-    if decoys.is_empty() {
+    if inputs == 0 {
       Err(io::Error::other("transaction had no inputs"))?;
     }
 
     Ok(match rct_type {
       RctType::Null => RctPrunable::Null,
       RctType::MlsagAggregate => RctPrunable::AggregateMlsagBorromean {
         borromean: read_raw_vec(BorromeanRange::read, outputs, r)?,
-        mlsag: Mlsag::read(decoys[0], decoys.len() + 1, r)?,
+        mlsag: Mlsag::read(ring_length, inputs + 1, r)?,
       },
       RctType::MlsagIndividual => RctPrunable::MlsagBorromean {
         borromean: read_raw_vec(BorromeanRange::read, outputs, r)?,
-        mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::<Result<_, _>>()?,
+        mlsags: (0 .. inputs).map(|_| Mlsag::read(ring_length, 2, r)).collect::<Result<_, _>>()?,
       },
       RctType::Bulletproofs | RctType::BulletproofsCompactAmount => {
         RctPrunable::MlsagBulletproofs {
@@ -295,8 +296,10 @@ impl RctPrunable {
             }
             Bulletproofs::read(r)?
           },
-          mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::<Result<_, _>>()?,
-          pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?,
+          mlsags: (0 .. inputs)
+            .map(|_| Mlsag::read(ring_length, 2, r))
+            .collect::<Result<_, _>>()?,
+          pseudo_outs: read_raw_vec(read_point, inputs, r)?,
         }
       }
       RctType::Clsag | RctType::BulletproofsPlus => RctPrunable::Clsag {
@@ -308,8 +311,8 @@ impl RctPrunable {
             r,
           )?
         },
-        clsags: (0 .. decoys.len()).map(|o| Clsag::read(decoys[o], r)).collect::<Result<_, _>>()?,
-        pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?,
+        clsags: (0 .. inputs).map(|_| Clsag::read(ring_length, r)).collect::<Result<_, _>>()?,
+        pseudo_outs: read_raw_vec(read_point, inputs, r)?,
       },
     })
   }
@@ -382,8 +385,16 @@ impl RctSignatures {
     serialized
   }
 
-  pub fn read<R: Read>(decoys: &[usize], outputs: usize, r: &mut R) -> io::Result<RctSignatures> {
-    let base = RctBase::read(decoys.len(), outputs, r)?;
-    Ok(RctSignatures { base: base.0, prunable: RctPrunable::read(base.1, decoys, outputs, r)? })
+  pub fn read<R: Read>(
+    ring_length: usize,
+    inputs: usize,
+    outputs: usize,
+    r: &mut R,
+  ) -> io::Result<RctSignatures> {
+    let base = RctBase::read(inputs, outputs, r)?;
+    Ok(RctSignatures {
+      base: base.0,
+      prunable: RctPrunable::read(base.1, ring_length, inputs, outputs, r)?,
+    })
   }
 }

coins/monero/src/transaction.rs

@@ -331,14 +331,11 @@ impl Transaction {
       }
     } else if prefix.version == 2 {
       rct_signatures = RctSignatures::read(
-        &prefix
-          .inputs
-          .iter()
-          .map(|input| match input {
-            Input::Gen(_) => 0,
-            Input::ToKey { key_offsets, .. } => key_offsets.len(),
-          })
-          .collect::<Vec<_>>(),
+        prefix.inputs.first().map_or(0, |input| match input {
+          Input::Gen(_) => 0,
+          Input::ToKey { key_offsets, .. } => key_offsets.len(),
+        }),
+        prefix.inputs.len(),
         prefix.outputs.len(),
         r,
       )?;