carrot_impl: make_signable_tx_hash_from_carrot_transaction_proposal_v1 and bug fixes

src/carrot_impl/carrot_tx_builder_utils.cpp

@@ -37,6 +37,7 @@ extern "C"
 #include "crypto/crypto-ops.h"
 }
 #include "cryptonote_basic/cryptonote_format_utils.h"
+#include "ringct/rctSigs.h"
 #include "ringct/rctOps.h"
 
 //third party headers
@@ -205,7 +206,7 @@ void make_carrot_transaction_proposal_v1(const std::vector<CarrotPaymentProposal
         tx_proposal_out.key_images_sorted.push_back(selected_input.key_image);
     std::sort(tx_proposal_out.key_images_sorted.begin(),
         tx_proposal_out.key_images_sorted.end(),
-        &compare_input_key_images);
+        std::greater{}); // consensus rules dictate inputs sorted in *reverse* lexicographical order since v7
 }
 //-------------------------------------------------------------------------------------------------------------------
 void make_carrot_transaction_proposal_v1_transfer_subtractable(
@@ -474,10 +475,32 @@ void make_carrot_transaction_proposal_v1_sweep(
         tx_proposal_out);
 }
 //-------------------------------------------------------------------------------------------------------------------
+void make_signable_tx_hash_from_carrot_transaction_proposal_v1(const CarrotTransactionProposalV1 &tx_proposal,
+    const view_balance_secret_device *s_view_balance_dev,
+    const view_incoming_key_device *k_view_dev,
+    crypto::hash &signable_tx_hash_out)
+{
+    //! @TODO: there's a more efficient way to do this than constructing&serializing a whole cryptonote::transaction
+    // HW devices will need to implement this function to sign tx proposals, and most of these devices don't have a lot of memory
+
+    cryptonote::transaction pruned_tx;
+    make_pruned_transaction_from_carrot_proposal_v1(tx_proposal,
+        s_view_balance_dev,
+        k_view_dev,
+        pruned_tx);
+
+    //! @TODO: better input number calculation in get_pre_mlsag_hash. possible?
+    pruned_tx.rct_signatures.p.pseudoOuts.resize(pruned_tx.vin.size());
+
+    hw::device &hwdev = hw::get_device("default");
+    const rct::key signable_tx_hash_k = rct::get_pre_mlsag_hash(pruned_tx.rct_signatures, hwdev);
+
+    signable_tx_hash_out = rct::rct2hash(signable_tx_hash_k);
+}
+//-------------------------------------------------------------------------------------------------------------------
 void make_pruned_transaction_from_carrot_proposal_v1(const CarrotTransactionProposalV1 &tx_proposal,
     const view_balance_secret_device *s_view_balance_dev,
     const view_incoming_key_device *k_view_dev,
-    const crypto::public_key &account_spend_pubkey,
     cryptonote::transaction &pruned_tx_out)
 {
     // collect self-sends proposal cores

src/carrot_impl/carrot_tx_builder_utils.h

@@ -61,11 +61,6 @@ static inline std::size_t get_fcmppp_tx_weight(const std::size_t num_inputs,
 std::size_t get_fcmppp_coinbase_tx_weight(const std::size_t num_outputs,
     const std::size_t tx_extra_size);
 
-static inline bool compare_input_key_images(const crypto::key_image &lhs, const crypto::key_image &rhs)
-{
-    return memcmp(lhs.data, rhs.data, sizeof(crypto::key_image)) > 0;
-}
-
 void make_carrot_transaction_proposal_v1(const std::vector<CarrotPaymentProposalV1> &normal_payment_proposals,
     const std::vector<CarrotPaymentProposalVerifiableSelfSendV1> &selfsend_payment_proposals,
     const rct::xmr_amount fee_per_weight,
@@ -112,10 +107,14 @@ void make_carrot_transaction_proposal_v1_sweep(
     const crypto::public_key &account_spend_pubkey,
     CarrotTransactionProposalV1 &tx_proposal_out);
 
+void make_signable_tx_hash_from_carrot_transaction_proposal_v1(const CarrotTransactionProposalV1 &tx_proposal,
+    const view_balance_secret_device *s_view_balance_dev,
+    const view_incoming_key_device *k_view_dev,
+    crypto::hash &signable_tx_hash_out);
+
 void make_pruned_transaction_from_carrot_proposal_v1(const CarrotTransactionProposalV1 &tx_proposal,
     const view_balance_secret_device *s_view_balance_dev,
     const view_incoming_key_device *k_view_dev,
-    const crypto::public_key &account_spend_pubkey,
     cryptonote::transaction &pruned_tx_out);
 
 } //namespace carrot

src/carrot_impl/carrot_tx_format_utils.cpp

@@ -281,12 +281,14 @@ bool try_load_carrot_from_transaction_v1(const cryptonote::transaction &tx,
     std::vector<mx25519_pubkey> enote_ephemeral_pubkeys;
     if (!try_load_carrot_extra_v1(tx.extra, enote_ephemeral_pubkeys, encrypted_payment_id_out))
         return false;
-    else if (enote_ephemeral_pubkeys.size() > nouts)
+
+    const size_t n_ephemeral = enote_ephemeral_pubkeys.size();
+    if (n_ephemeral < 0 || n_ephemeral > nouts)
         return false;
 
     // collect D_e
     for (size_t i = 0; i < enotes_out.size(); ++i)
-        enotes_out[i].enote_ephemeral_pubkey = enote_ephemeral_pubkeys.at(std::min(i, nouts - 1));
+        enotes_out[i].enote_ephemeral_pubkey = enote_ephemeral_pubkeys.at(std::min(i, n_ephemeral - 1));
 
     return true;
 }

tests/unit_tests/carrot_impl.cpp

@@ -168,7 +168,6 @@ static void subtest_multi_account_transfer_over_transaction(const unittest_trans
     make_pruned_transaction_from_carrot_proposal_v1(tx_proposal,
         &ss_keys.s_view_balance_dev,
         &ss_keys.k_view_incoming_dev,
-        ss_keys.carrot_account_spend_pubkey,
         tx);
 
     // calculate acceptable fee margin between proposed amount and actual amount for subtractable outputs

tests/unit_tests/wallet_scanning.cpp

@@ -368,7 +368,6 @@ static cryptonote::transaction construct_carrot_pruned_transaction_fake_inputs(
     carrot::make_pruned_transaction_from_carrot_proposal_v1(tx_proposal,
         /*s_view_balance_dev=*/nullptr,
         &k_view_dev,
-        acc_keys.m_account_address.m_spend_public_key,
         tx);
 
     return tx;

tests/unit_tests/wallet_tx_builder.cpp

@@ -135,3 +135,90 @@ TEST(wallet_tx_builder, input_selection_basic)
     }
     ASSERT_EQ(selected_transfer_indices.size(), matched_transfer_indices.size());
 }
+//----------------------------------------------------------------------------------------------------------------------
+TEST(wallet_tx_builder, make_carrot_transaction_proposal_wallet2_transfer_1)
+{
+    cryptonote::account_base alice;
+    alice.generate();
+    cryptonote::account_base bob;
+    bob.generate();
+
+    const tools::wallet2::transfer_container transfers{
+        gen_transfer_details(),
+        gen_transfer_details()};
+
+    const rct::xmr_amount out_amount = rct::randXmrAmount(transfers.front().amount() / 2);
+
+    const std::vector<cryptonote::tx_destination_entry> dsts{
+        cryptonote::tx_destination_entry(out_amount, bob.get_keys().m_account_address, false)
+    };
+
+    const uint64_t top_block_index = std::max(transfers.front().m_block_height, transfers.back().m_block_height)
+        + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE;
+
+    const carrot::CarrotTransactionProposalV1 tx_proposal = tools::wallet::make_carrot_transaction_proposal_wallet2_transfer(
+        transfers,
+        /*subaddress_map=*/{},
+        dsts,
+        /*fee_per_weight=*/1,
+        /*extra=*/{},
+        /*subaddr_account=*/0,
+        /*subaddr_indices=*/{},
+        /*ignore_above=*/MONEY_SUPPLY,
+        /*ignore_below=*/0,
+        top_block_index,
+        alice);
+
+    std::vector<crypto::key_image> expected_key_images{
+        transfers.front().m_key_image,
+        transfers.back().m_key_image};
+    std::sort(expected_key_images.begin(),
+        expected_key_images.end(),
+        std::greater{});
+
+    // Assert basic length facts about tx proposal
+    ASSERT_EQ(2, tx_proposal.key_images_sorted.size()); // we always try 2 when available
+    EXPECT_EQ(expected_key_images, tx_proposal.key_images_sorted);
+    ASSERT_EQ(1, tx_proposal.normal_payment_proposals.size());
+    ASSERT_EQ(1, tx_proposal.selfsend_payment_proposals.size());
+    EXPECT_EQ(0, tx_proposal.extra.size());
+
+    // Assert amounts
+    EXPECT_EQ(out_amount, tx_proposal.normal_payment_proposals.front().amount);
+    EXPECT_EQ(out_amount + tx_proposal.selfsend_payment_proposals.front().proposal.amount + tx_proposal.fee,
+        transfers.front().amount() + transfers.back().amount());
+}
+//----------------------------------------------------------------------------------------------------------------------
+TEST(wallet_tx_builder, make_carrot_transaction_proposal_wallet2_sweep_1)
+{
+    cryptonote::account_base alice;
+    alice.generate();
+    cryptonote::account_base bob;
+    bob.generate();
+
+    const tools::wallet2::transfer_container transfers{gen_transfer_details()};
+
+    const carrot::CarrotTransactionProposalV1 tx_proposal = tools::wallet::make_carrot_transaction_proposal_wallet2_sweep(
+        transfers,
+        /*subaddress_map=*/{},
+        {transfers.front().m_key_image},
+        bob.get_keys().m_account_address,
+        /*is_subaddress=*/false,
+        /*n_dests=*/1,
+        /*fee_per_weight=*/1,
+        /*extra=*/{},
+        transfers.front().m_block_height + CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE,
+        alice);
+
+    // Assert basic length facts about tx proposal
+    ASSERT_EQ(1, tx_proposal.key_images_sorted.size());
+    EXPECT_EQ(transfers.front().m_key_image, tx_proposal.key_images_sorted.front());
+    ASSERT_EQ(1, tx_proposal.normal_payment_proposals.size());
+    ASSERT_EQ(1, tx_proposal.selfsend_payment_proposals.size());
+    EXPECT_EQ(0, tx_proposal.extra.size());
+
+    // Assert amounts
+    EXPECT_EQ(0, tx_proposal.selfsend_payment_proposals.front().proposal.amount);
+    EXPECT_EQ(transfers.front().amount(), tx_proposal.fee + tx_proposal.normal_payment_proposals.front().amount);
+}
+//----------------------------------------------------------------------------------------------------------------------