carrot_impl: fix payment ID scanning in scanning_tools

src/wallet/scanning_tools.cpp

@@ -81,8 +81,8 @@ static bool parse_tx_extra_for_scanning(const std::vector<std::uint8_t> &tx_extr
 
     // 4. extract nonce string
     cryptonote::tx_extra_nonce field_extra_nonce;
-    if (!cryptonote::find_tx_extra_field_by_type(tx_extra_fields, field_extra_nonce))
+    if (cryptonote::find_tx_extra_field_by_type(tx_extra_fields, field_extra_nonce))
-        field_extra_nonce.nonce.clear();
+        tx_extra_nonce_out = std::move(field_extra_nonce.nonce);
 
     return fully_parsed;
 }

src/wallet/wallet2.cpp

@@ -2541,9 +2541,10 @@ void wallet2::process_new_scanned_transaction(
       MWARNING("Found OBSOLETE AND IGNORED unencrypted payment ID: these are bad for privacy, "
         << "use subaddresses or encrypted payment IDs instead");
     }
-    else
+    else if (enote_scan_info->payment_id != payment_id)
     {
       payment_id = enote_scan_info->payment_id;
+      MDEBUG("Found payment ID " << payment_id << " in TX " << txid);
     }
 
     notify = true;

tests/functional_tests/functional_tests_rpc.py

@@ -55,7 +55,7 @@ monerod_extra = [
   ["--add-exclusive-node", "127.0.0.1:18283"],
   ["--add-exclusive-node", "127.0.0.1:18282"],
 ]
