Fix the known issue with the DSA

I wrote it to only select TXs with a timelock, not only TXs which are unlocked.
This most likely explains why it so heavily selected coinbases.

Also moves an InternalError which would've never been hit on mainnet, yet
technically isn't an invariant, to only exist when cfg(test).

coins/monero/src/rpc/mod.rs

@@ -138,7 +138,7 @@ impl<R: RpcConnection> Rpc<R> {
       )
       .map_err(|_| RpcError::InvalidNode)?,
     )
-    .map_err(|_| RpcError::InternalError("Failed to parse JSON response"))
+    .map_err(|_| RpcError::InvalidNode)
   }
 
   /// Perform a JSON-RPC call with the specified method with the provided parameters
@@ -538,12 +538,10 @@ impl<R: RpcConnection> Rpc<R> {
       .iter()
       .enumerate()
       .map(|(i, out)| {
-        Ok(Some([rpc_point(&out.key)?, rpc_point(&out.mask)?]).filter(|_| {
+        Ok(
-          match txs[i].prefix.timelock {
+          Some([rpc_point(&out.key)?, rpc_point(&out.mask)?])
-            Timelock::Block(t_height) => t_height <= height,
+            .filter(|_| Timelock::Block(height) >= txs[i].prefix.timelock),
-            _ => false,
+        )
-          }
-        }))
       })
       .collect()
   }

coins/monero/src/wallet/decoys.rs

@@ -47,9 +47,11 @@ async fn select_n<'a, R: RngCore + CryptoRng, RPC: RpcConnection>(
   count: usize,
 ) -> Result<Vec<(u64, [EdwardsPoint; 2])>, RpcError> {
   if height >= rpc.get_height().await? {
+    // TODO: Don't use InternalError for the caller's failure
     Err(RpcError::InternalError("decoys being requested from too young blocks"))?;
   }
 
+  #[cfg(test)]
   let mut iters = 0;
   let mut confirmed = Vec::with_capacity(count);
   // Retries on failure. Retries are obvious as decoys, yet should be minimal
@@ -57,10 +59,13 @@ async fn select_n<'a, R: RngCore + CryptoRng, RPC: RpcConnection>(
     let remaining = count - confirmed.len();
     let mut candidates = Vec::with_capacity(remaining);
     while candidates.len() != remaining {
+      #[cfg(test)]
+      {
         iters += 1;
-      // This is cheap and on fresh chains, thousands of rounds may be needed
+        // This is cheap and on fresh chains, a lot of rounds may be needed
-      if iters == 10000 {
+        if iters == 100 {
-        Err(RpcError::InternalError("not enough decoy candidates"))?;
+          Err(RpcError::InternalError("hit decoy selection round limit"))?;
+        }
       }
 
       // Use a gamma distribution
@@ -183,7 +188,7 @@ impl Decoys {
       used.insert(o.0);
     }
 
-    // TODO: Simply create a TX with less than the target amount
+    // TODO: Create a TX with less than the target amount, as allowed by the protocol
     if (high - MATURITY) < u64::try_from(inputs.len() * ring_len).unwrap() {
       Err(RpcError::InternalError("not enough decoy candidates"))?;
     }

coins/monero/src/wallet/scan.rs

@@ -273,7 +273,11 @@ impl<O: Clone + Zeroize> Timelocked<O> {
   /// Returns None if the Timelocks aren't comparable. Returns Some(vec![]) if none are unlocked.
   pub fn unlocked(&self, timelock: Timelock) -> Option<Vec<O>> {
     // 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())
+    if self.0 <= timelock {
+      Some(self.1.clone())
+    } else {
+      None
+    }
   }
 
   pub fn ignore_timelock(&self) -> Vec<O> {