From 53e632b435582193d4a708e2b7cf0bfc01ae821b Mon Sep 17 00:00:00 2001 From: Crypto City Date: Sun, 12 Nov 2023 11:33:20 +0000 Subject: [PATCH] fix merge mining with more than one merge mined chain reported by sech1 --- .../cryptonote_format_utils.cpp | 20 +++++++---- .../cryptonote_format_utils.h | 2 +- src/cryptonote_basic/merge_mining.cpp | 10 +++--- src/cryptonote_basic/merge_mining.h | 4 +-- src/cryptonote_basic/tx_extra.h | 2 +- src/rpc/core_rpc_server.cpp | 34 ++++++++++++++----- src/rpc/core_rpc_server_commands_defs.h | 2 +- tests/unit_tests/crypto.cpp | 10 ++++-- 8 files changed, 56 insertions(+), 28 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& tx_extra, const crypto::hash& mm_merkle_root, size_t mm_merkle_tree_depth) + bool add_mm_merkle_root_to_tx_extra(std::vector& 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 get_additional_tx_pub_keys_from_extra(const transaction_prefix& tx); bool add_additional_tx_pub_keys_to_extra(std::vector& tx_extra, const std::vector& additional_pub_keys); bool add_extra_nonce_to_tx_extra(std::vector& tx_extra, const blobdata& extra_nonce); - bool add_mm_merkle_root_to_tx_extra(std::vector& tx_extra, const crypto::hash& mm_merkle_root, size_t mm_merkle_tree_depth); + bool add_mm_merkle_root_to_tx_extra(std::vector& tx_extra, const crypto::hash& mm_merkle_root, uint64_t mm_merkle_tree_depth); bool remove_field_from_tx_extra(std::vector& 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> aux_pow; std::vector aux_pow_raw; + std::vector 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,10 +2111,13 @@ namespace cryptonote uint32_t nonce; const uint32_t max_nonce = 65535; bool collision = true; + std::vector slots(aux_pow.size()); for (nonce = 0; nonce <= max_nonce; ++nonce) { - std::vector slots(aux_pow.size(), false); + std::vector 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()); @@ -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; BEGIN_KV_SERIALIZE_MAP() diff --git a/tests/unit_tests/crypto.cpp b/tests/unit_tests/crypto.cpp index e41f3955f..88ecb5853 100644 --- a/tests/unit_tests/crypto.cpp +++ b/tests/unit_tests/crypto.cpp @@ -307,17 +307,21 @@ TEST(Crypto, tree_branch) ASSERT_FALSE(crypto::tree_branch((const char(*)[32])inputs, 5, crypto::null_hash.data, (char(*)[32])branch, &depth, &path)); // depth encoding roundtrip - for (uint32_t n_chains = 1; n_chains <= 65; ++n_chains) + for (uint32_t n_chains = 1; n_chains <= 256; ++n_chains) { - for (uint32_t nonce = 0; nonce < 1024; ++nonce) + for (uint32_t nonce = 0xffffffff - 512; nonce != 1025; ++nonce) { - const uint32_t depth = cryptonote::encode_mm_depth(n_chains, nonce); + const uint64_t depth = cryptonote::encode_mm_depth(n_chains, nonce); uint32_t n_chains_2, nonce_2; ASSERT_TRUE(cryptonote::decode_mm_depth(depth, n_chains_2, nonce_2)); ASSERT_EQ(n_chains, n_chains_2); ASSERT_EQ(nonce, nonce_2); } } + + // 257 chains is too much + try { cryptonote::encode_mm_depth(257, 0); ASSERT_TRUE(false); } + catch (...) {} } TEST(Crypto, generator_consistency)