diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 26157a37a..2e91105bb 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -1554,7 +1554,7 @@ bool Blockchain::build_alt_chain(const crypto::hash &prev_id, std::vector ×tamps, block_verification_context &bvc, int *num_alt_checkpoints, - int *num_checkpoints) const + int *num_checkpoints) { //build alternative subchain, front -> mainchain, back -> alternative head cryptonote::alt_block_data_t data; @@ -1623,8 +1623,9 @@ bool Blockchain::build_alt_chain(const crypto::hash &prev_id, if(!alt_chain.empty()) { bool failed = false; + uint64_t blockchain_height = m_db->height(); // make sure alt chain doesn't somehow start past the end of the main chain - if (m_db->height() < alt_chain.front().height) + if (blockchain_height < alt_chain.front().height) { LOG_PRINT_L1("main blockchain wrong height: " << m_db->height() << ", alt_chain: " << alt_chain.front().height); failed = true; @@ -1646,9 +1647,16 @@ bool Blockchain::build_alt_chain(const crypto::hash &prev_id, failed = true; } + if (!failed && !m_checkpoints.is_alternative_block_allowed(blockchain_height, alt_chain.front().height, nullptr /*service_node_checkpoint*/)) + { + LOG_PRINT_L2("alternative chain is too old to consider: " << h); + failed = true; + } + if (failed) { // Cleanup alt chain, it's invalid + bvc.m_verifivation_failed = true; for (auto const &bei : alt_chain) m_db->remove_alt_block(cryptonote::get_block_hash(bei.bl)); diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 79540079d..0ad294053 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -1275,7 +1275,7 @@ namespace cryptonote * * @return true on success, false otherwise */ - bool build_alt_chain(const crypto::hash &prev_id, std::list& alt_chain, std::vector ×tamps, block_verification_context& bvc, int *num_alt_checkpoints, int *num_checkpoints) const; + bool build_alt_chain(const crypto::hash &prev_id, std::list& alt_chain, std::vector ×tamps, block_verification_context& bvc, int *num_alt_checkpoints, int *num_checkpoints); /** * @brief gets the difficulty requirement for a new block on an alternate chain diff --git a/tests/core_tests/chaingen.cpp b/tests/core_tests/chaingen.cpp index 89ff2e565..032d948a7 100644 --- a/tests/core_tests/chaingen.cpp +++ b/tests/core_tests/chaingen.cpp @@ -481,13 +481,13 @@ static void fill_nonce(cryptonote::block& blk, const cryptonote::difficulty_type loki_blockchain_entry loki_chain_generator::create_genesis_block(const cryptonote::account_base &miner, uint64_t timestamp) { - uint64_t height = 0; + uint64_t height = 0; loki_blockchain_entry result = {}; - cryptonote::block &blk = result.block; - blk.major_version = hf_version_; - blk.minor_version = hf_version_; - blk.timestamp = timestamp; - blk.prev_id = crypto::null_hash; + cryptonote::block &blk = result.block; + blk.major_version = hf_version_; + blk.minor_version = hf_version_; + blk.timestamp = timestamp; + blk.prev_id = crypto::null_hash; // TODO(doyle): Does this evaluate to 0? If so we can simplify this a lot more size_t target_block_weight = get_transaction_weight(blk.miner_tx); diff --git a/tests/core_tests/chaingen.h b/tests/core_tests/chaingen.h index acb6e8855..dea6f2334 100644 --- a/tests/core_tests/chaingen.h +++ b/tests/core_tests/chaingen.h @@ -1327,6 +1327,7 @@ public: bool build() { + m_tx_params.set_hf_version(m_hf_version); m_finished = true; std::vector sources; diff --git a/tests/core_tests/chaingen_main.cpp b/tests/core_tests/chaingen_main.cpp index 65b06cd01..b7c3de6b9 100644 --- a/tests/core_tests/chaingen_main.cpp +++ b/tests/core_tests/chaingen_main.cpp @@ -110,6 +110,7 @@ int main(int argc, char* argv[]) GENERATE_AND_PLAY(loki_checkpointing_alt_chain_handle_alt_blocks_at_tip); GENERATE_AND_PLAY(loki_checkpointing_alt_chain_more_service_node_checkpoints_less_pow_overtakes); GENERATE_AND_PLAY(loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back); + GENERATE_AND_PLAY(loki_checkpointing_alt_chain_too_old_should_be_dropped); GENERATE_AND_PLAY(loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints); GENERATE_AND_PLAY(loki_checkpointing_service_node_checkpoint_from_votes); GENERATE_AND_PLAY(loki_checkpointing_service_node_checkpoints_check_reorg_windows); diff --git a/tests/core_tests/loki_tests.cpp b/tests/core_tests/loki_tests.cpp index 12b99f3e2..756e82702 100644 --- a/tests/core_tests/loki_tests.cpp +++ b/tests/core_tests/loki_tests.cpp @@ -213,26 +213,13 @@ bool loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back::ge fork.create_and_add_next_block(); } - uint64_t second_checkpointed_height = fork.height(); - uint64_t second_checkpointed_height_hf = fork.top().block.major_version; - crypto::hash second_checkpointed_hash = cryptonote::get_block_hash(fork.top().block); - std::shared_ptr second_quorum = fork.get_quorum(service_nodes::quorum_type::checkpointing, gen.height()); - // NOTE: Fork generates service node votes, upon sending them over and the // main chain collecting them validly (they should be able to verify // signatures because we store alt quorums) it should generate a checkpoint // belonging to the forked chain- which should cause it to detach back to the // checkpoint height - // First send the votes for the newest checkpoint, we should reorg back halfway - for (size_t i = 0; i < service_nodes::CHECKPOINT_MIN_VOTES; i++) - { - auto keys = gen.get_cached_keys(second_quorum->validators[i]); - service_nodes::quorum_vote_t fork_vote = service_nodes::make_checkpointing_vote(second_checkpointed_height_hf, second_checkpointed_hash, second_checkpointed_height, i, keys); - events.push_back(loki_blockchain_addable(fork_vote, true/*can_be_added_to_blockchain*/, "A second_checkpoint vote from the forked chain should be accepted since we should be storing alternative service node states and quorums")); - } - - // Then we send the votes for the next newest checkpoint, we should reorg back to our forking point + // Then we send the votes for the 2nd newest checkpoint. We don't reorg back until we receive a block confirming this checkpoint. for (size_t i = 0; i < service_nodes::CHECKPOINT_MIN_VOTES; i++) { auto keys = gen.get_cached_keys(first_quorum->validators[i]); @@ -255,6 +242,50 @@ bool loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back::ge return true; } +bool loki_checkpointing_alt_chain_too_old_should_be_dropped::generate(std::vector &events) +{ + std::vector> hard_forks = loki_generate_sequential_hard_fork_table(); + loki_chain_generator gen(events, hard_forks); + gen.add_blocks_until_version(hard_forks.back().first); + gen.add_n_blocks(40); + gen.add_mined_money_unlock_blocks(); + + int constexpr NUM_SERVICE_NODES = service_nodes::CHECKPOINT_QUORUM_SIZE; + std::vector registration_txs(NUM_SERVICE_NODES); + for (auto i = 0u; i < NUM_SERVICE_NODES; ++i) + registration_txs[i] = gen.create_and_add_registration_tx(gen.first_miner()); + gen.create_and_add_next_block(registration_txs); + + // NOTE: Add blocks until we get to the first height that has a checkpointing quorum AND there are service nodes in the quorum. + int const MAX_TRIES = 16; + int tries = 0; + for (; tries < MAX_TRIES; tries++) + { + gen.add_blocks_until_next_checkpointable_height(); + std::shared_ptr quorum = gen.get_quorum(service_nodes::quorum_type::checkpointing, gen.height()); + if (quorum && quorum->validators.size()) break; + } + assert(tries != MAX_TRIES); + + loki_chain_generator fork = gen; + gen.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); + gen.add_blocks_until_next_checkpointable_height(); + fork.add_blocks_until_next_checkpointable_height(); + + gen.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); + gen.add_blocks_until_next_checkpointable_height(); + fork.add_blocks_until_next_checkpointable_height(); + + gen.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); + gen.create_and_add_next_block(); + + // NOTE: We now have 3 checkpoints. Extending this alt-chain is no longer + // possible because this alt-chain starts before the immutable height, it + // should be deleted and removed. + fork.create_and_add_next_block({}, nullptr, false, "Can not add block to alt chain because the alt chain starts before the immutable height. Those blocks should be locked into the chain"); + return true; +} + // NOTE: - Checks that an alt chain eventually takes over the main chain with // only 1 checkpoint, by progressively adding 2 more checkpoints at the next // available checkpoint heights whilst maintaining equal heights with the main chain @@ -289,20 +320,17 @@ bool loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints::gene // Setup the two chains as follows, where C = checkpointed block, B = normal // block, the main chain should NOT reorg to the fork chain as they have the // same PoW-ish and equal number of checkpoints. - // Main chain - C B B B B - // Fork chain - B B B B C + // Main chain C B B B B + // Fork chain B B B B C + loki_chain_generator fork = gen; gen.create_and_add_next_block(); gen.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); - fork.create_and_add_next_block(); - - gen.add_n_blocks(service_nodes::CHECKPOINT_INTERVAL); - gen.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); - - fork.add_n_blocks(service_nodes::CHECKPOINT_INTERVAL); - fork.add_service_node_checkpoint(gen.height(), service_nodes::CHECKPOINT_MIN_VOTES); + gen.add_blocks_until_next_checkpointable_height(); + fork.add_blocks_until_next_checkpointable_height(); fork.add_service_node_checkpoint(fork.height(), service_nodes::CHECKPOINT_MIN_VOTES); + crypto::hash const gen_top_hash = cryptonote::get_block_hash(gen.top().block); loki_register_callback(events, "check_still_on_main_chain", [&events, gen_top_hash](cryptonote::core &c, size_t ev_index) { @@ -315,8 +343,8 @@ bool loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints::gene }); // Now create the following chain, the fork chain should be switched to due to now having more checkpoints - // Main chain - C B B B B | B B B B - // Fork chain - B B B B C | B B B C + // Main chain C B B B B | B B B B B + // Fork chain B B B B C | B B B C gen.add_blocks_until_next_checkpointable_height(); gen.create_and_add_next_block(); diff --git a/tests/core_tests/loki_tests.h b/tests/core_tests/loki_tests.h index 5e25d3b49..352acb674 100644 --- a/tests/core_tests/loki_tests.h +++ b/tests/core_tests/loki_tests.h @@ -38,6 +38,7 @@ struct loki_checkpointing_alt_chain_handle_alt_blocks_at_tip : public test_chain_unit_base { bool generate(std::vector& events); }; struct loki_checkpointing_alt_chain_more_service_node_checkpoints_less_pow_overtakes : public test_chain_unit_base { bool generate(std::vector& events); }; struct loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back : public test_chain_unit_base { bool generate(std::vector& events); }; +struct loki_checkpointing_alt_chain_too_old_should_be_dropped : public test_chain_unit_base { bool generate(std::vector& events); }; struct loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints : public test_chain_unit_base { bool generate(std::vector& events); }; struct loki_checkpointing_service_node_checkpoint_from_votes : public test_chain_unit_base { bool generate(std::vector& events); }; struct loki_checkpointing_service_node_checkpoints_check_reorg_windows : public test_chain_unit_base { bool generate(std::vector& events); };