From 26fdc1d1f111c969b3e2efbabf7df9f83b81f0ab Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Mon, 3 Jul 2023 19:04:33 -0400
Subject: [PATCH] Correct handling of commitment masks when scanning

---
 coins/monero/src/wallet/mod.rs  | 26 +++++++++++++++++---------
 coins/monero/src/wallet/scan.rs |  5 ++---
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs
index 6953a3d5..b0e8065d 100644
--- a/coins/monero/src/wallet/mod.rs
+++ b/coins/monero/src/wallet/mod.rs
@@ -93,28 +93,36 @@ pub(crate) fn amount_encryption(amount: u64, key: Scalar) -> [u8; 8] {
   (amount ^ u64::from_le_bytes(hash(&amount_mask)[.. 8].try_into().unwrap())).to_le_bytes()
 }
 
-fn amount_decryption(amount: &EncryptedAmount, key: Scalar) -> u64 {
+// TODO: Move this under EncryptedAmount?
+fn amount_decryption(amount: &EncryptedAmount, key: Scalar) -> (Scalar, u64) {
   match amount {
-    EncryptedAmount::Original { mask: _, amount } => {
+    EncryptedAmount::Original { mask, amount } => {
       #[cfg(feature = "experimental")]
       {
-        let shared_sec = hash(&hash(key.as_bytes()));
+        let mask_shared_sec = hash(key.as_bytes());
+        let mask =
+          Scalar::from_bytes_mod_order(*mask) - Scalar::from_bytes_mod_order(mask_shared_sec);
+
+        let amount_shared_sec = hash(&mask_shared_sec);
         let amount_scalar =
-          Scalar::from_bytes_mod_order(*amount) - Scalar::from_bytes_mod_order(shared_sec);
+          Scalar::from_bytes_mod_order(*amount) - Scalar::from_bytes_mod_order(amount_shared_sec);
         // d2b from rctTypes.cpp
-        let amount_significant_bytes = amount_scalar.to_bytes()[0 .. 8].try_into().unwrap();
-        u64::from_le_bytes(amount_significant_bytes)
+        let amount = u64::from_le_bytes(amount_scalar.to_bytes()[0 .. 8].try_into().unwrap());
+
+        (mask, amount)
       }
 
       #[cfg(not(feature = "experimental"))]
       {
+        let _ = mask;
         let _ = amount;
         todo!("decrypting a legacy monero transaction's amount")
       }
     }
-    EncryptedAmount::Compact { amount } => {
-      u64::from_le_bytes(amount_encryption(u64::from_le_bytes(*amount), key))
-    }
+    EncryptedAmount::Compact { amount } => (
+      commitment_mask(key),
+      u64::from_le_bytes(amount_encryption(u64::from_le_bytes(*amount), key)),
+    ),
   }
 }
 
diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs
index 5beef897..1a29d21e 100644
--- a/coins/monero/src/wallet/scan.rs
+++ b/coins/monero/src/wallet/scan.rs
@@ -16,7 +16,6 @@ use crate::{
   rpc::{RpcError, RpcConnection, Rpc},
   wallet::{
     PaymentId, Extra, address::SubaddressIndex, Scanner, uniqueness, shared_key, amount_decryption,
-    commitment_mask,
   },
 };
 
@@ -379,7 +378,7 @@ impl Scanner {
           commitment.amount = amount;
         // Regular transaction
         } else {
-          let amount = match tx.rct_signatures.base.encrypted_amounts.get(o) {
+          let (mask, amount) = match tx.rct_signatures.base.encrypted_amounts.get(o) {
             Some(amount) => amount_decryption(amount, shared_key),
             // This should never happen, yet it may be possible with miner transactions?
             // Using get just decreases the possibility of a panic and lets us move on in that case
@@ -387,7 +386,7 @@ impl Scanner {
           };
 
           // Rebuild the commitment to verify it
-          commitment = Commitment::new(commitment_mask(shared_key), amount);
+          commitment = Commitment::new(mask, amount);
           // If this is a malicious commitment, move to the next output
           // Any other R value will calculate to a different spend key and are therefore ignorable
           if Some(&commitment.calculate()) != tx.rct_signatures.base.commitments.get(o) {