From a803befcd313c3b643eaa58d5d97050f40fa7c88 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Sep 2015 13:25:22 +0100 Subject: [PATCH] hardfork: change window semantics to not count the newly added block This allows knowing the hard fork a block must obey in order to be added to the blockchain. The previous semantics would use that new block's version vote to determine this hard fork, which made it impossible to use the rules to validate transactions entering the tx pool (and made it impossible to validate a block before adding it to the blockchain). --- src/cryptonote_core/hardfork.cpp | 25 ++++++++++++++++++++----- tests/unit_tests/hardfork.cpp | 23 +++++++++++++---------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/cryptonote_core/hardfork.cpp b/src/cryptonote_core/hardfork.cpp index d2e95b58e..f4df1f9c6 100644 --- a/src/cryptonote_core/hardfork.cpp +++ b/src/cryptonote_core/hardfork.cpp @@ -43,6 +43,10 @@ HardFork::HardFork(cryptonote::BlockchainDB &db, uint8_t original_version, time_ window_size(window_size), threshold_percent(threshold_percent) { + if (window_size == 0) + throw "window_size needs to be strictly positive"; + if (threshold_percent > 100) + throw "threshold_percent needs to be between 0 and 100"; } bool HardFork::add(uint8_t version, uint64_t height, time_t time) @@ -93,6 +97,8 @@ bool HardFork::add(const cryptonote::block &block, uint64_t height) if (!do_check(block)) return false; + db.set_hard_fork_version(height, heights[current_fork_index].version); + const uint8_t version = get_effective_version(block); while (versions.size() >= window_size) { @@ -105,16 +111,15 @@ bool HardFork::add(const cryptonote::block &block, uint64_t height) last_versions[version]++; versions.push_back(version); - uint8_t voted = get_voted_fork_index(height); + uint8_t voted = get_voted_fork_index(height + 1); if (voted > current_fork_index) { for (int v = heights[current_fork_index].version + 1; v <= heights[voted].version; ++v) { - db.set_hard_fork_starting_height(v, height); + // we reached the vote threshold with this block, next one will be forked + db.set_hard_fork_starting_height(v, height + 1); } current_fork_index = voted; } - db.set_hard_fork_version(height, heights[current_fork_index].version); - return true; } @@ -159,7 +164,7 @@ bool HardFork::reorganize_from_block_height(uint64_t height) for (size_t n = 0; n < 256; ++n) last_versions[n] = 0; - const uint64_t rescan_height = height >= (window_size - 1) ? height - (window_size - 1) : 0; + const uint64_t rescan_height = height >= (window_size - 1) ? height - (window_size -1) : 0; const uint8_t start_version = height == 0 ? original_version : db.get_hard_fork_version(height); while (heights[current_fork_index].version > start_version) { db.set_hard_fork_starting_height(heights[current_fork_index].version, std::numeric_limits::max()); @@ -171,6 +176,16 @@ bool HardFork::reorganize_from_block_height(uint64_t height) last_versions[v]++; versions.push_back(v); } + + uint8_t voted = get_voted_fork_index(height + 1); + if (voted > current_fork_index) { + for (int v = heights[current_fork_index].version + 1; v <= heights[voted].version; ++v) { + // we reached the vote threshold with this block, next one will be forked + db.set_hard_fork_starting_height(v, height + 1); + } + current_fork_index = voted; + } + const uint64_t bc_height = db.height(); for (uint64_t h = height + 1; h < bc_height; ++h) { add(db.get_block_from_height(h), h); diff --git a/tests/unit_tests/hardfork.cpp b/tests/unit_tests/hardfork.cpp index 66431e7a8..1fd179950 100644 --- a/tests/unit_tests/hardfork.cpp +++ b/tests/unit_tests/hardfork.cpp @@ -236,7 +236,7 @@ TEST(steps_1, Success) } for (uint64_t h = 0; h < 10; ++h) { - ASSERT_EQ(hf.get(h), h+1); + ASSERT_EQ(hf.get(h), std::max(1,(int)h)); } } @@ -263,7 +263,7 @@ TEST(reorganize, Same) for (uint64_t rh = 0; rh < 20; ++rh) { hf.reorganize_from_block_height(rh); for (int hh = 0; hh < 20; ++hh) { - uint8_t version = hh >= (history-1) ? block_versions[hh - (history-1)] : 1; + uint8_t version = hh >= history ? block_versions[hh - history] : 1; ASSERT_EQ(hf.get(hh), version); } } @@ -283,8 +283,10 @@ TEST(reorganize, Changed) ASSERT_TRUE(hf.add(9, 6, 3)); hf.init(); + // fork 4 7 9 // index 0 1 2 3 4 5 6 7 8 9 static const uint8_t block_versions[] = { 1, 1, 4, 4, 7, 7, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9 }; + static const uint8_t expected_versions[] = { 1, 1, 1, 1, 1, 1, 4, 4, 7, 7, 9, 9, 9, 9, 9, 9 }; for (uint64_t h = 0; h < 16; ++h) { db.add_block(mkblock(block_versions[h]), 0, 0, 0, crypto::hash()); ASSERT_TRUE (hf.add(db.get_block_from_height(h), h)); @@ -293,14 +295,13 @@ TEST(reorganize, Changed) for (uint64_t rh = 0; rh < 16; ++rh) { hf.reorganize_from_block_height(rh); for (int hh = 0; hh < 16; ++hh) { - uint8_t version = hh >= (history-1) ? block_versions[hh - (history-1)] : 1; - ASSERT_EQ(hf.get(hh), version); + ASSERT_EQ(hf.get(hh), expected_versions[hh]); } } // delay a bit for 9, and go back to 1 to check it stays at 9 static const uint8_t block_versions_new[] = { 1, 1, 4, 4, 7, 7, 4, 7, 7, 7, 9, 9, 9, 9, 9, 1 }; - static const uint8_t expected_versions_new[] = { 1, 1, 1, 1, 1, 4, 4, 4, 4, 4, 7, 7, 7, 9, 9, 9 }; + static const uint8_t expected_versions_new[] = { 1, 1, 1, 1, 1, 1, 4, 4, 4, 4, 4, 7, 7, 7, 9, 9 }; for (uint64_t h = 3; h < 16; ++h) { db.remove_block(); } @@ -334,11 +335,13 @@ TEST(voting, threshold) db.add_block(mkblock(v), 0, 0, 0, crypto::hash()); bool ret = hf.add(db.get_block_from_height(h), h); if (h >= 8 && threshold == 87) { + // for threshold 87, we reach the treshold at height 7, so from height 8, hard fork to version 2, but 8 tries to add 1 ASSERT_FALSE(ret); } else { + // for threshold 88, we never reach the threshold ASSERT_TRUE(ret); - uint8_t expected = threshold == 88 ? 1 : h < 7 ? 1 : 2; + uint8_t expected = threshold == 88 ? 1 : h < 8 ? 1 : 2; ASSERT_EQ(hf.get(h), expected); } } @@ -368,7 +371,7 @@ TEST(new_blocks, denied) ASSERT_FALSE(hf.add(mkblock(1), 9)); // so this one can't get added ASSERT_TRUE(hf.add(mkblock(2), 10)); - ASSERT_EQ(hf.get_start_height(2), 8); + ASSERT_EQ(hf.get_start_height(2), 9); } TEST(new_version, early) @@ -426,8 +429,8 @@ TEST(reorganize, changed) ADD_TRUE(3, 7); ADD_TRUE(4, 8); ADD_TRUE(4, 9); - ASSERT_EQ(hf.get_start_height(2), 3); - ASSERT_EQ(hf.get_start_height(3), 8); + ASSERT_EQ(hf.get_start_height(2), 4); // reaches threshold 2 at height 3, so height 4 forks + ASSERT_EQ(hf.get_start_height(3), 9); ASSERT_EQ(hf.get_current_version(), 3); // pop a few blocks and check current version goes back down @@ -444,7 +447,7 @@ TEST(reorganize, changed) ADD_TRUE(2, 7); ADD_TRUE(2, 8); ADD_TRUE(2, 9); - ASSERT_EQ(hf.get_start_height(2), 3); // unchanged + ASSERT_EQ(hf.get_start_height(2), 4); // unchanged ASSERT_EQ(hf.get_current_version(), 2); // we did not bump to 3 this time ASSERT_EQ(hf.get_start_height(3), std::numeric_limits::max()); // not yet }