FCMP++: guarantee every usable tree root is unique

1) if coinbase `unlock_time >= FCMP++ fork height`, then require
at least 1 coinbase out, and start doing FCMP++ torsion validation

2) Require FCMP++ txs' `reference_block+1 >= FCMP++ fork height`.

We want unique tree roots so that a tx with `reference_block=m`
can't possibly have a valid membership proof for some other
`reference_block=n`.

src/cryptonote_core/blockchain.cpp

@@ -90,7 +90,10 @@ DISABLE_VS_WARNINGS(4267)
 namespace
 {
 //------------------------------------------------------------------
-static bool get_fcmp_tx_tree_root(const BlockchainDB *db, const cryptonote::transaction &tx, crypto::ec_point &tree_root_out)
+static bool get_fcmp_tx_tree_root(const BlockchainDB *db,
+  const HardFork *m_hardfork,
+  const cryptonote::transaction &tx,
+  crypto::ec_point &tree_root_out)
 {
   tree_root_out = crypto::ec_point{};
   if (!rct::is_rct_fcmp(tx.rct_signatures.type))
@@ -101,6 +104,11 @@ static bool get_fcmp_tx_tree_root(const BlockchainDB *db, const cryptonote::tran
   CHECK_AND_NO_ASSERT_MES_L1(tx.rct_signatures.p.reference_block < db->height(), false,
       "tx " << get_transaction_hash(tx) << " included reference block that was too high");
 
+  // Make sure the reference block is from the block immediately before the FCMP++ fork height or higher.
+  // This allows wallets to start constructing FCMP++ txs that can land in the first fork block.
+  CHECK_AND_NO_ASSERT_MES_L1((tx.rct_signatures.p.reference_block + 1) >= m_hardfork->get_earliest_ideal_height_for_version(HF_VERSION_FCMP_PLUS_PLUS), false,
+      "tx " << get_transaction_hash(tx) << " included reference block that was too low");
+
   // Get the tree root and n tree layers at provided block
   const uint8_t n_tree_layers = db->get_tree_root_at_blk_idx(tx.rct_signatures.p.reference_block, tree_root_out);
 
@@ -726,7 +734,7 @@ block Blockchain::pop_block_from_blockchain(bool keep_txs)
       else if (rct::is_rct_fcmp(tx.rct_signatures.type))
       {
         crypto::ec_point ref_tree_root{};
-        if (get_fcmp_tx_tree_root(m_db, tx, ref_tree_root))
+        if (get_fcmp_tx_tree_root(m_db, m_hardfork, tx, ref_tree_root))
         {
           valid_input_verification_id = make_input_verification_id(get_transaction_hash(tx), ref_tree_root, tx.rct_signatures.p.n_tree_layers);
         }
@@ -1501,17 +1509,24 @@ bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_bl
     base_reward = money_in_use - fee;
   }
 
-  if (version >= HF_VERSION_FCMP_PLUS_PLUS)
+  // From FCMP++ fork, we want to guarantee every output that enters the chain is a valid output with no torsion.
+  if (version >= HF_VERSION_FCMP_PLUS_PLUS || b.miner_tx.unlock_time >= m_hardfork->get_earliest_ideal_height_for_version(HF_VERSION_FCMP_PLUS_PLUS))
   {
+    // We start requiring at least 1 coinbase out and valid outs as soon as unlock_time >= FCMP++ height,
+    // so that we guarantee every usable tree root is unique, since at least 1 new out will be appended to the tree.
+    CHECK_AND_ASSERT_MES(b.miner_tx.vout.size() > 0, false, "miner transaction must have outputs");
+
     // Collect pubkeys and commitments for torsion check
     std::vector<rct::key> pubkeys_and_commitments;
     pubkeys_and_commitments.reserve(b.miner_tx.vout.size() * 2);
 
-    if (!collect_points_for_torsion_check(b.miner_tx, transparent_amount_commitments, pubkeys_and_commitments))
+    if (!collect_pubkeys_and_commitments(b.miner_tx, transparent_amount_commitments, pubkeys_and_commitments))
     {
         MERROR_VER("failed to collect pubkeys and commitments from miner tx");
         return false;
     }
+    CHECK_AND_ASSERT_MES(pubkeys_and_commitments.size() == (b.miner_tx.vout.size() * 2), false,
+      "missing collected pubkeys and commitments from miner tx");
 
     if (!rct::verPointsForTorsion(pubkeys_and_commitments))
     {
@@ -2768,6 +2783,7 @@ static bool fill(BlockchainDB *db, const crypto::hash &tx_hash, tx_blob_entry &t
 }
 //------------------------------------------------------------------
 static bool set_fcmp_tx_tree_root(const BlockchainDB *db,
+  const HardFork *m_hardfork,
   const cryptonote::transaction &tx,
   std::unordered_map<uint64_t, std::pair<crypto::ec_point, uint8_t>> &tree_root_by_block_idx_inout)
 {
@@ -2791,7 +2807,7 @@ static bool set_fcmp_tx_tree_root(const BlockchainDB *db,
 
   // Get ref block's tree root from the db
   crypto::ec_point tree_root;
-  if (!get_fcmp_tx_tree_root(db, tx, tree_root))
+  if (!get_fcmp_tx_tree_root(db, m_hardfork, tx, tree_root))
   {
     MERROR_VER("Failed to get referenced tree root");
     return false;
@@ -2802,6 +2818,7 @@ static bool set_fcmp_tx_tree_root(const BlockchainDB *db,
 }
 //------------------------------------------------------------------
 static bool batch_verify_fcmp_pp_txs(const BlockchainDB *db,
+  const HardFork *m_hardfork,
   pool_supplement &extra_block_txs,
   std::unordered_map<crypto::hash, crypto::hash> &valid_input_verification_id_by_txid_out)
 {
@@ -2812,7 +2829,7 @@ static bool batch_verify_fcmp_pp_txs(const BlockchainDB *db,
   for (const auto &extra_tx : extra_block_txs)
   {
     const cryptonote::transaction &tx = extra_tx.second.first;
-    if (!set_fcmp_tx_tree_root(db, tx, tree_root_by_block_idx))
+    if (!set_fcmp_tx_tree_root(db, m_hardfork, tx, tree_root_by_block_idx))
     {
       MERROR_VER("Failed to set FCMP tx tree root");
       return false;
@@ -3876,7 +3893,7 @@ bool Blockchain::check_tx_inputs(transaction& tx,
       *pmax_used_block_height = tx.rct_signatures.p.reference_block;
 
     // Read the db for the tree root for FCMP tx. Enforces that the ref block is in the chain
-    if (!get_fcmp_tx_tree_root(m_db, tx, ref_tree_root))
+    if (!get_fcmp_tx_tree_root(m_db, m_hardfork, tx, ref_tree_root))
     {
       // We might not be synced yet and an honest synced peer may have sent us the tx, so we make this a no-drop-offense
       tvc.m_no_drop_offense = true;
@@ -4617,7 +4634,7 @@ leave:
   if (!fast_check)
 #endif
   {
-    if (!batch_verify_fcmp_pp_txs(m_db, extra_block_txs, batched_fcmp_valid_input_verification_id_by_txid))
+    if (!batch_verify_fcmp_pp_txs(m_db, m_hardfork, extra_block_txs, batched_fcmp_valid_input_verification_id_by_txid))
     {
       MERROR_VER("Failed to batch verify FCMP++ txs");
       bvc.m_verifivation_failed = true;

src/cryptonote_core/tx_verification_utils.cpp

@@ -391,8 +391,18 @@ static bool ver_non_input_consensus_templated(TxForwardIt tx_begin,
         if (tx.version >= 2)
             rvv.push_back(&tx.rct_signatures);
 
+        // Check if we need to do the torsion check on this tx's outs
+        bool do_torsion_check = false;
+        for (const auto &out : tx.vout)
+        {
+            if (!cryptonote::output_checked_for_torsion(out.target))
+                continue;
+            do_torsion_check = true;
+            break;
+        }
+
         // Collect pubkeys and commitments for torsion check
-        if (!collect_points_for_torsion_check(tx, transparent_amount_commitments, pubkeys_and_commitments))
+        if (do_torsion_check && !collect_pubkeys_and_commitments(tx, transparent_amount_commitments, pubkeys_and_commitments))
         {
             tvc.m_verifivation_failed = true;
             return false;
@@ -478,16 +488,12 @@ static bool collect_fcmp_pp_tx_verify_input(cryptonote::transaction &tx,
 namespace cryptonote
 {
 
-bool collect_points_for_torsion_check(const transaction& tx,
+bool collect_pubkeys_and_commitments(const transaction& tx,
     const std::unordered_map<uint64_t, rct::key> &transparent_amount_commitments,
     std::vector<rct::key> &pubkeys_and_commitments_inout)
 {
     for (std::size_t i = 0; i < tx.vout.size(); ++i)
     {
-        // Don't need to collect points if we're not checking the tx outs for torsion
-        if (!cryptonote::output_checked_for_torsion(tx.vout[i].target))
-            continue;
-
         crypto::public_key output_pubkey;
         if (!cryptonote::get_output_public_key(tx.vout[i], output_pubkey))
             return false;

src/cryptonote_core/tx_verification_utils.h

@@ -46,7 +46,7 @@ struct pool_supplement;
  * @param pubkeys_and_commitments_inout the collection by ref
  * @return successful insert
  */
-bool collect_points_for_torsion_check(const transaction& tx,
+bool collect_pubkeys_and_commitments(const transaction& tx,
     const std::unordered_map<uint64_t, rct::key> &transparent_amount_commitments,
     std::vector<rct::key> &pubkeys_and_commitments_inout);