-wallet_base = [builddir + "/bin/monero-wallet-rpc", "--wallet-dir", WALLET_DIRECTORY, "--rpc-bind-port", "wallet_port", "--disable-rpc-login", "--rpc-ssl", "disabled", "--daemon-ssl", "disabled", "--log-level", "1", "--allow-mismatched-daemon-version"]
+wallet_base = [builddir + "/bin/monero-wallet-rpc", "--wallet-dir", WALLET_DIRECTORY, "--rpc-bind-port", "wallet_port", "--rpc-ssl", "disabled", "--daemon-ssl", "disabled", "--log-level", "2", "--allow-mismatched-daemon-version"]
 wallet_extra = [
   ["--daemon-port", "18180"],
   ["--daemon-port", "18180"],

tests/unit_tests/tx_construction_helpers.cpp

@@ -35,6 +35,7 @@
 #include "carrot_impl/carrot_tx_builder_utils.h"
 #include "carrot_impl/carrot_tx_format_utils.h"
 #include "crypto/generators.h"
+#include "wallet/scanning_tools.h"
 
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "unit_tests.tx_construction_helpers"
@@ -200,6 +201,7 @@ cryptonote::transaction construct_pre_carrot_tx_with_fake_inputs(
     std::vector<stripped_down_tx_source_entry_t> &&stripped_sources,
     std::vector<cryptonote::tx_destination_entry> &destinations,
     const boost::optional<cryptonote::account_public_address> &change_addr,
+    const crypto::hash &payment_id,
     const rct::xmr_amount fee,
     const uint8_t hf_version,
     crypto::secret_key &main_tx_privkey_out,
@@ -301,6 +303,36 @@ cryptonote::transaction construct_pre_carrot_tx_with_fake_inputs(
     // construct tx
     cryptonote::transaction tx;
 
+    // count integrated addresses and check <= 1
+    size_t num_integrated = 0;
+    for (const cryptonote::tx_destination_entry &dst : destinations)
+        num_integrated += dst.is_integrated;
+    CHECK_AND_ASSERT_THROW_MES(num_integrated <= 1, "max 1 integrated address allowed");
+
+    // make put unencrypted payment_id in tx_extra
+    std::vector<uint8_t> extra;
+    if (payment_id != crypto::null_hash)
+    {
+        const bool is_long_payment_id = tools::wallet::is_long_payment_id(payment_id);
+        if (!is_long_payment_id && !num_integrated)
+            MWARNING("Short payment ID provided but no destination marked as integrated");
+
+        cryptonote::tx_extra_nonce extra_nonce;
+        if (is_long_payment_id)
+        {
+            cryptonote::set_payment_id_to_tx_extra_nonce(extra_nonce.nonce, payment_id);
+        }
+        else // !is_long_payment_id
+        {
+            crypto::hash8 pid_8;
+            memcpy(&pid_8, &payment_id, sizeof(pid_8));
+            cryptonote::set_encrypted_payment_id_to_tx_extra_nonce(extra_nonce.nonce, pid_8);
+        }
+
+        CHECK_AND_ASSERT_THROW_MES(cryptonote::add_extra_nonce_to_tx_extra(extra, extra_nonce.nonce),
+            "failed to add nonce to tx_extra");
+    }
+
     fcmp_pp::ProofParams dummy_fcmp_params;
     const bool r = cryptonote::construct_tx_and_get_tx_key(
         sender_account_keys,
@@ -308,7 +340,7 @@ cryptonote::transaction construct_pre_carrot_tx_with_fake_inputs(
         sources,
         destinations,
         change_addr,
-        /*extra=*/{},
+        extra,
         tx,
         main_tx_privkey_out,
         additional_tx_privkeys_out,
@@ -337,6 +369,7 @@ cryptonote::transaction construct_pre_carrot_tx_with_fake_inputs(
         std::forward<std::vector<stripped_down_tx_source_entry_t>>(stripped_sources),
         destinations,
         change_addr,
+        crypto::null_hash,
         fee,
         hf_version,
         dummy_main_tx_privkey,

tests/unit_tests/tx_construction_helpers.h

@@ -81,6 +81,7 @@ cryptonote::transaction construct_pre_carrot_tx_with_fake_inputs(
     std::vector<stripped_down_tx_source_entry_t> &&stripped_sources,
     std::vector<cryptonote::tx_destination_entry> &destinations,
     const boost::optional<cryptonote::account_public_address> &change_addr,
+    const crypto::hash &payment_id,
     const rct::xmr_amount fee,
     const uint8_t hf_version,
     crypto::secret_key &main_tx_privkey_out,

tests/unit_tests/wallet_scanning.cpp

@@ -69,6 +69,7 @@ TEST(wallet_scanning, view_scan_as_sender_mainaddr)
             {},
             destinations,
             {},
+            crypto::null_hash,
             fee,
             hf_version,
             main_tx_privkey,
@@ -110,6 +111,155 @@ TEST(wallet_scanning, view_scan_as_sender_mainaddr)
     }
 }
 //----------------------------------------------------------------------------------------------------------------------
+TEST(wallet_scanning, view_scan_long_payment_id)
+{
+    cryptonote::account_base aether;
+    aether.generate();
+
+    cryptonote::account_base bob;
+    bob.generate();
+    const cryptonote::account_public_address bob_main_addr = bob.get_keys().m_account_address;
+    const crypto::public_key bob_main_spend_pubkey = bob_main_addr.m_spend_public_key;
+
+    const rct::xmr_amount amount = rct::randXmrAmount(10 * COIN);
+
+    const rct::xmr_amount fee = 565678;
+
+    const crypto::hash payment_id = crypto::rand<crypto::hash>();
+
+    for (uint8_t hf_version = 1; hf_version < HF_VERSION_FCMP_PLUS_PLUS; ++hf_version)
+    {
+        MDEBUG("view_scan_as_sender_mainaddr: hf_version=" << static_cast<int>(hf_version));
+
+        std::vector<cryptonote::tx_destination_entry> destinations{
+            cryptonote::tx_destination_entry(amount, bob_main_addr, false)};
+        destinations.front().is_integrated = true;
+
+        crypto::secret_key main_tx_privkey;
+        std::vector<crypto::secret_key> additional_tx_privkeys;
+        const cryptonote::transaction tx = mock::construct_pre_carrot_tx_with_fake_inputs(aether.get_keys(),
+            {{aether.get_keys().m_account_address.m_spend_public_key, {0, 0}}},
+            {},
+            destinations,
+            {},
+            payment_id,
+            fee,
+            hf_version,
+            main_tx_privkey,
+            additional_tx_privkeys);
+
+        ASSERT_EQ(0, additional_tx_privkeys.size());
+        ASSERT_GT(tx.vout.size(), 0);
+
+        // parse tx_extra and check for long payment ID field
+        std::vector<cryptonote::tx_extra_field> tx_extra_fields;
+        ASSERT_TRUE(cryptonote::parse_tx_extra(tx.extra, tx_extra_fields));
+        cryptonote::tx_extra_nonce tx_extra_nonce;
+        ASSERT_TRUE(cryptonote::find_tx_extra_field_by_type(tx_extra_fields, tx_extra_nonce));
+        crypto::hash parsed_payment_id;
+        ASSERT_TRUE(cryptonote::get_payment_id_from_tx_extra_nonce(tx_extra_nonce.nonce, parsed_payment_id));
+        ASSERT_EQ(payment_id, parsed_payment_id);
+
+        // call view_incoming_scan_transaction with no meaningful key nor subaddresses maps,
+        // just with the proper ECDH
+        std::vector<std::optional<tools::wallet::enote_view_incoming_scan_info_t>> enote_scan_infos(tx.vout.size());
+        tools::wallet::view_incoming_scan_transaction(tx,
+            bob.get_keys(),
+            {{bob_main_spend_pubkey, {}}}, // use a fake subaddress map with just the provided address in it
+            epee::to_mut_span(enote_scan_infos));
+
+        bool matched = false;
+        for (const auto &enote_scan_info : enote_scan_infos)
+        {
+            if (enote_scan_info)
+            {
+                ASSERT_FALSE(matched);
+                ASSERT_EQ(amount, enote_scan_info->amount);
+                ASSERT_EQ(bob_main_spend_pubkey, enote_scan_info->address_spend_pubkey);
+                ASSERT_EQ(payment_id, enote_scan_info->payment_id);
+                matched = true;
+            }
+        }
+        ASSERT_TRUE(matched);
+    }
+}
+//----------------------------------------------------------------------------------------------------------------------
+TEST(wallet_scanning, view_scan_short_payment_id)
+{
+    cryptonote::account_base aether;
+    aether.generate();
+
+    cryptonote::account_base bob;
+    bob.generate();
+    const cryptonote::account_public_address bob_main_addr = bob.get_keys().m_account_address;
+    const crypto::public_key bob_main_spend_pubkey = bob_main_addr.m_spend_public_key;
+
+    const rct::xmr_amount amount = rct::randXmrAmount(10 * COIN);
+
+    const rct::xmr_amount fee = 565678;
+
+    const crypto::hash8 pid_8 = crypto::rand<crypto::hash8>();
+    crypto::hash payment_id = crypto::null_hash;
+    memcpy(&payment_id, &pid_8, sizeof(pid_8));
+
+    ASSERT_FALSE(tools::wallet::is_long_payment_id(payment_id));
+
+    for (uint8_t hf_version = 1; hf_version < HF_VERSION_FCMP_PLUS_PLUS; ++hf_version)
+    {
+        MDEBUG("view_scan_as_sender_mainaddr: hf_version=" << static_cast<int>(hf_version));
+
+        std::vector<cryptonote::tx_destination_entry> destinations{
+            cryptonote::tx_destination_entry(amount, bob_main_addr, false)};
+        destinations.front().is_integrated = true;
+
+        crypto::secret_key main_tx_privkey;
+        std::vector<crypto::secret_key> additional_tx_privkeys;
+        const cryptonote::transaction tx = mock::construct_pre_carrot_tx_with_fake_inputs(aether.get_keys(),
+            {{aether.get_keys().m_account_address.m_spend_public_key, {0, 0}}},
+            {},
+            destinations,
+            {},
+            payment_id,
+            fee,
+            hf_version,
+            main_tx_privkey,
+            additional_tx_privkeys);
+
+        ASSERT_EQ(0, additional_tx_privkeys.size());
+        ASSERT_GT(tx.vout.size(), 0);
+
+        // parse tx_extra and check that a short payment ID field exists
+        std::vector<cryptonote::tx_extra_field> tx_extra_fields;
+        ASSERT_TRUE(cryptonote::parse_tx_extra(tx.extra, tx_extra_fields));
+        cryptonote::tx_extra_nonce tx_extra_nonce;
+        ASSERT_TRUE(cryptonote::find_tx_extra_field_by_type(tx_extra_fields, tx_extra_nonce));
+        crypto::hash8 pid_enc_8;
+        ASSERT_TRUE(cryptonote::get_encrypted_payment_id_from_tx_extra_nonce(tx_extra_nonce.nonce, pid_enc_8));
+
+        // call view_incoming_scan_transaction with no meaningful key nor subaddresses maps,
+        // just with the proper ECDH
+        std::vector<std::optional<tools::wallet::enote_view_incoming_scan_info_t>> enote_scan_infos(tx.vout.size());
+        tools::wallet::view_incoming_scan_transaction(tx,
+            bob.get_keys(),
+            {{bob_main_spend_pubkey, {}}}, // use a fake subaddress map with just the provided address in it
+            epee::to_mut_span(enote_scan_infos));
+
+        bool matched = false;
+        for (const auto &enote_scan_info : enote_scan_infos)
+        {
+            if (enote_scan_info)
+            {
+                ASSERT_FALSE(matched);
+                ASSERT_EQ(amount, enote_scan_info->amount);
+                ASSERT_EQ(bob_main_spend_pubkey, enote_scan_info->address_spend_pubkey);
+                ASSERT_EQ(payment_id, enote_scan_info->payment_id);
+                matched = true;
+            }
+        }
+        ASSERT_TRUE(matched);
+    }
+}
+//----------------------------------------------------------------------------------------------------------------------
 TEST(wallet_scanning, positive_smallout_main_addr_all_types_outputs)
 {
     // Test that wallet can scan and recover enotes of following type: