fix merge mining with more than one merge mined chain

reported by sech1

7 files changed, 49 insertions, 25 deletions

diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp
index 9aeee3d55..c86e7d848 100644
--- a/src/cryptonote_basic/cryptonote_format_utils.cpp
+++ b/src/cryptonote_basic/cryptonote_format_utils.cpp
@@ -739,22 +739,28 @@ namespace cryptonote
     return true;
   }
   //---------------------------------------------------------------
-  bool add_mm_merkle_root_to_tx_extra(std::vector<uint8_t>& tx_extra, const crypto::hash& mm_merkle_root, size_t mm_merkle_tree_depth)
+  bool add_mm_merkle_root_to_tx_extra(std::vector<uint8_t>& tx_extra, const crypto::hash& mm_merkle_root, uint64_t mm_merkle_tree_depth)
   {
-    CHECK_AND_ASSERT_MES(mm_merkle_tree_depth < 32, false, "merge mining merkle tree depth should be less than 32");
     size_t start_pos = tx_extra.size();
-    tx_extra.resize(tx_extra.size() + 3 + 32);
+    static const size_t max_varint_size = 16;
+    tx_extra.resize(tx_extra.size() + 2 + 32 + max_varint_size);
     //write tag
     tx_extra[start_pos] = TX_EXTRA_MERGE_MINING_TAG;
     //write data size
     ++start_pos;
-    tx_extra[start_pos] = 33;
-    //write depth varint (always one byte here)
+    const off_t len_bytes = start_pos;
+    // one byte placeholder for length since we'll only know the size later after writing a varint
+    tx_extra[start_pos] = 0;
+    //write depth varint
     ++start_pos;
-    tx_extra[start_pos] = mm_merkle_tree_depth;
+    uint8_t *ptr = &tx_extra[start_pos], *start = ptr;
+    tools::write_varint(ptr, mm_merkle_tree_depth);
     //write data
-    ++start_pos;
+    const size_t varint_size = ptr - start;
+    start_pos += varint_size;
     memcpy(&tx_extra[start_pos], &mm_merkle_root, 32);
+    tx_extra.resize(tx_extra.size() - (max_varint_size - varint_size));
+    tx_extra[len_bytes] = 32 + varint_size;
     return true;
   }
   //---------------------------------------------------------------
diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h
index 33fec238e..bd4dcd83d 100644
--- a/src/cryptonote_basic/cryptonote_format_utils.h
+++ b/src/cryptonote_basic/cryptonote_format_utils.h
@@ -83,7 +83,7 @@ namespace cryptonote
   std::vector<crypto::public_key> get_additional_tx_pub_keys_from_extra(const transaction_prefix& tx);
   bool add_additional_tx_pub_keys_to_extra(std::vector<uint8_t>& tx_extra, const std::vector<crypto::public_key>& additional_pub_keys);
   bool add_extra_nonce_to_tx_extra(std::vector<uint8_t>& tx_extra, const blobdata& extra_nonce);
-  bool add_mm_merkle_root_to_tx_extra(std::vector<uint8_t>& tx_extra, const crypto::hash& mm_merkle_root, size_t mm_merkle_tree_depth);
+  bool add_mm_merkle_root_to_tx_extra(std::vector<uint8_t>& tx_extra, const crypto::hash& mm_merkle_root, uint64_t mm_merkle_tree_depth);
   bool remove_field_from_tx_extra(std::vector<uint8_t>& tx_extra, const std::type_info &type);
   void set_payment_id_to_tx_extra_nonce(blobdata& extra_nonce, const crypto::hash& payment_id);
   void set_encrypted_payment_id_to_tx_extra_nonce(blobdata& extra_nonce, const crypto::hash8& payment_id);
diff --git a/src/cryptonote_basic/merge_mining.cpp b/src/cryptonote_basic/merge_mining.cpp
index 0e649e095..3e26c00e3 100644
--- a/src/cryptonote_basic/merge_mining.cpp
+++ b/src/cryptonote_basic/merge_mining.cpp
@@ -71,21 +71,21 @@ uint32_t get_path_from_aux_slot(uint32_t slot, uint32_t n_aux_chains)
   return path;
 }
 //---------------------------------------------------------------
-uint32_t encode_mm_depth(uint32_t n_aux_chains, uint32_t nonce)
+uint64_t encode_mm_depth(uint32_t n_aux_chains, uint32_t nonce)
 {
   CHECK_AND_ASSERT_THROW_MES(n_aux_chains > 0, "n_aux_chains is 0");
+  CHECK_AND_ASSERT_THROW_MES(n_aux_chains <= 256, "n_aux_chains is too large");
 
   // how many bits to we need to representing n_aux_chains - 1
   uint32_t n_bits = 1;
-  while ((1u << n_bits) < n_aux_chains && n_bits < 16)
+  while ((1u << n_bits) < n_aux_chains)
     ++n_bits;
-  CHECK_AND_ASSERT_THROW_MES(n_bits <= 16, "Way too many bits required");
 
-  const uint32_t depth = (n_bits - 1) | ((n_aux_chains - 1) << 3) | (nonce << (3 + n_bits));
+  const uint64_t depth = (n_bits - 1) | ((n_aux_chains - 1) << 3) | (((uint64_t)nonce) << (3 + n_bits));
   return depth;
 }
 //---------------------------------------------------------------
-bool decode_mm_depth(uint32_t depth, uint32_t &n_aux_chains, uint32_t &nonce)
+bool decode_mm_depth(uint64_t depth, uint32_t &n_aux_chains, uint32_t &nonce)
 {
   const uint32_t n_bits = 1 + (depth & 7);
   n_aux_chains = 1 + (depth >> 3 & ((1 << n_bits) - 1));
diff --git a/src/cryptonote_basic/merge_mining.h b/src/cryptonote_basic/merge_mining.h
index acb70ebf0..ef4c92d67 100644
--- a/src/cryptonote_basic/merge_mining.h
+++ b/src/cryptonote_basic/merge_mining.h
@@ -36,6 +36,6 @@ namespace cryptonote
 {
   uint32_t get_aux_slot(const crypto::hash &id, uint32_t nonce, uint32_t n_aux_chains);
   uint32_t get_path_from_aux_slot(uint32_t slot, uint32_t n_aux_chains);
-  uint32_t encode_mm_depth(uint32_t n_aux_chains, uint32_t nonce);
-  bool decode_mm_depth(uint32_t depth, uint32_t &n_aux_chains, uint32_t &nonce);
+  uint64_t encode_mm_depth(uint32_t n_aux_chains, uint32_t nonce);
+  bool decode_mm_depth(uint64_t depth, uint32_t &n_aux_chains, uint32_t &nonce);
 }
diff --git a/src/cryptonote_basic/tx_extra.h b/src/cryptonote_basic/tx_extra.h
index af6ee5197..5eddb6b3a 100644
--- a/src/cryptonote_basic/tx_extra.h
+++ b/src/cryptonote_basic/tx_extra.h
@@ -124,7 +124,7 @@ namespace cryptonote
       END_SERIALIZE()
     };
 
-    size_t depth;
+    uint64_t depth;
     crypto::hash merkle_root;
 
     // load
diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp
index fbca32f80..9a0b02f70 100644
--- a/src/rpc/core_rpc_server.cpp
+++ b/src/rpc/core_rpc_server.cpp
@@ -2082,11 +2082,12 @@ namespace cryptonote
     }
 
     crypto::hash merkle_root;
-    size_t merkle_tree_depth = 0;
     std::vector<std::pair<crypto::hash, crypto::hash>> aux_pow;
     std::vector<crypto::hash> aux_pow_raw;
+    std::vector<crypto::hash> aux_pow_id_raw;
     aux_pow.reserve(req.aux_pow.size());
-    aux_pow_raw.reserve(req.aux_pow.size());
+    aux_pow_raw.resize(req.aux_pow.size());
+    aux_pow_id_raw.resize(req.aux_pow.size());
     for (const auto &s: req.aux_pow)
     {
       aux_pow.push_back({});
@@ -2102,7 +2103,6 @@ namespace cryptonote
         error_resp.message = "Invalid aux pow hash";
         return false;
       }
