Have a public monero-rpc type be properly formatted

It was public as the raw RPC response. It's more polite to handle the
formatting in the RPC, and allows us to return a better structure.

.github/actions/test-dependencies/action.yml

@@ -10,7 +10,7 @@ inputs:
   bitcoin-version:
     description: "Bitcoin version to download and run as a regtest node"
     required: false
-    default: "27.0"
+    default: "27.1"
 
 runs:
   using: "composite"

Cargo.lock

@@ -4781,7 +4781,6 @@ dependencies = [
  "monero-primitives",
  "rand_core",
  "std-shims",
- "subtle",
  "thiserror",
  "zeroize",
 ]

coins/monero/ringct/bulletproofs/Cargo.toml

@@ -22,7 +22,6 @@ thiserror = { version = "1", default-features = false, optional = true }
 
 rand_core = { version = "0.6", default-features = false }
 zeroize = { version = "^1.5", default-features = false, features = ["zeroize_derive"] }
-subtle = { version = "^2.4", default-features = false }
 
 # Cryptographic dependencies
 curve25519-dalek = { version = "4", default-features = false, features = ["alloc", "zeroize"] }
@@ -47,7 +46,6 @@ std = [
 
   "rand_core/std",
   "zeroize/std",
-  "subtle/std",
 
   "monero-io/std",
   "monero-generators/std",

coins/monero/ringct/bulletproofs/src/original/mod.rs

@@ -167,19 +167,18 @@ impl<'a> AggregateRangeStatement<'a> {
 
     let (y, z) = Self::transcript_A_S(transcript, A, S);
     transcript = z;
+    let z = ScalarVector::powers(z, 3 + padded_pow_of_2);
 
     let twos = ScalarVector::powers(Scalar::from(2u8), COMMITMENT_BITS);
 
-    let l = [aL - z, sL];
+    let l = [aL - z[1], sL];
     let y_pow_n = ScalarVector::powers(y, aR.len());
-    let mut r = [((aR + z) * &y_pow_n), sR * &y_pow_n];
+    let mut r = [((aR + z[1]) * &y_pow_n), sR * &y_pow_n];
     {
-      let mut z_current = z * z;
       for j in 0 .. padded_pow_of_2 {
         for i in 0 .. COMMITMENT_BITS {
-          r[0].0[(j * COMMITMENT_BITS) + i] += z_current * twos[i];
+          r[0].0[(j * COMMITMENT_BITS) + i] += z[2 + j] * twos[i];
         }
-        z_current *= z;
       }
     }
     let t1 = (l[0].clone().inner_product(&r[1])) + (r[0].clone().inner_product(&l[1]));
@@ -216,10 +215,8 @@ impl<'a> AggregateRangeStatement<'a> {
     let t_hat = l.clone().inner_product(&r);
     let mut tau_x = ((tau_2 * x) + tau_1) * x;
     {
-      let mut z_current = z * z;
-      for commitment in &witness.commitments {
-        tau_x += z_current * commitment.mask;
-        z_current *= z;
+      for (i, commitment) in witness.commitments.iter().enumerate() {
+        tau_x += z[2 + i] * commitment.mask;
       }
     }
     let mu = alpha + (rho * x);
@@ -268,6 +265,7 @@ impl<'a> AggregateRangeStatement<'a> {
 
     let (y, z) = Self::transcript_A_S(transcript, proof.A, proof.S);
     transcript = z;
+    let z = ScalarVector::powers(z, 3 + padded_pow_of_2);
     transcript = Self::transcript_T12(transcript, proof.T1, proof.T2);
     let x = transcript;
     transcript = Self::transcript_tau_x_mu_t_hat(transcript, proof.tau_x, proof.mu, proof.t_hat);
@@ -293,18 +291,14 @@ impl<'a> AggregateRangeStatement<'a> {
       // These will now sum to 0 if equal
       let weight = -weight;
 
-      verifier.0.h += weight * (z - (z * z)) * y_pow_n.sum();
+      verifier.0.h += weight * (z[1] - (z[2])) * y_pow_n.sum();
 
-      let mut z_current = z * z;
-      for commitment in &commitments {
-        verifier.0.other.push((weight * z_current, *commitment));
-        z_current *= z;
+      for (i, commitment) in commitments.iter().enumerate() {
+        verifier.0.other.push((weight * z[2 + i], *commitment));
       }
 
-      let mut z_current = z * z * z;
-      for _ in 0 .. padded_pow_of_2 {
-        verifier.0.h -= weight * z_current * twos.clone().sum();
-        z_current *= z;
+      for i in 0 .. padded_pow_of_2 {
+        verifier.0.h -= weight * z[3 + i] * twos.clone().sum();
       }
       verifier.0.other.push((weight * x, proof.T1));
       verifier.0.other.push((weight * (x * x), proof.T2));
@@ -315,22 +309,23 @@ impl<'a> AggregateRangeStatement<'a> {
     // 66
     verifier.0.other.push((ip_weight, proof.A));
     verifier.0.other.push((ip_weight * x, proof.S));
-    // TODO: g_sum
+    // We can replace these with a g_sum, h_sum scalar in the batch verifier
+    // It'd trade `2 * ip_rows` scalar additions (per proof) for one scalar addition and an
+    // additional term in the MSM
+    let ip_z = ip_weight * z[1];
     for i in 0 .. ip_rows {
-      verifier.0.g_bold[i] += ip_weight * -z;
+      verifier.0.h_bold[i] += ip_z;
     }
-    // TODO: h_sum
+    let neg_ip_z = -ip_z;
     for i in 0 .. ip_rows {
-      verifier.0.h_bold[i] += ip_weight * z;
+      verifier.0.g_bold[i] += neg_ip_z;
     }
     {
-      let mut z_current = z * z;
       for j in 0 .. padded_pow_of_2 {
         for i in 0 .. COMMITMENT_BITS {
           let full_i = (j * COMMITMENT_BITS) + i;
-          verifier.0.h_bold[full_i] += ip_weight * y_inv_pow_n[full_i] * z_current * twos[i];
+          verifier.0.h_bold[full_i] += ip_weight * y_inv_pow_n[full_i] * z[2 + j] * twos[i];
         }
-        z_current *= z;
       }
     }
     verifier.0.h += ip_weight * x_ip * proof.t_hat;

coins/monero/ringct/bulletproofs/src/plus/weighted_inner_product.rs

@@ -166,6 +166,17 @@ impl WipStatement {
     let mut g_bold = PointVector(g_bold);
     let mut h_bold = PointVector(h_bold);
 
+    let mut y_inv = {
+      let mut i = 1;
+      let mut to_invert = vec![];
+      while i < g_bold.len() {
+        to_invert.push(y[i - 1]);
+        i *= 2;
+      }
+      Scalar::batch_invert(&mut to_invert);
+      to_invert
+    };
+
     // Check P has the expected relationship
     #[cfg(debug_assertions)]
     {
@@ -219,8 +230,7 @@ impl WipStatement {
       let c_l = a1.clone().weighted_inner_product(&b2, &y);
       let c_r = (a2.clone() * y_n_hat).weighted_inner_product(&b1, &y);
 
-      // TODO: Calculate these with a batch inversion
-      let y_inv_n_hat = y_n_hat.invert();
+      let y_inv_n_hat = y_inv.pop().unwrap();
 
       let mut L_terms = (a1.clone() * y_inv_n_hat)
         .0

coins/monero/rpc/simple-request/tests/tests.rs

@@ -102,3 +102,26 @@ async fn test_decoy_rpc() {
     rpc.get_output_distribution(1 .. 0).await.unwrap_err();
   }
 }
+
+// This test passes yet requires a mainnet node, which we don't have reliable access to in CI.
+/*
+#[tokio::test]
+async fn test_zero_out_tx_o_indexes() {
+  use monero_rpc::Rpc;
+
+  let rpc = SimpleRequestRpc::new("https://node.sethforprivacy.com".to_string()).await.unwrap();
+
+  assert_eq!(
+    rpc
+      .get_o_indexes(
+        hex::decode("17ce4c8feeb82a6d6adaa8a89724b32bf4456f6909c7f84c8ce3ee9ebba19163")
+          .unwrap()
+          .try_into()
+          .unwrap()
+      )
+      .await
+      .unwrap(),
+    Vec::<u64>::new()
+  );
+}
+*/

coins/monero/rpc/src/lib.rs

@@ -19,7 +19,7 @@ use zeroize::Zeroize;
 
 use async_trait::async_trait;
 
-use curve25519_dalek::edwards::EdwardsPoint;
+use curve25519_dalek::edwards::{CompressedEdwardsY, EdwardsPoint};
 
 use serde::{Serialize, Deserialize, de::DeserializeOwned};
 use serde_json::{Value, json};
@@ -28,6 +28,7 @@ use monero_serai::{
   io::*,
   transaction::{Input, Timelock, Transaction},
   block::Block,
+  DEFAULT_LOCK_WINDOW,
 };
 use monero_address::Address;
 
@@ -188,19 +189,24 @@ struct TransactionsResponse {
   txs: Vec<TransactionResponse>,
 }
 
-/// The response to an output query.
-#[derive(Debug, Deserialize)]
-pub struct OutputResponse {
-  /// The height of the block this output was added to the chain in.
+/// The response to an query for the information of a RingCT output.
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub struct OutputInformation {
+  /// The block number of the block this output was added to the chain in.
+  ///
+  /// This is equivalent to he height of the blockchain at the time the block was added.
   pub height: usize,
   /// If the output is unlocked, per the node's local view.
   pub unlocked: bool,
   /// The output's key.
-  pub key: String,
+  ///
+  /// This is a CompressedEdwardsY, not an EdwardsPoint, as it may be invalid. CompressedEdwardsY
+  /// only asserts validity on decompression and allows representing compressed types.
+  pub key: CompressedEdwardsY,
   /// The output's commitment.
-  pub mask: String,
+  pub commitment: EdwardsPoint,
   /// The transaction which created this output.
-  pub txid: String,
+  pub transaction: [u8; 32],
 }
 
 fn rpc_hex(value: &str) -> Result<Vec<u8>, RpcError> {
@@ -497,7 +503,6 @@ pub trait Rpc: Sync + Clone + Debug {
   /// This may be manipulated to unsafe levels and MUST be sanity checked.
   ///
   /// This MUST NOT be expected to be deterministic in any way.
-  // TODO: Take a sanity check argument
   async fn get_fee_rate(&self, priority: FeePriority) -> Result<FeeRate, RpcError> {
     #[derive(Debug, Deserialize)]
     struct FeeResponse {
@@ -788,7 +793,6 @@ pub trait Rpc: Sync + Clone + Debug {
         }
 
         // If the Vec was empty, it would've been omitted, hence the unwrap_or
-        // TODO: Test against a 0-output TX, such as the ones found in block 202612
         Ok(res.unwrap_or(vec![]))
       };
 
@@ -821,7 +825,7 @@ pub trait DecoyRpc: Sync + Clone + Debug {
   ) -> Result<Vec<u64>, RpcError>;
 
   /// Get the specified outputs from the RingCT (zero-amount) pool.
-  async fn get_outs(&self, indexes: &[u64]) -> Result<Vec<OutputResponse>, RpcError>;
+  async fn get_outs(&self, indexes: &[u64]) -> Result<Vec<OutputInformation>, RpcError>;
 
   /// Get the specified outputs from the RingCT (zero-amount) pool, but only return them if their
   /// timelock has been satisfied.
@@ -829,16 +833,16 @@ pub trait DecoyRpc: Sync + Clone + Debug {
   /// The timelock being satisfied is distinct from being free of the 10-block lock applied to all
   /// Monero transactions.
   ///
-  /// The node is trusted for if the output is unlocked unless `fingerprintable_canonical` is set
-  /// to true. If `fingerprintable_canonical` is set to true, the node's local view isn't used, yet
-  /// the transaction's timelock is checked to be unlocked at the specified `height`. This offers a
-  /// canonical decoy selection, yet is fingerprintable as time-based timelocks aren't evaluated
-  /// (and considered locked, preventing their selection).
+  /// The node is trusted for if the output is unlocked unless `fingerprintable_deterministic` is
+  /// set to true. If `fingerprintable_deterministic` is set to true, the node's local view isn't
+  /// used, yet the transaction's timelock is checked to be unlocked at the specified `height`.
+  /// This offers a deterministic decoy selection, yet is fingerprintable as time-based timelocks
+  /// aren't evaluated (and considered locked, preventing their selection).
   async fn get_unlocked_outputs(
     &self,
     indexes: &[u64],
     height: usize,
-    fingerprintable_canonical: bool,
+    fingerprintable_deterministic: bool,
   ) -> Result<Vec<Option<[EdwardsPoint; 2]>>, RpcError>;
 }
 
@@ -941,7 +945,16 @@ impl<R: Rpc> DecoyRpc for R {
     Ok(distribution)
   }
 
-  async fn get_outs(&self, indexes: &[u64]) -> Result<Vec<OutputResponse>, RpcError> {
+  async fn get_outs(&self, indexes: &[u64]) -> Result<Vec<OutputInformation>, RpcError> {
+    #[derive(Debug, Deserialize)]
+    struct OutputResponse {
+      height: usize,
+      unlocked: bool,
+      key: String,
+      mask: String,
+      txid: String,
+    }
+
     #[derive(Debug, Deserialize)]
     struct OutsResponse {
       status: String,
@@ -965,24 +978,38 @@ impl<R: Rpc> DecoyRpc for R {
       Err(RpcError::InvalidNode("bad response to get_outs".to_string()))?;
     }
 
-    Ok(res.outs)
+    Ok(
+      res
+        .outs
+        .into_iter()
+        .map(|output| {
+          Ok(OutputInformation {
+            height: output.height,
+            unlocked: output.unlocked,
+            key: CompressedEdwardsY(
+              rpc_hex(&output.key)?
+                .try_into()
+                .map_err(|_| RpcError::InvalidNode("output key wasn't 32 bytes".to_string()))?,
+            ),
+            commitment: rpc_point(&output.mask)?,
+            transaction: hash_hex(&output.txid)?,
+          })
+        })
+        .collect::<Result<Vec<_>, RpcError>>()?,
+    )
   }
 
   async fn get_unlocked_outputs(
     &self,
     indexes: &[u64],
     height: usize,
-    fingerprintable_canonical: bool,
+    fingerprintable_deterministic: bool,
   ) -> Result<Vec<Option<[EdwardsPoint; 2]>>, RpcError> {
-    let outs: Vec<OutputResponse> = self.get_outs(indexes).await?;
+    let outs = self.get_outs(indexes).await?;
 
-    // Only need to fetch txs to do canonical check on timelock
-    let txs = if fingerprintable_canonical {
-      self
-        .get_transactions(
-          &outs.iter().map(|out| hash_hex(&out.txid)).collect::<Result<Vec<_>, _>>()?,
-        )
-        .await?
+    // Only need to fetch txs to do deterministic check on timelock
+    let txs = if fingerprintable_deterministic {
+      self.get_transactions(&outs.iter().map(|out| out.transaction).collect::<Vec<_>>()).await?
     } else {
       vec![]
     };
@@ -996,19 +1023,20 @@ impl<R: Rpc> DecoyRpc for R {
         // decoy
         // Only valid keys can be used in CLSAG proofs, hence the need for re-selection, yet
         // invalid keys may honestly exist on the blockchain
-        // Only a recent hard fork checked output keys were valid points
-        let Some(key) = decompress_point(
-          rpc_hex(&out.key)?
-            .try_into()
-            .map_err(|_| RpcError::InvalidNode("non-32-byte point".to_string()))?,
-        ) else {
+        let Some(key) = out.key.decompress() else {
           return Ok(None);
         };
-        Ok(Some([key, rpc_point(&out.mask)?]).filter(|_| {
-          if fingerprintable_canonical {
-            // TODO: Are timelock blocks by height or number?
-            // TODO: This doesn't check the default timelock has been passed
-            Timelock::Block(height) >= txs[i].prefix().additional_timelock
+        Ok(Some([key, out.commitment]).filter(|_| {
+          if fingerprintable_deterministic {
+            // https://github.com/monero-project/monero/blob
+            //   /cc73fe71162d564ffda8e549b79a350bca53c454/src/cryptonote_core/blockchain.cpp#L90
+            const ACCEPTED_TIMELOCK_DELTA: usize = 1;
+
+            // https://github.com/monero-project/monero/blob
+            //   /cc73fe71162d564ffda8e549b79a350bca53c454/src/cryptonote_core/blockchain.cpp#L3836
+            ((out.height + DEFAULT_LOCK_WINDOW) <= height) &&
+              (Timelock::Block(height - 1 + ACCEPTED_TIMELOCK_DELTA) >=
+                txs[i].prefix().additional_timelock)
           } else {
             out.unlocked
           }

coins/monero/wallet/address/src/lib.rs

@@ -65,7 +65,6 @@ impl AddressType {
   }
 
   /// The payment ID within this address.
-  // TODO: wallet-core PaymentId? TX extra crate imported here?
   pub fn payment_id(&self) -> Option<[u8; 8]> {
     if let AddressType::LegacyIntegrated(id) = self {
       Some(*id)
@@ -164,15 +163,19 @@ impl AddressBytes {
   }
 }
 
-// TODO: Cite origin
+// https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+//   /src/cryptonote_config.h#L216-L225
+// https://gist.github.com/kayabaNerve/01c50bbc35441e0bbdcee63a9d823789 for featured
 const MONERO_MAINNET_BYTES: AddressBytes = match AddressBytes::new(18, 19, 42, 70) {
   Some(bytes) => bytes,
   None => panic!("mainnet byte constants conflicted"),
 };
+// https://github.com/monero-project/monero/blob/master/src/cryptonote_config.h#L277-L281
 const MONERO_STAGENET_BYTES: AddressBytes = match AddressBytes::new(24, 25, 36, 86) {
   Some(bytes) => bytes,
   None => panic!("stagenet byte constants conflicted"),
 };
+// https://github.com/monero-project/monero/blob/master/src/cryptonote_config.h#L262-L266
 const MONERO_TESTNET_BYTES: AddressBytes = match AddressBytes::new(53, 54, 63, 111) {
   Some(bytes) => bytes,
   None => panic!("testnet byte constants conflicted"),

coins/monero/wallet/src/decoys.rs

@@ -28,7 +28,7 @@ async fn select_n(
   height: usize,
   real_output: u64,
   ring_len: usize,
-  fingerprintable_canonical: bool,
+  fingerprintable_deterministic: bool,
 ) -> Result<Vec<(u64, [EdwardsPoint; 2])>, RpcError> {
   if height < DEFAULT_LOCK_WINDOW {
     Err(RpcError::InternalError("not enough blocks to select decoys".to_string()))?;
@@ -141,7 +141,7 @@ async fn select_n(
     };
 
     for (i, output) in rpc
-      .get_unlocked_outputs(&candidates, height, fingerprintable_canonical)
+      .get_unlocked_outputs(&candidates, height, fingerprintable_deterministic)
       .await?
       .iter_mut()
       .enumerate()
@@ -172,8 +172,7 @@ async fn select_decoys<R: RngCore + CryptoRng>(
   ring_len: usize,
   height: usize,
   input: &WalletOutput,
-  // TODO: Decide "canonical" or "deterministic" (updating RPC terminology accordingly)
-  fingerprintable_canonical: bool,
+  fingerprintable_deterministic: bool,
 ) -> Result<Decoys, RpcError> {
   // Select all decoys for this transaction, assuming we generate a sane transaction
   // We should almost never naturally generate an insane transaction, hence why this doesn't
@@ -184,7 +183,7 @@ async fn select_decoys<R: RngCore + CryptoRng>(
     height,
     input.relative_id.index_on_blockchain,
     ring_len,
-    fingerprintable_canonical,
+    fingerprintable_deterministic,
   )
   .await?;
 

coins/monero/wallet/src/extra.rs

@@ -204,7 +204,10 @@ impl Extra {
   ///
   /// This returns all keys specified with `PublicKey` and the first set of keys specified with
   /// `PublicKeys`, so long as they're well-formed.
-  // TODO: Cite this
+  // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c45
+  //   /src/wallet/wallet2.cpp#L2290-L2300
+  // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+  // /src/wallet/wallet2.cpp#L2337-L2340
   pub fn keys(&self) -> Option<(Vec<EdwardsPoint>, Option<Vec<EdwardsPoint>>)> {
     let mut keys = vec![];
     let mut additional = None;
@@ -255,18 +258,24 @@ impl Extra {
 
   pub(crate) fn new(key: EdwardsPoint, additional: Vec<EdwardsPoint>) -> Extra {
     let mut res = Extra(Vec::with_capacity(3));
-    res.push(ExtraField::PublicKey(key));
+    // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+    //   /src/cryptonote_basic/cryptonote_format_utils.cpp#L627-L633
+    // We only support pushing nonces which come after these in the sort order
+    res.0.push(ExtraField::PublicKey(key));
     if !additional.is_empty() {
-      res.push(ExtraField::PublicKeys(additional));
+      res.0.push(ExtraField::PublicKeys(additional));
     }
     res
   }
 
-  pub(crate) fn push(&mut self, field: ExtraField) {
-    self.0.push(field);
+  pub(crate) fn push_nonce(&mut self, nonce: Vec<u8>) {
+    self.0.push(ExtraField::Nonce(nonce));
   }
 
   /// Write the Extra.
+  ///
+  /// This is not of deterministic length nor length-prefixed. It should only be written to a
+  /// buffer which will be delimited.
   pub fn write<W: Write>(&self, w: &mut W) -> io::Result<()> {
     for field in &self.0 {
       field.write(w)?;
@@ -281,17 +290,19 @@ impl Extra {
     buf
   }
 
-  // TODO: Is this supposed to silently drop trailing gibberish?
   /// Read an `Extra`.
+  ///
+  /// This is not of deterministic length nor length-prefixed. It should only be read from a buffer
+  /// already delimited.
   #[allow(clippy::unnecessary_wraps)]
   pub fn read<R: BufRead>(r: &mut R) -> io::Result<Extra> {
     let mut res = Extra(vec![]);
-    let mut field;
-    while {
-      field = ExtraField::read(r);
-      field.is_ok()
-    } {
-      res.0.push(field.unwrap());
+    // Extra reads until EOF
+    // We take a BufRead so we can detect when the buffer is empty
+    // `fill_buf` returns the current buffer, filled if empty, only empty if the reader is
+    // exhausted
+    while !r.fill_buf()?.is_empty() {
+      res.0.push(ExtraField::read(r)?);
     }
     Ok(res)
   }

coins/monero/wallet/src/lib.rs

@@ -102,7 +102,6 @@ impl SharedKeyDerivations {
   }
 
   // H(8Ra || 0x8d)
-  // TODO: Make this itself a PaymentId
   #[allow(clippy::needless_pass_by_value)]
   fn payment_id_xor(ecdh: Zeroizing<EdwardsPoint>) -> [u8; 8] {
     // 8Ra

coins/monero/wallet/src/scan.rs

@@ -135,9 +135,8 @@ impl InternalScanner {
       // This will be None if there's no additional keys, Some(None) if there's additional keys
       // yet not one for this output (which is non-standard), and Some(Some(_)) if there's an
       // additional key for this output
-      // https://github.com/monero-project/monero/
-      //   blob/04a1e2875d6e35e27bb21497988a6c822d319c28/
-      //   src/cryptonote_basic/cryptonote_format_utils.cpp#L1062
+      // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+      //   /src/cryptonote_basic/cryptonote_format_utils.cpp#L1060-L1070
       let additional = additional.as_ref().map(|additional| additional.get(o));
 
       #[allow(clippy::manual_let_else)]

coins/monero/wallet/src/send/tx.rs

@@ -13,7 +13,7 @@ use crate::{
     RctProofs,
   },
   transaction::{Input, Output, Timelock, TransactionPrefix, Transaction},
-  extra::{ARBITRARY_DATA_MARKER, PaymentId, ExtraField, Extra},
+  extra::{ARBITRARY_DATA_MARKER, PaymentId, Extra},
   send::{InternalPayment, SignableTransaction, SignableTransactionWithKeyImages},
 };
 
@@ -74,7 +74,7 @@ impl SignableTransaction {
       let id = (u64::from_le_bytes(id) ^ u64::from_le_bytes(*id_xor)).to_le_bytes();
       let mut id_vec = Vec::with_capacity(1 + 8);
       PaymentId::Encrypted(id).write(&mut id_vec).unwrap();
-      extra.push(ExtraField::Nonce(id_vec));
+      extra.push_nonce(id_vec);
     } else {
       // If there's no payment ID, we push a dummy (as wallet2 does) if there's only one payment
       if (self.payments.len() == 2) &&
@@ -89,7 +89,7 @@ impl SignableTransaction {
         let mut id_vec = Vec::with_capacity(1 + 8);
         // The dummy payment ID is [0; 8], which when xor'd with the mask, is just the mask
         PaymentId::Encrypted(*payment_id_xor).write(&mut id_vec).unwrap();
-        extra.push(ExtraField::Nonce(id_vec));
+        extra.push_nonce(id_vec);
       }
     }
 
@@ -97,7 +97,7 @@ impl SignableTransaction {
     for part in &self.data {
       let mut arb = vec![ARBITRARY_DATA_MARKER];
       arb.extend(part);
-      extra.push(ExtraField::Nonce(arb));
+      extra.push_nonce(arb);
     }
 
     let mut serialized = Vec::with_capacity(32 * amount_of_keys);

coins/monero/wallet/src/send/tx_keys.rs

@@ -186,7 +186,8 @@ impl SignableTransaction {
       let mut additional_keys_pub = vec![];
       for (additional_key, payment) in additional_keys.into_iter().zip(&self.payments) {
         let addr = payment.address();
-        // TODO: Double check this against wallet2
+        // https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454
+        //   /src/device/device_default.cpp#L308-L312
         if addr.is_subaddress() {
           additional_keys_pub.push(additional_key.deref() * addr.spend());
         } else {

coins/monero/wallet/src/tests/extra.rs

@@ -105,15 +105,13 @@ fn padding_only_max_size() {
 #[test]
 fn padding_only_exceed_max_size() {
   let buf: Vec<u8> = vec![0; MAX_TX_EXTRA_PADDING_COUNT + 1];
-  let extra = Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap();
-  assert!(extra.0.is_empty());
+  Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap_err();
 }
 
 #[test]
 fn invalid_padding_only() {
   let buf: Vec<u8> = vec![0, 42];
-  let extra = Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap();
-  assert!(extra.0.is_empty());
+  Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap_err();
 }
 
 #[test]
@@ -137,8 +135,7 @@ fn extra_nonce_only_wrong_size() {
   let mut buf: Vec<u8> = vec![0; 20];
   buf[0] = 2;
   buf[1] = 255;
-  let extra = Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap();
-  assert!(extra.0.is_empty());
+  Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap_err();
 }
 
 #[test]
@@ -158,8 +155,7 @@ fn pub_key_and_padding() {
 fn pub_key_and_invalid_padding() {
   let mut buf: Vec<u8> = PUB_KEY_BYTES.to_vec();
   buf.extend([0, 1]);
-  let extra = Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap();
-  assert_eq!(extra.0, vec![ExtraField::PublicKey(pub_key())]);
+  Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap_err();
 }
 
 #[test]
@@ -185,8 +181,7 @@ fn extra_mysterious_minergate_only_wrong_size() {
   let mut buf: Vec<u8> = vec![0; 20];
   buf[0] = 222;
   buf[1] = 255;
-  let extra = Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap();
-  assert!(extra.0.is_empty());
+  Extra::read::<&[u8]>(&mut buf.as_ref()).unwrap_err();
 }
 
 #[test]

coins/monero/wallet/tests/decoys.rs

@@ -2,7 +2,7 @@ use monero_simple_request_rpc::SimpleRequestRpc;
 use monero_wallet::{
   DEFAULT_LOCK_WINDOW,
   transaction::Transaction,
-  rpc::{OutputResponse, Rpc, DecoyRpc},
+  rpc::{Rpc, DecoyRpc},
   WalletOutput,
 };
 
@@ -54,8 +54,7 @@ test!(
       let most_recent_o_index = rpc.get_o_indexes(tx.hash()).await.unwrap().pop().unwrap();
 
       // Make sure output from tx1 is in the block in which it unlocks
-      let out_tx1: OutputResponse =
-        rpc.get_outs(&[most_recent_o_index]).await.unwrap().swap_remove(0);
+      let out_tx1 = rpc.get_outs(&[most_recent_o_index]).await.unwrap().swap_remove(0);
       assert_eq!(out_tx1.height, height - DEFAULT_LOCK_WINDOW);
       assert!(out_tx1.unlocked);
 
@@ -133,8 +132,7 @@ test!(
       let most_recent_o_index = rpc.get_o_indexes(tx.hash()).await.unwrap().pop().unwrap();
 
       // Make sure output from tx1 is in the block in which it unlocks
-      let out_tx1: OutputResponse =
-        rpc.get_outs(&[most_recent_o_index]).await.unwrap().swap_remove(0);
+      let out_tx1 = rpc.get_outs(&[most_recent_o_index]).await.unwrap().swap_remove(0);
       assert_eq!(out_tx1.height, height - DEFAULT_LOCK_WINDOW);
       assert!(out_tx1.unlocked);
 

orchestration/src/coins/bitcoin.rs

@@ -7,7 +7,7 @@ pub fn bitcoin(orchestration_path: &Path, network: Network) {
   const DOWNLOAD_BITCOIN: &str = r#"
 FROM alpine:latest as bitcoin
 
-ENV BITCOIN_VERSION=27.0
+ENV BITCOIN_VERSION=27.1
 
 RUN apk --no-cache add git gnupg