From 5eef64578ba1e19c9a6045a25369db19d4136e75 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 21:56:37 +0000 Subject: [PATCH 01/13] db: throw when given a non txout_to_key output to add The check was explicit in the original version, so it seems safer to make it explicit here, especially as it is now done implicitely in a different place, away from the original check. --- src/blockchain_db/berkeleydb/db_bdb.cpp | 4 ++++ src/blockchain_db/lmdb/db_lmdb.cpp | 4 ++++ src/cryptonote_core/blockchain.cpp | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/src/blockchain_db/berkeleydb/db_bdb.cpp b/src/blockchain_db/berkeleydb/db_bdb.cpp index 236bc4fe..4799afeb 100644 --- a/src/blockchain_db/berkeleydb/db_bdb.cpp +++ b/src/blockchain_db/berkeleydb/db_bdb.cpp @@ -391,6 +391,10 @@ void BlockchainBDB::add_output(const crypto::hash& tx_hash, const tx_out& tx_out if (m_output_keys->put(DB_DEFAULT_TX, &k, &data, 0)) throw0(DB_ERROR("Failed to add output pubkey to db transaction")); } + else + { + throw0(DB_ERROR("Wrong output type: expected txout_to_key")); + } m_num_outputs++; } diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 4a455017..b5f76301 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -695,6 +695,10 @@ void BlockchainLMDB::add_output(const crypto::hash& tx_hash, const tx_out& tx_ou if (mdb_put(*m_write_txn, m_output_keys, &k, &data, 0)) throw0(DB_ERROR("Failed to add output pubkey to db transaction")); } + else + { + throw0(DB_ERROR("Wrong output type: expected txout_to_key")); + } m_num_outputs++; } diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index badccd06..b83a5fd0 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2246,6 +2246,11 @@ bool Blockchain::check_tx_input(const txin_to_key& txin, const crypto::hash& tx_ return false; } + // The original code includes a check for the output corresponding to this input + // to be a txout_to_key. This is removed, as the database does not store this info, + // but only txout_to_key outputs are stored in the DB in the first place, done in + // Blockchain*::add_output + m_output_keys.push_back(pubkey); return true; } From 3cabdb5ef239fa1d1638d5233de882f59c787dc8 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 21:59:26 +0000 Subject: [PATCH 02/13] core: catch exceptions from get_output_key This can happen when trying to find an amount that does not exist, and fixes a core test. --- src/cryptonote_core/blockchain.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index b83a5fd0..64b7a5b2 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -155,7 +155,15 @@ bool Blockchain::scan_outputkeys_for_indexes(const txin_to_key& tx_in_to_key, vi if (!found) { - m_db->get_output_key(tx_in_to_key.amount, absolute_offsets, outputs); + try + { + m_db->get_output_key(tx_in_to_key.amount, absolute_offsets, outputs); + } + catch (...) + { + LOG_PRINT_L0("Output does not exist! amount = " << tx_in_to_key.amount); + return false; + } } else { @@ -167,7 +175,15 @@ bool Blockchain::scan_outputkeys_for_indexes(const txin_to_key& tx_in_to_key, vi std::vector add_outputs; for (size_t i = outputs.size(); i < absolute_offsets.size(); i++) add_offsets.push_back(absolute_offsets[i]); - m_db->get_output_key(tx_in_to_key.amount, add_offsets, add_outputs); + try + { + m_db->get_output_key(tx_in_to_key.amount, add_offsets, add_outputs); + } + catch (...) + { + LOG_PRINT_L0("Output does not exist! amount = " << tx_in_to_key.amount); + return false; + } outputs.insert(outputs.end(), add_outputs.begin(), add_outputs.end()); } } From 5cec076e130f4ae2d1b704086715d59625358feb Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:01:21 +0000 Subject: [PATCH 03/13] blockchain: add a missing validity check to rollback_blockchain_switching It was present in the original code --- src/cryptonote_core/blockchain.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 64b7a5b2..f3111a96 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -717,6 +717,12 @@ bool Blockchain::rollback_blockchain_switching(std::list& original_chain, LOG_PRINT_L3("Blockchain::" << __func__); CRITICAL_REGION_LOCAL(m_blockchain_lock); + // fail if rollback_height passed is too high + if (rollback_height > m_db->height()) + { + return true; + } + m_timestamps_and_difficulties_height = 0; // remove blocks from blockchain until we get back to where we should be. From d837c0ca906ed94e423eba10cc4dc7b3185cb842 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:02:07 +0000 Subject: [PATCH 04/13] blockchain: fix switch to alternative blockchain for more than one block When rolling over more than one block, the db height will decrease, but the split height should be constant, as per the original code. --- src/cryptonote_core/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index f3111a96..128dd9e4 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -797,7 +797,7 @@ bool Blockchain::switch_to_alternative_blockchain(std::listheight()); + rollback_blockchain_switching(disconnected_chain, split_height); // FIXME: Why do we keep invalid blocks around? Possibly in case we hear // about them again so we can immediately dismiss them, but needs some From 22ddf09bea566afad8f6bb97c77c742e223643a4 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:04:13 +0000 Subject: [PATCH 05/13] blockchain: add missing m_tx_pool.on_blockchain_dec It was missing in the port to DB. This is actually a noop, so should not have functional changes. --- src/cryptonote_core/blockchain.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 128dd9e4..6bdbdf4c 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -490,6 +490,7 @@ block Blockchain::pop_block_from_blockchain() } } } + m_tx_pool.on_blockchain_dec(m_blocks.size()-1, get_tail_id()); return popped_block; } From 81cb0fcdccf08afd6f37d21ccfefb31131fd3950 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:07:58 +0000 Subject: [PATCH 06/13] blockchain: fix bitflipping test with quantized block rewards Block reward may now be less than the full amount allowed. This was breaking the bitflipping test. We now keep track of whether a block which was accepted by the core has a lower than allowed block reward, and allow this in the test. --- src/cryptonote_core/blockchain.cpp | 5 ++++- src/cryptonote_core/blockchain.h | 2 +- src/cryptonote_core/verification_context.h | 1 + tests/core_tests/block_validation.cpp | 3 ++- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 6bdbdf4c..a178d120 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -941,13 +941,14 @@ bool Blockchain::prevalidate_miner_transaction(const block& b, uint64_t height) } //------------------------------------------------------------------ // This function validates the miner transaction reward -bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_block_size, uint64_t fee, uint64_t& base_reward, uint64_t already_generated_coins) +bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_block_size, uint64_t fee, uint64_t& base_reward, uint64_t already_generated_coins, bool &partial_block_reward) { LOG_PRINT_L3("Blockchain::" << __func__); //validate reward uint64_t money_in_use = 0; BOOST_FOREACH(auto& o, b.miner_tx.vout) money_in_use += o.amount; + partial_block_reward = false; std::vector last_blocks_sizes; get_last_n_blocks_sizes(last_blocks_sizes, CRYPTONOTE_REWARD_BLOCKS_WINDOW); @@ -977,6 +978,8 @@ bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_bl // emission. This modifies the emission curve very slightly. CHECK_AND_ASSERT_MES(money_in_use - fee <= base_reward, false, "base reward calculation bug"); base_reward = money_in_use - fee; + if(base_reward + fee != money_in_use) + partial_block_reward = true; } return true; } diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 1efc4e39..381236db 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -253,7 +253,7 @@ namespace cryptonote bool handle_alternative_block(const block& b, const crypto::hash& id, block_verification_context& bvc); difficulty_type get_next_difficulty_for_alternative_chain(const std::list& alt_chain, block_extended_info& bei) const; bool prevalidate_miner_transaction(const block& b, uint64_t height); - bool validate_miner_transaction(const block& b, size_t cumulative_block_size, uint64_t fee, uint64_t& base_reward, uint64_t already_generated_coins); + bool validate_miner_transaction(const block& b, size_t cumulative_block_size, uint64_t fee, uint64_t& base_reward, uint64_t already_generated_coins, bool &partial_block_reward); bool validate_transaction(const block& b, uint64_t height, const transaction& tx); bool rollback_blockchain_switching(std::list& original_chain, uint64_t rollback_height); bool add_transaction_from_block(const transaction& tx, const crypto::hash& tx_id, const crypto::hash& bl_id, uint64_t bl_height); diff --git a/src/cryptonote_core/verification_context.h b/src/cryptonote_core/verification_context.h index c467d35f..9766b217 100644 --- a/src/cryptonote_core/verification_context.h +++ b/src/cryptonote_core/verification_context.h @@ -48,5 +48,6 @@ namespace cryptonote bool m_verifivation_failed; //bad block, should drop connection bool m_marked_as_orphaned; bool m_already_exists; + bool m_partial_block_reward; }; } diff --git a/tests/core_tests/block_validation.cpp b/tests/core_tests/block_validation.cpp index 440e4664..e4242f8f 100644 --- a/tests/core_tests/block_validation.cpp +++ b/tests/core_tests/block_validation.cpp @@ -618,7 +618,8 @@ bool gen_block_invalid_binary_format::check_block_verification_context(const cry } else { - return !bvc.m_added_to_main_chain && (bvc.m_already_exists || bvc.m_marked_as_orphaned || bvc.m_verifivation_failed); + return (!bvc.m_added_to_main_chain && (bvc.m_already_exists || bvc.m_marked_as_orphaned || bvc.m_verifivation_failed)) + || (bvc.m_added_to_main_chain && bvc.m_partial_block_reward); } } From 737b6d6cf5b46a68304a8f9f83e262e560c33c0a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:10:27 +0000 Subject: [PATCH 07/13] blockchain: make some flag twiddling code closer to the original Probably paranoid and unnecessary --- src/cryptonote_core/blockchain.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index a178d120..86efc882 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -1327,8 +1327,8 @@ bool Blockchain::handle_alternative_block(const block& b, const crypto::hash& id bool r = switch_to_alternative_blockchain(alt_chain, true); - bvc.m_added_to_main_chain = r; - bvc.m_verifivation_failed = !r; + if(r) bvc.m_added_to_main_chain = true; + else bvc.m_verifivation_failed = true; return r; } From f294be35bce2fff97d85a657347871685990f792 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:11:21 +0000 Subject: [PATCH 08/13] blockchain: reinstate double spending checks in check_tx_inputs This fixes some double spending tests. This may or may not be unneeded in normal (non test) circumstances, to be determined later. Keeping these for now may be slower, but safer. --- src/cryptonote_core/blockchain.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 86efc882..e3ce6607 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2008,9 +2008,7 @@ bool Blockchain::have_tx_keyimges_as_spent(const transaction &tx) const return false; } //------------------------------------------------------------------ -// This function validates transaction inputs and their keys. Previously -// it also performed double spend checking, but that has been moved to its -// own function. +// This function validates transaction inputs and their keys. bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_block_height) { LOG_PRINT_L3("Blockchain::" << __func__); @@ -2112,6 +2110,12 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc // make sure tx output has key offset(s) (is signed to be used) CHECK_AND_ASSERT_MES(in_to_key.key_offsets.size(), false, "empty in_to_key.key_offsets in transaction with id " << get_transaction_hash(tx)); + if(have_tx_keyimg_as_spent(in_to_key.k_image)) + { + LOG_PRINT_L1("Key image already spent in blockchain: " << epee::string_tools::pod_to_hex(in_to_key.k_image)); + return false; + } + // basically, make sure number of inputs == number of signatures CHECK_AND_ASSERT_MES(sig_index < tx.signatures.size(), false, "wrong transaction: not signature entry for input with index= " << sig_index); From a9ff11c8167ac52a4f120d02edf45ea0feac411c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:12:52 +0000 Subject: [PATCH 09/13] blockchain: fix an off by one error in unlocked time check --- src/cryptonote_core/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index e3ce6607..03bf1df6 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2231,7 +2231,7 @@ bool Blockchain::is_tx_spendtime_unlocked(uint64_t unlock_time) const { // ND: Instead of calling get_current_blockchain_height(), call m_db->height() // directly as get_current_blockchain_height() locks the recursive mutex. - if(m_db->height() + CRYPTONOTE_LOCKED_TX_ALLOWED_DELTA_BLOCKS >= unlock_time) + if(m_db->height()-1 + CRYPTONOTE_LOCKED_TX_ALLOWED_DELTA_BLOCKS >= unlock_time) return true; else return false; From f33a88cfc17177e3a079ee2226df9674dfa2d138 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:13:38 +0000 Subject: [PATCH 10/13] blockchain: fix a few block addition bugs If the block reward was too high, the verification failed flag was set, but the function continued. The code which was supposed to trap this flag and return failure failed to trap it, and, while the block was not added to the chain, the function would return success. The reason for avoiding returning when the block reward problem was detected was to be able to return any transactions to the pool if needed. This is now mooted by moving the transaction return code to a separate function, which is now called at all appropriate points, making the logic much simpler, and hopefully correct now. We also move the hard fork version check after the prev_id check, as block which does not go on the top of the chain might not have the expected version there, without being invalid just for this reason. Last, we trap the case where a block fails to be added due to using already spent key images, to set the verification failed flag. --- src/cryptonote_core/blockchain.cpp | 81 +++++++++++++++++------------- src/cryptonote_core/blockchain.h | 1 + 2 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 03bf1df6..71a2b884 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2363,6 +2363,23 @@ bool Blockchain::check_block_timestamp(const block& b) const return check_block_timestamp(timestamps, b); } //------------------------------------------------------------------ +void Blockchain::return_tx_to_pool(const std::vector &txs) +{ + for (auto& tx : txs) + { + cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc); + // We assume that if they were in a block, the transactions are already + // known to the network as a whole. However, if we had mined that block, + // that might not be always true. Unlikely though, and always relaying + // these again might cause a spike of traffic as many nodes re-relay + // all the transactions in a popped block when a reorg happens. + if (!m_tx_pool.add_tx(tx, tvc, true, true)) + { + LOG_PRINT_L0("Failed to return taken transaction with hash: " << get_transaction_hash(tx) << " to tx_pool"); + } + } +} +//------------------------------------------------------------------ // Needs to validate the block and acquire each transaction from the // transaction mem_pool, then pass the block and transactions to // m_db->add_block() @@ -2374,16 +2391,17 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& CRITICAL_REGION_LOCAL(m_blockchain_lock); TIME_MEASURE_START(t1); + if(bl.prev_id != get_tail_id()) + { + LOG_PRINT_L1("Block with id: " << id << std::endl << "has wrong prev_id: " << bl.prev_id << std::endl << "expected: " << get_tail_id()); + return false; + } + // this is a cheap test if (!m_hardfork->check(bl)) { LOG_PRINT_L1("Block with id: " << id << std::endl << "has old version: " << (unsigned)bl.major_version << std::endl << "current: " << (unsigned)m_hardfork->get_current_version()); - return false; - } - - if(bl.prev_id != get_tail_id()) - { - LOG_PRINT_L1("Block with id: " << id << std::endl << "has wrong prev_id: " << bl.prev_id << std::endl << "expected: " << get_tail_id()); + bvc.m_verifivation_failed = true; return false; } @@ -2498,7 +2516,6 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& uint64_t t_exists = 0; uint64_t t_pool = 0; uint64_t t_dblspnd = 0; - bool add_tx_to_pool = false; TIME_MEASURE_FINISH(t3); // XXX old code adds miner tx here @@ -2520,7 +2537,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& { LOG_PRINT_L1("Block with id: " << id << " attempting to add transaction already in blockchain with id: " << tx_id); bvc.m_verifivation_failed = true; - break; + return_tx_to_pool(txs); + return false; } TIME_MEASURE_FINISH(aa); @@ -2532,7 +2550,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& { LOG_PRINT_L1("Block with id: " << id << " has at least one unknown transaction with id: " << tx_id); bvc.m_verifivation_failed = true; - break; + return_tx_to_pool(txs); + return false; } TIME_MEASURE_FINISH(bb); @@ -2568,8 +2587,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& add_block_as_invalid(bl, id); LOG_PRINT_L1("Block with id " << id << " added as invalid because of wrong inputs in transactions"); bvc.m_verifivation_failed = true; - add_tx_to_pool = true; - break; + return_tx_to_pool(txs); + return false; } } #if defined(PER_BLOCK_CHECKPOINT) @@ -2584,8 +2603,8 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& add_block_as_invalid(bl, id); LOG_PRINT_L1("Block with id " << id << " added as invalid because of wrong inputs in transactions"); bvc.m_verifivation_failed = true; - add_tx_to_pool = true; - break; + return_tx_to_pool(txs); + return false; } } #endif @@ -2600,10 +2619,12 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& TIME_MEASURE_START(vmt); uint64_t base_reward = 0; uint64_t already_generated_coins = m_db->height() ? m_db->get_block_already_generated_coins(m_db->height() - 1) : 0; - if(!validate_miner_transaction(bl, cumulative_block_size, fee_summary, base_reward, already_generated_coins)) + if(!validate_miner_transaction(bl, cumulative_block_size, fee_summary, base_reward, already_generated_coins, bvc.m_partial_block_reward)) { LOG_PRINT_L1("Block with id: " << id << " has incorrect miner transaction"); bvc.m_verifivation_failed = true; + return_tx_to_pool(txs); + return false; } TIME_MEASURE_FINISH(vmt); @@ -2623,40 +2644,30 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& TIME_MEASURE_START(addblock); uint64_t new_height = 0; - bool add_success = true; if (!bvc.m_verifivation_failed) { try { new_height = m_db->add_block(bl, block_size, cumulative_difficulty, already_generated_coins, txs); } + catch (const KEY_IMAGE_EXISTS& e) + { + LOG_ERROR("Error adding block with hash: " << id << " to blockchain, what = " << e.what()); + bvc.m_verifivation_failed = true; + return_tx_to_pool(txs); + return false; + } catch (const std::exception& e) { //TODO: figure out the best way to deal with this failure LOG_ERROR("Error adding block with hash: " << id << " to blockchain, what = " << e.what()); - add_success = false; + return_tx_to_pool(txs); + return false; } } - - // if we failed for any reason to verify the block, return taken - // transactions to the tx_pool. - if ((bvc.m_verifivation_failed && add_tx_to_pool) || !add_success) + else { - // return taken transactions to transaction pool - for (auto& tx : txs) - { - cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc); - // We assume that if they were in a block, the transactions are already - // known to the network as a whole. However, if we had mined that block, - // that might not be always true. Unlikely though, and always relaying - // these again might cause a spike of traffic as many nodes re-relay - // all the transactions in a popped block when a reorg happens. - if (!m_tx_pool.add_tx(tx, tvc, true, true)) - { - LOG_PRINT_L0("Failed to return taken transaction with hash: " << get_transaction_hash(tx) << " to tx_pool"); - } - } - return false; + LOG_ERROR("Blocks that failed verification should not reach here"); } TIME_MEASURE_FINISH(addblock); diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 381236db..2f9287e8 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -269,6 +269,7 @@ namespace cryptonote uint64_t get_adjusted_time() const; bool complete_timestamps_vector(uint64_t start_height, std::vector& timestamps); bool update_next_cumulative_size_limit(); + void return_tx_to_pool(const std::vector &txs); bool check_for_double_spend(const transaction& tx, key_images_container& keys_this_block) const; void get_timestamp_and_difficulty(uint64_t ×tamp, difficulty_type &difficulty, const int offset) const; From 2358d0d5be34f8dca0c14f94979300ed49e3f29f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:22:12 +0000 Subject: [PATCH 11/13] tests: use 255 as a "too high" block version While the original cryptonote accepted only the current major version, we can accept higher ones. --- tests/core_tests/block_validation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/core_tests/block_validation.cpp b/tests/core_tests/block_validation.cpp index e4242f8f..8589f07b 100644 --- a/tests/core_tests/block_validation.cpp +++ b/tests/core_tests/block_validation.cpp @@ -79,7 +79,7 @@ bool gen_block_big_major_version::generate(std::vector& events BLOCK_VALIDATION_INIT_GENERATE(); block blk_1; - generator.construct_block_manually(blk_1, blk_0, miner_account, test_generator::bf_major_ver, CURRENT_BLOCK_MAJOR_VERSION + 1); + generator.construct_block_manually(blk_1, blk_0, miner_account, test_generator::bf_major_ver, 255); events.push_back(blk_1); DO_CALLBACK(events, "check_block_purged"); @@ -92,7 +92,7 @@ bool gen_block_big_minor_version::generate(std::vector& events BLOCK_VALIDATION_INIT_GENERATE(); block blk_1; - generator.construct_block_manually(blk_1, blk_0, miner_account, test_generator::bf_minor_ver, 0, CURRENT_BLOCK_MINOR_VERSION + 1); + generator.construct_block_manually(blk_1, blk_0, miner_account, test_generator::bf_minor_ver, 0, 255); events.push_back(blk_1); DO_CALLBACK(events, "check_block_accepted"); From d0a8362b6bceeb9f99ee29360c84ce49db9904d1 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:23:36 +0000 Subject: [PATCH 12/13] tests: fix some double spending tests Some tests assume the first output in a transaction goes to the recipient. However, it can be the change. When it is, the recipient's keys will not recognize this output. To fix this, we send all we have, to ensure there is no change, and the first output goes to the recipient. I'm not sure why this worked with Cryptonote. The tests sent 17 coins, which seems way smaller than the first Bytecoin block reward, so there would have been change too. Maybe outputs were not shuffled originally. --- tests/core_tests/double_spend.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/core_tests/double_spend.h b/tests/core_tests/double_spend.h index 0a2e6057..46743559 100644 --- a/tests/core_tests/double_spend.h +++ b/tests/core_tests/double_spend.h @@ -32,13 +32,14 @@ #include "chaingen.h" const size_t invalid_index_value = std::numeric_limits::max(); +const uint64_t FIRST_BLOCK_REWARD = 17592186044415; template class gen_double_spend_base : public test_chain_unit_base { public: - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; gen_double_spend_base(); @@ -60,7 +61,7 @@ private: template struct gen_double_spend_in_tx : public gen_double_spend_base< gen_double_spend_in_tx > { - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const bool has_invalid_tx = true; static const size_t expected_pool_txs_count = 0; static const uint64_t expected_bob_balance = send_amount; @@ -73,7 +74,7 @@ struct gen_double_spend_in_tx : public gen_double_spend_base< gen_double_spend_i template struct gen_double_spend_in_the_same_block : public gen_double_spend_base< gen_double_spend_in_the_same_block > { - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const bool has_invalid_tx = !txs_keeped_by_block; static const size_t expected_pool_txs_count = has_invalid_tx ? 1 : 2; static const uint64_t expected_bob_balance = send_amount; @@ -86,7 +87,7 @@ struct gen_double_spend_in_the_same_block : public gen_double_spend_base< gen_do template struct gen_double_spend_in_different_blocks : public gen_double_spend_base< gen_double_spend_in_different_blocks > { - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const bool has_invalid_tx = !txs_keeped_by_block; static const size_t expected_pool_txs_count = has_invalid_tx ? 0 : 1; static const uint64_t expected_bob_balance = 0; @@ -99,7 +100,7 @@ struct gen_double_spend_in_different_blocks : public gen_double_spend_base< gen_ template struct gen_double_spend_in_alt_chain_in_the_same_block : public gen_double_spend_base< gen_double_spend_in_alt_chain_in_the_same_block > { - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const bool has_invalid_tx = !txs_keeped_by_block; static const size_t expected_pool_txs_count = has_invalid_tx ? 1 : 2; static const uint64_t expected_bob_balance = send_amount; @@ -112,7 +113,7 @@ struct gen_double_spend_in_alt_chain_in_the_same_block : public gen_double_spend template struct gen_double_spend_in_alt_chain_in_different_blocks : public gen_double_spend_base< gen_double_spend_in_alt_chain_in_different_blocks > { - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const bool has_invalid_tx = !txs_keeped_by_block; static const size_t expected_pool_txs_count = has_invalid_tx ? 1 : 2; static const uint64_t expected_bob_balance = send_amount; @@ -125,7 +126,7 @@ struct gen_double_spend_in_alt_chain_in_different_blocks : public gen_double_spe class gen_double_spend_in_different_chains : public test_chain_unit_base { public: - static const uint64_t send_amount = MK_COINS(17); + static const uint64_t send_amount = FIRST_BLOCK_REWARD - TESTS_DEFAULT_FEE; static const size_t expected_blockchain_height = 4 + 2 * CRYPTONOTE_MINED_MONEY_UNLOCK_WINDOW; gen_double_spend_in_different_chains(); From 79beed221d9b3842b652587b5eaeed17423bd9de Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 25 Dec 2015 22:26:34 +0000 Subject: [PATCH 13/13] tests: fix various tests by using parameters better suited to monero Either smaller coin values (as monero has smaller block rewards), or pre-hard fork values (full reward zone), or post-Bytecoin values (emission speed). --- tests/core_tests/block_reward.cpp | 6 +++--- tests/core_tests/block_validation.cpp | 2 +- tests/core_tests/tx_validation.cpp | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/core_tests/block_reward.cpp b/tests/core_tests/block_reward.cpp index 789c6fff..fa06ed71 100644 --- a/tests/core_tests/block_reward.cpp +++ b/tests/core_tests/block_reward.cpp @@ -80,7 +80,7 @@ namespace generator.get_last_n_block_sizes(block_sizes, get_block_hash(blk_prev), median_block_count); size_t median = misc_utils::median(block_sizes); - median = std::max(median, static_cast(CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE)); + median = std::max(median, static_cast(CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE_V1)); transaction miner_tx; bool r = construct_miner_tx_by_size(miner_tx, get_block_height(blk_prev) + 1, generator.get_already_generated_coins(blk_prev), @@ -249,11 +249,11 @@ bool gen_block_reward::check_block_rewards(cryptonote::core& /*c*/, size_t /*ev_ DEFINE_TESTS_ERROR_CONTEXT("gen_block_reward_without_txs::check_block_rewards"); std::array blk_rewards; - blk_rewards[0] = MONEY_SUPPLY >> 18; + blk_rewards[0] = MONEY_SUPPLY >> EMISSION_SPEED_FACTOR_PER_MINUTE; uint64_t cumulative_reward = blk_rewards[0]; for (size_t i = 1; i < blk_rewards.size(); ++i) { - blk_rewards[i] = (MONEY_SUPPLY - cumulative_reward) >> 18; + blk_rewards[i] = (MONEY_SUPPLY - cumulative_reward) >> EMISSION_SPEED_FACTOR_PER_MINUTE; cumulative_reward += blk_rewards[i]; } diff --git a/tests/core_tests/block_validation.cpp b/tests/core_tests/block_validation.cpp index 8589f07b..6a16ee99 100644 --- a/tests/core_tests/block_validation.cpp +++ b/tests/core_tests/block_validation.cpp @@ -578,7 +578,7 @@ bool gen_block_invalid_binary_format::generate(std::vector& ev while (diffic < 1500); blk_last = boost::get(events.back()); - MAKE_TX(events, tx_0, miner_account, miner_account, MK_COINS(120), boost::get(events[1])); + MAKE_TX(events, tx_0, miner_account, miner_account, MK_COINS(30), boost::get(events[1])); DO_CALLBACK(events, "corrupt_blocks_boundary"); block blk_test; diff --git a/tests/core_tests/tx_validation.cpp b/tests/core_tests/tx_validation.cpp index c642f545..62672c5c 100644 --- a/tests/core_tests/tx_validation.cpp +++ b/tests/core_tests/tx_validation.cpp @@ -397,17 +397,17 @@ bool gen_tx_key_offest_points_to_foreign_key::generate(std::vector sources_bob; std::vector destinations_bob; - fill_tx_sources_and_destinations(events, blk_2, bob_account, miner_account, MK_COINS(60) + 1 - TESTS_DEFAULT_FEE, TESTS_DEFAULT_FEE, 0, sources_bob, destinations_bob); + fill_tx_sources_and_destinations(events, blk_2, bob_account, miner_account, MK_COINS(15) + 1 - TESTS_DEFAULT_FEE, TESTS_DEFAULT_FEE, 0, sources_bob, destinations_bob); std::vector sources_alice; std::vector destinations_alice; - fill_tx_sources_and_destinations(events, blk_2, alice_account, miner_account, MK_COINS(60) + 1 - TESTS_DEFAULT_FEE, TESTS_DEFAULT_FEE, 0, sources_alice, destinations_alice); + fill_tx_sources_and_destinations(events, blk_2, alice_account, miner_account, MK_COINS(15) + 1 - TESTS_DEFAULT_FEE, TESTS_DEFAULT_FEE, 0, sources_alice, destinations_alice); tx_builder builder; builder.step1_init();