-      aux_pow_raw.push_back(aux_pow.back().second);
     }
 
     size_t path_domain = 1;
@@ -2111,11 +2111,14 @@ namespace cryptonote
     uint32_t nonce;
     const uint32_t max_nonce = 65535;
     bool collision = true;
+    std::vector<uint32_t> slots(aux_pow.size());
     for (nonce = 0; nonce <= max_nonce; ++nonce)
     {
-      std::vector<bool> slots(aux_pow.size(), false);
+      std::vector<bool> slot_seen(aux_pow.size(), false);
       collision = false;
       for (size_t idx = 0; idx < aux_pow.size(); ++idx)
+        slots[idx] = 0xffffffff;
+      for (size_t idx = 0; idx < aux_pow.size(); ++idx)
       {
         const uint32_t slot = cryptonote::get_aux_slot(aux_pow[idx].first, nonce, aux_pow.size());
         if (slot >= aux_pow.size())
@@ -2124,12 +2127,13 @@ namespace cryptonote
           error_resp.message = "Computed slot is out of range";
           return false;
         }
-        if (slots[slot])
+        if (slot_seen[slot])
         {
           collision = true;
           break;
         }
-        slots[slot] = true;
+        slot_seen[slot] = true;
+        slots[idx] = slot;
       }
       if (!collision)
         break;
@@ -2141,6 +2145,19 @@ namespace cryptonote
       return false;
     }
 
+    // set the order determined above
+    for (size_t i = 0; i < aux_pow.size(); ++i)
+    {
+      if (slots[i] >= aux_pow.size())
+      {
+        error_resp.code = CORE_RPC_ERROR_CODE_INTERNAL_ERROR;
+        error_resp.message = "Slot value out of range";
+        return false;
+      }
+      aux_pow_raw[slots[i]] = aux_pow[i].second;
+      aux_pow_id_raw[slots[i]] = aux_pow[i].first;
+    }
+
     crypto::tree_hash((const char(*)[crypto::HASH_SIZE])aux_pow_raw.data(), aux_pow_raw.size(), merkle_root.data);
     res.merkle_root = epee::string_tools::pod_to_hex(merkle_root);
     res.merkle_tree_depth = cryptonote::encode_mm_depth(aux_pow.size(), nonce);
@@ -2167,7 +2184,7 @@ namespace cryptonote
       error_resp.message = "Error removing existing merkle root";
       return false;
     }
-    if (!add_mm_merkle_root_to_tx_extra(b.miner_tx.extra, merkle_root, merkle_tree_depth))
+    if (!add_mm_merkle_root_to_tx_extra(b.miner_tx.extra, merkle_root, res.merkle_tree_depth))
     {
       error_resp.code = CORE_RPC_ERROR_CODE_INTERNAL_ERROR;
       error_resp.message = "Error adding merkle root";
@@ -2181,7 +2198,8 @@ namespace cryptonote
 
     res.blocktemplate_blob = string_tools::buff_to_hex_nodelimer(block_blob);
     res.blockhashing_blob = string_tools::buff_to_hex_nodelimer(hashing_blob);
-    res.aux_pow = req.aux_pow;
+    for (size_t i = 0; i < aux_pow_raw.size(); ++i)
+      res.aux_pow.push_back({epee::string_tools::pod_to_hex(aux_pow_id_raw[i]), epee::string_tools::pod_to_hex(aux_pow_raw[i])});
     res.status = CORE_RPC_STATUS_OK;
     return true;
   }
diff --git a/src/rpc/core_rpc_server_commands_defs.h b/src/rpc/core_rpc_server_commands_defs.h
index c0330eed9..2a0a6201d 100644
--- a/src/rpc/core_rpc_server_commands_defs.h
+++ b/src/rpc/core_rpc_server_commands_defs.h
@@ -1094,7 +1094,7 @@ namespace cryptonote
       blobdata blocktemplate_blob;
       blobdata blockhashing_blob;
       std::string merkle_root;
-      uint32_t merkle_tree_depth;
+      uint64_t merkle_tree_depth;
       std::vector<aux_pow_t> aux_pow;
 
       BEGIN_KV_SERIALIZE_MAP()