From d6bce4be36c5636e1fd4dcf208e469c29d191726 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Apr 2016 16:10:10 +0100 Subject: [PATCH 1/5] core: move tx_extra parsing errors to log level 1 They're not fatal, though indicate something wrong --- contrib/epee/include/misc_log_ex.h | 10 +++++++++- src/cryptonote_core/cryptonote_format_utils.cpp | 8 ++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/contrib/epee/include/misc_log_ex.h b/contrib/epee/include/misc_log_ex.h index 7cb1e61a..82760dff 100644 --- a/contrib/epee/include/misc_log_ex.h +++ b/contrib/epee/include/misc_log_ex.h @@ -1441,8 +1441,16 @@ POP_WARNINGS #define CHECK_AND_ASSERT_MES(expr, fail_ret_val, message) do{if(!(expr)) {LOG_ERROR(message); return fail_ret_val;};}while(0) #endif +#ifndef CHECK_AND_NO_ASSERT_MES_L +#define CHECK_AND_NO_ASSERT_MES_L(expr, fail_ret_val, l, message) do{if(!(expr)) {LOG_PRINT_L##l(message); /*LOCAL_ASSERT(expr);*/ return fail_ret_val;};}while(0) +#endif + #ifndef CHECK_AND_NO_ASSERT_MES -#define CHECK_AND_NO_ASSERT_MES(expr, fail_ret_val, message) do{if(!(expr)) {LOG_PRINT_L0(message); /*LOCAL_ASSERT(expr);*/ return fail_ret_val;};}while(0) +#define CHECK_AND_NO_ASSERT_MES(expr, fail_ret_val, message) CHECK_AND_NO_ASSERT_MES_L(expr, fail_ret_val, 0, message) +#endif + +#ifndef CHECK_AND_NO_ASSERT_MES_L1 +#define CHECK_AND_NO_ASSERT_MES_L1(expr, fail_ret_val, message) CHECK_AND_NO_ASSERT_MES_L(expr, fail_ret_val, 1, message) #endif diff --git a/src/cryptonote_core/cryptonote_format_utils.cpp b/src/cryptonote_core/cryptonote_format_utils.cpp index 3b9dcc8a..ff752ae4 100644 --- a/src/cryptonote_core/cryptonote_format_utils.cpp +++ b/src/cryptonote_core/cryptonote_format_utils.cpp @@ -291,14 +291,14 @@ namespace cryptonote { tx_extra_field field; bool r = ::do_serialize(ar, field); - CHECK_AND_NO_ASSERT_MES(r, false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); + CHECK_AND_NO_ASSERT_MES_L1(r, false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); tx_extra_fields.push_back(field); std::ios_base::iostate state = iss.rdstate(); eof = (EOF == iss.peek()); iss.clear(state); } - CHECK_AND_NO_ASSERT_MES(::serialization::check_stream_state(ar), false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); + CHECK_AND_NO_ASSERT_MES_L1(::serialization::check_stream_state(ar), false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); return true; } @@ -357,7 +357,7 @@ namespace cryptonote { tx_extra_field field; bool r = ::do_serialize(ar, field); - CHECK_AND_NO_ASSERT_MES(r, false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); + CHECK_AND_NO_ASSERT_MES_L1(r, false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); if (field.type() != typeid(tx_extra_nonce)) ::do_serialize(newar, field); @@ -365,7 +365,7 @@ namespace cryptonote eof = (EOF == iss.peek()); iss.clear(state); } - CHECK_AND_NO_ASSERT_MES(::serialization::check_stream_state(ar), false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); + CHECK_AND_NO_ASSERT_MES_L1(::serialization::check_stream_state(ar), false, "failed to deserialize extra field. extra = " << string_tools::buff_to_hex_nodelimer(std::string(reinterpret_cast(tx_extra.data()), tx_extra.size()))); tx_extra.clear(); std::string s = oss.str(); tx_extra.reserve(s.size()); From 7a663873cebd135eb32ca68ad1f05873e29248a9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 1 May 2016 11:31:30 +0100 Subject: [PATCH 2/5] unit_tests: fix UNBOUND_LIBRARIES/UNBOUND_LIBRARY typo --- tests/unit_tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index 2e510022..ff7582eb 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -70,7 +70,7 @@ target_link_libraries(unit_tests ${Boost_REGEX_LIBRARY} ${Boost_SYSTEM_LIBRARY} ${Boost_THREAD_LIBRARY} - ${UNBOUND_LIBRARIES} + ${UNBOUND_LIBRARY} ${EXTRA_LIBRARIES}) set_property(TARGET unit_tests PROPERTY From 3eff37f931cf0b69fc098c9cbe7323eaf11c2255 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 1 May 2016 11:32:10 +0100 Subject: [PATCH 3/5] unit_tests: add a write_varint/read_varint test --- tests/unit_tests/CMakeLists.txt | 3 +- tests/unit_tests/varint.cpp | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 tests/unit_tests/varint.cpp diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index ff7582eb..1b272cf5 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -50,7 +50,8 @@ set(unit_tests_sources test_peerlist.cpp test_protocol_pack.cpp hardfork.cpp - unbound.cpp) + unbound.cpp + varint.cpp) set(unit_tests_headers unit_tests_utils.h) diff --git a/tests/unit_tests/varint.cpp b/tests/unit_tests/varint.cpp new file mode 100644 index 00000000..a483cbd5 --- /dev/null +++ b/tests/unit_tests/varint.cpp @@ -0,0 +1,66 @@ +// Copyright (c) 2014-2016, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers + +#include +#include +#include +#include +#include +#include +#include "cryptonote_core/cryptonote_basic.h" +#include "cryptonote_core/cryptonote_basic_impl.h" +#include "serialization/serialization.h" +#include "serialization/binary_archive.h" +#include "serialization/json_archive.h" +#include "serialization/debug_archive.h" +#include "serialization/variant.h" +#include "serialization/vector.h" +#include "serialization/binary_utils.h" +#include "gtest/gtest.h" +using namespace std; + +TEST(varint, equal) +{ + for (uint64_t idx = 0; idx < 65537; ++idx) + { + char buf[12]; + char *bufptr = buf; + tools::write_varint(bufptr, idx); + uint64_t bytes = bufptr - buf; + ASSERT_TRUE (bytes > 0 && bytes <= sizeof(buf)); + + uint64_t idx2; + bufptr = buf; + std::string s(buf, bytes); + int read = tools::read_varint(s.begin(), s.end(), idx2); + ASSERT_EQ (read, bytes); + ASSERT_TRUE(idx2 == idx); + } +} From a6e717ed303ca1c7d1b0efd72e20ea3e6a9d3ba4 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 1 May 2016 11:42:48 +0100 Subject: [PATCH 4/5] cn_deserialize: deserialize tx_extra too --- src/blockchain_utilities/cn_deserialize.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/blockchain_utilities/cn_deserialize.cpp b/src/blockchain_utilities/cn_deserialize.cpp index bf02dc15..b8d38c9a 100644 --- a/src/blockchain_utilities/cn_deserialize.cpp +++ b/src/blockchain_utilities/cn_deserialize.cpp @@ -27,6 +27,7 @@ // THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "cryptonote_core/cryptonote_basic.h" +#include "cryptonote_core/tx_extra.h" #include "cryptonote_core/blockchain.h" #include "blockchain_utilities.h" #include "common/command_line.h" @@ -140,6 +141,7 @@ int main(int argc, char* argv[]) cryptonote::block block; cryptonote::transaction tx; + std::vector fields; if (cryptonote::parse_and_validate_block_from_blob(blob, block)) { std::cout << "Parsed block:" << std::endl; @@ -149,6 +151,25 @@ int main(int argc, char* argv[]) { std::cout << "Parsed transaction:" << std::endl; std::cout << cryptonote::obj_to_json_str(tx) << std::endl; + + if (cryptonote::parse_tx_extra(tx.extra, fields)) + { + std::cout << "tx_extra has " << fields.size() << " field(s)" << std::endl; + for (size_t n = 0; n < fields.size(); ++n) + { + std::cout << "field " << n << ": "; + if (typeid(cryptonote::tx_extra_padding) == fields[n].type()) std::cout << "extra padding: " << boost::get(fields[n]).size << " bytes"; + else if (typeid(cryptonote::tx_extra_pub_key) == fields[n].type()) std::cout << "extra pub key: " << boost::get(fields[n]).pub_key; + else if (typeid(cryptonote::tx_extra_nonce) == fields[n].type()) std::cout << "extra nonce: " << boost::get(fields[n]).nonce; + else if (typeid(cryptonote::tx_extra_merge_mining_tag) == fields[n].type()) std::cout << "extra merge mining tag: depth " << boost::get(fields[n]).depth << ", merkle root " << boost::get(fields[n]).merkle_root; + else std::cout << "unknown"; + std::cout << std::endl; + } + } + else + { + std::cout << "Failed to parse tx_extra" << std::endl; + } } else { From 9ef8c7b694f8f00c8a70e05ff88a5768b8f78a3c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 1 May 2016 19:40:14 +0100 Subject: [PATCH 5/5] tests: fix tests broken by the removal of the block reward accumulation loop The tests for rejection of unmixable outputs in v2 are commented out, as there are no unmixable outputs created anymore. This should be restored at some point. --- src/cryptonote_core/cryptonote_format_utils.h | 2 +- tests/core_tests/chaingen.cpp | 2 +- tests/core_tests/chaingen.h | 2 +- tests/core_tests/chaingen_main.cpp | 6 ++-- tests/core_tests/v2_tests.cpp | 28 ++++++++----------- tests/core_tests/v2_tests.h | 2 +- 6 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/cryptonote_core/cryptonote_format_utils.h b/src/cryptonote_core/cryptonote_format_utils.h index 4142829e..056940e9 100644 --- a/src/cryptonote_core/cryptonote_format_utils.h +++ b/src/cryptonote_core/cryptonote_format_utils.h @@ -44,7 +44,7 @@ namespace cryptonote crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx); - bool construct_miner_tx(size_t height, size_t median_size, uint64_t already_generated_coins, size_t current_block_size, uint64_t fee, const account_public_address &miner_address, transaction& tx, const blobdata& extra_nonce = blobdata(), size_t max_outs = 1, uint8_t hard_fork_version = 1); + bool construct_miner_tx(size_t height, size_t median_size, uint64_t already_generated_coins, size_t current_block_size, uint64_t fee, const account_public_address &miner_address, transaction& tx, const blobdata& extra_nonce = blobdata(), size_t max_outs = 999, uint8_t hard_fork_version = 1); bool encrypt_payment_id(crypto::hash8 &payment_id, const crypto::public_key &public_key, const crypto::secret_key &secret_key); bool decrypt_payment_id(crypto::hash8 &payment_id, const crypto::public_key &public_key, const crypto::secret_key &secret_key); diff --git a/tests/core_tests/chaingen.cpp b/tests/core_tests/chaingen.cpp index 4fdd0a60..efd0bf1e 100644 --- a/tests/core_tests/chaingen.cpp +++ b/tests/core_tests/chaingen.cpp @@ -222,7 +222,7 @@ bool test_generator::construct_block_manually(block& blk, const block& prev_bloc blk.timestamp = actual_params & bf_timestamp ? timestamp : prev_block.timestamp + DIFFICULTY_BLOCKS_ESTIMATE_TIMESPAN; // Keep difficulty unchanged blk.prev_id = actual_params & bf_prev_id ? prev_id : get_block_hash(prev_block); blk.tx_hashes = actual_params & bf_tx_hashes ? tx_hashes : std::vector(); - max_outs = actual_params & bf_max_outs ? max_outs : 1; + max_outs = actual_params & bf_max_outs ? max_outs : 9999; size_t height = get_block_height(prev_block) + 1; uint64_t already_generated_coins = get_already_generated_coins(prev_block); diff --git a/tests/core_tests/chaingen.h b/tests/core_tests/chaingen.h index 652413b8..0e5dbb0e 100644 --- a/tests/core_tests/chaingen.h +++ b/tests/core_tests/chaingen.h @@ -241,7 +241,7 @@ public: const cryptonote::account_base& miner_acc, int actual_params = bf_none, uint8_t major_ver = 0, uint8_t minor_ver = 0, uint64_t timestamp = 0, const crypto::hash& prev_id = crypto::hash(), const cryptonote::difficulty_type& diffic = 1, const cryptonote::transaction& miner_tx = cryptonote::transaction(), - const std::vector& tx_hashes = std::vector(), size_t txs_sizes = 0, size_t max_outs = 0); + const std::vector& tx_hashes = std::vector(), size_t txs_sizes = 0, size_t max_outs = 999); bool construct_block_manually_tx(cryptonote::block& blk, const cryptonote::block& prev_block, const cryptonote::account_base& miner_acc, const std::vector& tx_hashes, size_t txs_size); diff --git a/tests/core_tests/chaingen_main.cpp b/tests/core_tests/chaingen_main.cpp index 9f8a5782..e20f7a15 100644 --- a/tests/core_tests/chaingen_main.cpp +++ b/tests/core_tests/chaingen_main.cpp @@ -168,9 +168,9 @@ int main(int argc, char* argv[]) GENERATE_AND_PLAY(gen_v2_tx_mixable_0_mixin); GENERATE_AND_PLAY(gen_v2_tx_mixable_low_mixin); - GENERATE_AND_PLAY(gen_v2_tx_unmixable_only); - GENERATE_AND_PLAY(gen_v2_tx_unmixable_one); - GENERATE_AND_PLAY(gen_v2_tx_unmixable_two); +// GENERATE_AND_PLAY(gen_v2_tx_unmixable_only); +// GENERATE_AND_PLAY(gen_v2_tx_unmixable_one); +// GENERATE_AND_PLAY(gen_v2_tx_unmixable_two); GENERATE_AND_PLAY(gen_v2_tx_dust); std::cout << (failed_tests.empty() ? concolor::green : concolor::magenta); diff --git a/tests/core_tests/v2_tests.cpp b/tests/core_tests/v2_tests.cpp index d0545628..fe6b8b27 100644 --- a/tests/core_tests/v2_tests.cpp +++ b/tests/core_tests/v2_tests.cpp @@ -38,7 +38,7 @@ using namespace cryptonote; //---------------------------------------------------------------------------------------------------------------------- // Tests -bool gen_v2_tx_validation_base::generate_with(std::vector& events, const int *out_idx, int mixin, uint64_t amount_paid, size_t max_outs, bool valid) const +bool gen_v2_tx_validation_base::generate_with(std::vector& events, const int *out_idx, int mixin, uint64_t amount_paid, bool valid) const { uint64_t ts_start = 1338224400; @@ -52,9 +52,9 @@ bool gen_v2_tx_validation_base::generate_with(std::vector& eve for (size_t n = 0; n < 4; ++n) { miner_accounts[n].generate(); CHECK_AND_ASSERT_MES(generator.construct_block_manually(blocks[n], *prev_block, miner_accounts[n], - test_generator::bf_major_ver | test_generator::bf_minor_ver | test_generator::bf_timestamp | test_generator::bf_max_outs, + test_generator::bf_major_ver | test_generator::bf_minor_ver | test_generator::bf_timestamp, 2, 2, prev_block->timestamp + DIFFICULTY_BLOCKS_ESTIMATE_TIMESPAN * 2, // v2 has blocks twice as long - crypto::hash(), 0, transaction(), std::vector(), 0, max_outs), + crypto::hash(), 0, transaction(), std::vector(), 0, 0), false, "Failed to generate block"); events.push_back(blocks[n]); prev_block = blocks + n; @@ -68,9 +68,9 @@ bool gen_v2_tx_validation_base::generate_with(std::vector& eve { cryptonote::block blk; CHECK_AND_ASSERT_MES(generator.construct_block_manually(blk, blk_last, miner_account, - test_generator::bf_major_ver | test_generator::bf_minor_ver | test_generator::bf_timestamp | test_generator::bf_max_outs, + test_generator::bf_major_ver | test_generator::bf_minor_ver | test_generator::bf_timestamp, 2, 2, blk_last.timestamp + DIFFICULTY_BLOCKS_ESTIMATE_TIMESPAN * 2, // v2 has blocks twice as long - crypto::hash(), 0, transaction(), std::vector(), 0, max_outs), + crypto::hash(), 0, transaction(), std::vector(), 0, 0), false, "Failed to generate block"); events.push_back(blk); blk_last = blk; @@ -123,8 +123,7 @@ bool gen_v2_tx_mixable_0_mixin::generate(std::vector& events) const int mixin = 0; const int out_idx[] = {1, -1}; const uint64_t amount_paid = 10000; - const size_t max_outs = 11; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, false); + return generate_with(events, out_idx, mixin, amount_paid, false); } bool gen_v2_tx_mixable_low_mixin::generate(std::vector& events) const @@ -132,8 +131,7 @@ bool gen_v2_tx_mixable_low_mixin::generate(std::vector& events const int mixin = 1; const int out_idx[] = {1, -1}; const uint64_t amount_paid = 10000; - const size_t max_outs = 11; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, false); + return generate_with(events, out_idx, mixin, amount_paid, false); } bool gen_v2_tx_unmixable_only::generate(std::vector& events) const @@ -141,8 +139,7 @@ bool gen_v2_tx_unmixable_only::generate(std::vector& events) c const int mixin = 0; const int out_idx[] = {0, -1}; const uint64_t amount_paid = 10000; - const size_t max_outs = 1; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, true); + return generate_with(events, out_idx, mixin, amount_paid, true); } bool gen_v2_tx_unmixable_one::generate(std::vector& events) const @@ -150,8 +147,7 @@ bool gen_v2_tx_unmixable_one::generate(std::vector& events) co const int mixin = 0; const int out_idx[] = {0, 1, -1}; const uint64_t amount_paid = 10000; - const size_t max_outs = 11; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, true); + return generate_with(events, out_idx, mixin, amount_paid, true); } bool gen_v2_tx_unmixable_two::generate(std::vector& events) const @@ -159,8 +155,7 @@ bool gen_v2_tx_unmixable_two::generate(std::vector& events) co const int mixin = 0; const int out_idx[] = {0, 1, 2, -1}; const uint64_t amount_paid = 10000; - const size_t max_outs = 11; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, false); + return generate_with(events, out_idx, mixin, amount_paid, false); } bool gen_v2_tx_dust::generate(std::vector& events) const @@ -168,6 +163,5 @@ bool gen_v2_tx_dust::generate(std::vector& events) const const int mixin = 2; const int out_idx[] = {1, -1}; const uint64_t amount_paid = 10001; - const size_t max_outs = 11; - return generate_with(events, out_idx, mixin, amount_paid, max_outs, false); + return generate_with(events, out_idx, mixin, amount_paid, false); } diff --git a/tests/core_tests/v2_tests.h b/tests/core_tests/v2_tests.h index c296f0d8..10049ec9 100644 --- a/tests/core_tests/v2_tests.h +++ b/tests/core_tests/v2_tests.h @@ -70,7 +70,7 @@ struct gen_v2_tx_validation_base : public test_chain_unit_base } bool generate_with(std::vector& events, const int *out_idx, int mixin, - uint64_t amount_paid, size_t max_outs, bool valid) const; + uint64_t amount_paid, bool valid) const; private: size_t m_invalid_tx_index;