hardfork: fix more major/minor issues

Also add some more tests, and rename some instances of
"version" and "add" for clarity.

NOTE: the starting height values are sometimes wrong.
I suspect this is due to the hard fork reorg code being
buggy, since they're good when syncing after the fact.
However, they're not actually used by the consensus code,
so I'm ignoring this for now, but this needs debugging.
This commit is contained in:
moneromooo-monero 2015-11-24 20:46:10 +00:00
parent 3b47ca2d7d
commit d887c18e33
No known key found for this signature in database
GPG key ID: 686F07454D6CEFC3
4 changed files with 83 additions and 68 deletions

View file

@ -257,13 +257,13 @@ bool Blockchain::init(BlockchainDB* db, const bool testnet)
if (testnet) { if (testnet) {
m_hardfork = new HardFork(*db, 1, testnet_hard_fork_version_1_till); m_hardfork = new HardFork(*db, 1, testnet_hard_fork_version_1_till);
for (size_t n = 0; n < sizeof(testnet_hard_forks) / sizeof(testnet_hard_forks[0]); ++n) for (size_t n = 0; n < sizeof(testnet_hard_forks) / sizeof(testnet_hard_forks[0]); ++n)
m_hardfork->add(testnet_hard_forks[n].version, testnet_hard_forks[n].height, testnet_hard_forks[n].threshold, testnet_hard_forks[n].time); m_hardfork->add_fork(testnet_hard_forks[n].version, testnet_hard_forks[n].height, testnet_hard_forks[n].threshold, testnet_hard_forks[n].time);
} }
else else
{ {
m_hardfork = new HardFork(*db, 1, mainnet_hard_fork_version_1_till); m_hardfork = new HardFork(*db, 1, mainnet_hard_fork_version_1_till);
for (size_t n = 0; n < sizeof(mainnet_hard_forks) / sizeof(mainnet_hard_forks[0]); ++n) for (size_t n = 0; n < sizeof(mainnet_hard_forks) / sizeof(mainnet_hard_forks[0]); ++n)
m_hardfork->add(mainnet_hard_forks[n].version, mainnet_hard_forks[n].height, mainnet_hard_forks[n].threshold, mainnet_hard_forks[n].time); m_hardfork->add_fork(mainnet_hard_forks[n].version, mainnet_hard_forks[n].height, mainnet_hard_forks[n].threshold, mainnet_hard_forks[n].time);
} }
m_hardfork->init(); m_hardfork->init();

View file

@ -66,7 +66,7 @@ HardFork::HardFork(cryptonote::BlockchainDB &db, uint8_t original_version, uint6
throw "default_threshold_percent needs to be between 0 and 100"; throw "default_threshold_percent needs to be between 0 and 100";
} }
bool HardFork::add(uint8_t version, uint64_t height, uint8_t threshold, time_t time) bool HardFork::add_fork(uint8_t version, uint64_t height, uint8_t threshold, time_t time)
{ {
CRITICAL_REGION_LOCAL(lock); CRITICAL_REGION_LOCAL(lock);
@ -87,42 +87,43 @@ bool HardFork::add(uint8_t version, uint64_t height, uint8_t threshold, time_t t
return true; return true;
} }
bool HardFork::add(uint8_t version, uint64_t height, time_t time) bool HardFork::add_fork(uint8_t version, uint64_t height, time_t time)
{ {
return add(version, height, default_threshold_percent, time); return add_fork(version, height, default_threshold_percent, time);
} }
uint8_t HardFork::get_effective_version(uint8_t version) const uint8_t HardFork::get_effective_version(uint8_t voting_version) const
{ {
if (!heights.empty()) { if (!heights.empty()) {
uint8_t max_version = heights.back().version; uint8_t max_version = heights.back().version;
if (version > max_version) if (voting_version > max_version)
version = max_version; voting_version = max_version;
} }
return version; return voting_version;
} }
bool HardFork::do_check(uint8_t version) const bool HardFork::do_check(uint8_t block_version, uint8_t voting_version) const
{ {
return version >= heights[current_fork_index].version; return block_version >= heights[current_fork_index].version
&& voting_version >= heights[current_fork_index].version;
} }
bool HardFork::check(const cryptonote::block &block) const bool HardFork::check(const cryptonote::block &block) const
{ {
CRITICAL_REGION_LOCAL(lock); CRITICAL_REGION_LOCAL(lock);
return do_check(::get_block_version(block)); return do_check(::get_block_version(block), ::get_block_vote(block));
} }
bool HardFork::add(uint8_t block_version, uint64_t height) bool HardFork::add(uint8_t block_version, uint8_t voting_version, uint64_t height)
{ {
CRITICAL_REGION_LOCAL(lock); CRITICAL_REGION_LOCAL(lock);
if (!do_check(block_version)) if (!do_check(block_version, voting_version))
return false; return false;
db.set_hard_fork_version(height, heights[current_fork_index].version); db.set_hard_fork_version(height, heights[current_fork_index].version);
const uint8_t version = get_effective_version(block_version); voting_version = get_effective_version(voting_version);
while (versions.size() >= window_size) { while (versions.size() >= window_size) {
const uint8_t old_version = versions.front(); const uint8_t old_version = versions.front();
@ -131,8 +132,8 @@ bool HardFork::add(uint8_t block_version, uint64_t height)
versions.pop_front(); versions.pop_front();
} }
last_versions[version]++; last_versions[voting_version]++;
versions.push_back(version); versions.push_back(voting_version);
uint8_t voted = get_voted_fork_index(height + 1); uint8_t voted = get_voted_fork_index(height + 1);
if (voted > current_fork_index) { if (voted > current_fork_index) {
@ -148,7 +149,7 @@ bool HardFork::add(uint8_t block_version, uint64_t height)
bool HardFork::add(const cryptonote::block &block, uint64_t height) bool HardFork::add(const cryptonote::block &block, uint64_t height)
{ {
return add(::get_block_version(block), height); return add(::get_block_version(block), ::get_block_vote(block), height);
} }
void HardFork::init() void HardFork::init()
@ -230,7 +231,7 @@ bool HardFork::reorganize_from_block_height(uint64_t height)
const uint64_t bc_height = db.height(); const uint64_t bc_height = db.height();
for (uint64_t h = height + 1; h < bc_height; ++h) { for (uint64_t h = height + 1; h < bc_height; ++h) {
add(get_block_version(h), h); add(db.get_block_from_height(h), h);
} }
db.batch_stop(); db.batch_stop();

View file

@ -71,7 +71,7 @@ namespace cryptonote
* @param threshold The threshold of votes needed for this fork (0-100) * @param threshold The threshold of votes needed for this fork (0-100)
* @param time Approximate time of the hardfork (seconds since epoch) * @param time Approximate time of the hardfork (seconds since epoch)
*/ */
bool add(uint8_t version, uint64_t height, uint8_t threshold, time_t time); bool add_fork(uint8_t version, uint64_t height, uint8_t threshold, time_t time);
/** /**
* @brief add a new hardfork height * @brief add a new hardfork height
@ -79,10 +79,11 @@ namespace cryptonote
* returns true if no error, false otherwise * returns true if no error, false otherwise
* *
* @param version the major block version for the fork * @param version the major block version for the fork
* @param voting_version the minor block version for the fork, used for voting
* @param height The height the hardfork takes effect * @param height The height the hardfork takes effect
* @param time Approximate time of the hardfork (seconds since epoch) * @param time Approximate time of the hardfork (seconds since epoch)
*/ */
bool add(uint8_t version, uint64_t height, time_t time); bool add_fork(uint8_t version, uint64_t height, time_t time);
/** /**
* @brief initialize the object * @brief initialize the object
@ -203,10 +204,10 @@ namespace cryptonote
private: private:
uint8_t get_block_version(uint64_t height) const; uint8_t get_block_version(uint64_t height) const;
bool do_check(uint8_t version) const; bool do_check(uint8_t block_version, uint8_t voting_version) const;
int get_voted_fork_index(uint64_t height) const; int get_voted_fork_index(uint64_t height) const;
uint8_t get_effective_version(uint8_t version) const; uint8_t get_effective_version(uint8_t voting_version) const;
bool add(uint8_t block_version, uint64_t height); bool add(uint8_t block_version, uint8_t voting_version, uint64_t height);
bool rescan_from_block_height(uint64_t height); bool rescan_from_block_height(uint64_t height);
bool rescan_from_chain_height(uint64_t height); bool rescan_from_chain_height(uint64_t height);

View file

@ -135,6 +135,7 @@ private:
static cryptonote::block mkblock(uint8_t version) static cryptonote::block mkblock(uint8_t version)
{ {
cryptonote::block b; cryptonote::block b;
b.major_version = version;
b.minor_version = version; b.minor_version = version;
return b; return b;
} }
@ -144,7 +145,7 @@ TEST(empty_hardforks, Success)
TestDB db; TestDB db;
HardFork hf(db); HardFork hf(db);
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
hf.init(); hf.init();
ASSERT_TRUE(hf.get_state(time(NULL)) == HardFork::Ready); ASSERT_TRUE(hf.get_state(time(NULL)) == HardFork::Ready);
ASSERT_TRUE(hf.get_state(time(NULL) + 3600*24*400) == HardFork::Ready); ASSERT_TRUE(hf.get_state(time(NULL) + 3600*24*400) == HardFork::Ready);
@ -163,13 +164,13 @@ TEST(ordering, Success)
TestDB db; TestDB db;
HardFork hf(db); HardFork hf(db);
ASSERT_TRUE(hf.add(2, 2, 1)); ASSERT_TRUE(hf.add_fork(2, 2, 1));
ASSERT_FALSE(hf.add(3, 3, 1)); ASSERT_FALSE(hf.add_fork(3, 3, 1));
ASSERT_FALSE(hf.add(3, 2, 2)); ASSERT_FALSE(hf.add_fork(3, 2, 2));
ASSERT_FALSE(hf.add(2, 3, 2)); ASSERT_FALSE(hf.add_fork(2, 3, 2));
ASSERT_TRUE(hf.add(3, 10, 2)); ASSERT_TRUE(hf.add_fork(3, 10, 2));
ASSERT_TRUE(hf.add(4, 20, 3)); ASSERT_TRUE(hf.add_fork(4, 20, 3));
ASSERT_FALSE(hf.add(5, 5, 4)); ASSERT_FALSE(hf.add_fork(5, 5, 4));
} }
TEST(states, Success) TEST(states, Success)
@ -177,8 +178,8 @@ TEST(states, Success)
TestDB db; TestDB db;
HardFork hf(db); HardFork hf(db);
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, BLOCKS_PER_YEAR, SECONDS_PER_YEAR)); ASSERT_TRUE(hf.add_fork(2, BLOCKS_PER_YEAR, SECONDS_PER_YEAR));
ASSERT_TRUE(hf.get_state(0) == HardFork::Ready); ASSERT_TRUE(hf.get_state(0) == HardFork::Ready);
ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR / 2) == HardFork::Ready); ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR / 2) == HardFork::Ready);
@ -186,7 +187,7 @@ TEST(states, Success)
ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR + (HardFork::DEFAULT_UPDATE_TIME + HardFork::DEFAULT_FORKED_TIME) / 2) == HardFork::UpdateNeeded); ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR + (HardFork::DEFAULT_UPDATE_TIME + HardFork::DEFAULT_FORKED_TIME) / 2) == HardFork::UpdateNeeded);
ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR + HardFork::DEFAULT_FORKED_TIME * 2) == HardFork::LikelyForked); ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR + HardFork::DEFAULT_FORKED_TIME * 2) == HardFork::LikelyForked);
ASSERT_TRUE(hf.add(3, BLOCKS_PER_YEAR * 5, SECONDS_PER_YEAR * 5)); ASSERT_TRUE(hf.add_fork(3, BLOCKS_PER_YEAR * 5, SECONDS_PER_YEAR * 5));
ASSERT_TRUE(hf.get_state(0) == HardFork::Ready); ASSERT_TRUE(hf.get_state(0) == HardFork::Ready);
ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR / 2) == HardFork::Ready); ASSERT_TRUE(hf.get_state(SECONDS_PER_YEAR / 2) == HardFork::Ready);
@ -201,10 +202,10 @@ TEST(steps_asap, Success)
HardFork hf(db, 1,0,1,1,1); HardFork hf(db, 1,0,1,1,1);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(4, 2, 1)); ASSERT_TRUE(hf.add_fork(4, 2, 1));
ASSERT_TRUE(hf.add(7, 4, 2)); ASSERT_TRUE(hf.add_fork(7, 4, 2));
ASSERT_TRUE(hf.add(9, 6, 3)); ASSERT_TRUE(hf.add_fork(9, 6, 3));
hf.init(); hf.init();
for (uint64_t h = 0; h < 10; ++h) { for (uint64_t h = 0; h < 10; ++h) {
@ -229,9 +230,9 @@ TEST(steps_1, Success)
TestDB db; TestDB db;
HardFork hf(db, 1,0,1,1,1); HardFork hf(db, 1,0,1,1,1);
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
for (int n = 1 ; n < 10; ++n) for (int n = 1 ; n < 10; ++n)
ASSERT_TRUE(hf.add(n+1, n, n)); ASSERT_TRUE(hf.add_fork(n+1, n, n));
hf.init(); hf.init();
for (uint64_t h = 0 ; h < 10; ++h) { for (uint64_t h = 0 ; h < 10; ++h) {
@ -251,10 +252,10 @@ TEST(reorganize, Same)
HardFork hf(db, 1, 0, 1, 1, history, 100); HardFork hf(db, 1, 0, 1, 1, history, 100);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(4, 2, 1)); ASSERT_TRUE(hf.add_fork(4, 2, 1));
ASSERT_TRUE(hf.add(7, 4, 2)); ASSERT_TRUE(hf.add_fork(7, 4, 2));
ASSERT_TRUE(hf.add(9, 6, 3)); ASSERT_TRUE(hf.add_fork(9, 6, 3));
hf.init(); hf.init();
// index 0 1 2 3 4 5 6 7 8 9 // index 0 1 2 3 4 5 6 7 8 9
@ -270,6 +271,10 @@ TEST(reorganize, Same)
uint8_t version = hh >= history ? block_versions[hh - history] : 1; uint8_t version = hh >= history ? block_versions[hh - history] : 1;
ASSERT_EQ(hf.get(hh), version); ASSERT_EQ(hf.get(hh), version);
} }
ASSERT_EQ(hf.get_start_height(1), 0);
ASSERT_EQ(hf.get_start_height(4), 2 + history);
ASSERT_EQ(hf.get_start_height(7), 4 + history);
ASSERT_EQ(hf.get_start_height(9), 6 + history);
} }
} }
} }
@ -281,15 +286,15 @@ TEST(reorganize, Changed)
HardFork hf(db, 1, 0, 1, 1, 4, 100); HardFork hf(db, 1, 0, 1, 1, 4, 100);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(4, 2, 1)); ASSERT_TRUE(hf.add_fork(4, 2, 1));
ASSERT_TRUE(hf.add(7, 4, 2)); ASSERT_TRUE(hf.add_fork(7, 4, 2));
ASSERT_TRUE(hf.add(9, 6, 3)); ASSERT_TRUE(hf.add_fork(9, 6, 3));
hf.init(); hf.init();
// fork 4 7 9 // fork 4 7 9
// index 0 1 2 3 4 5 6 7 8 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 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 }; 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) { for (uint64_t h = 0; h < 16; ++h) {
db.add_block(mkblock(block_versions[h]), 0, 0, 0, crypto::hash()); db.add_block(mkblock(block_versions[h]), 0, 0, 0, crypto::hash());
@ -301,6 +306,10 @@ TEST(reorganize, Changed)
for (int hh = 0; hh < 16; ++hh) { for (int hh = 0; hh < 16; ++hh) {
ASSERT_EQ(hf.get(hh), expected_versions[hh]); ASSERT_EQ(hf.get(hh), expected_versions[hh]);
} }
ASSERT_EQ(hf.get_start_height(1), 0);
ASSERT_EQ(hf.get_start_height(4), 6);
ASSERT_EQ(hf.get_start_height(7), 8);
ASSERT_EQ(hf.get_start_height(9), 10);
} }
// delay a bit for 9, and go back to 1 to check it stays at 9 // delay a bit for 9, and go back to 1 to check it stays at 9
@ -321,6 +330,10 @@ TEST(reorganize, Changed)
for (int hh = 0; hh < 15; ++hh) { for (int hh = 0; hh < 15; ++hh) {
ASSERT_EQ(hf.get(hh), expected_versions_new[hh]); ASSERT_EQ(hf.get(hh), expected_versions_new[hh]);
} }
ASSERT_EQ(hf.get_start_height(1), 0);
ASSERT_EQ(hf.get_start_height(4), 6);
ASSERT_EQ(hf.get_start_height(7), 11);
ASSERT_EQ(hf.get_start_height(9), 14);
} }
TEST(voting, threshold) TEST(voting, threshold)
@ -330,8 +343,8 @@ TEST(voting, threshold)
HardFork hf(db, 1, 0, 1, 1, 8, threshold); HardFork hf(db, 1, 0, 1, 1, 8, threshold);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 2, 1)); ASSERT_TRUE(hf.add_fork(2, 2, 1));
hf.init(); hf.init();
for (uint64_t h = 0; h <= 8; ++h) { for (uint64_t h = 0; h <= 8; ++h) {
@ -359,10 +372,10 @@ TEST(voting, different_thresholds)
HardFork hf(db, 1, 0, 1, 1, 4, 50); // window size 4 HardFork hf(db, 1, 0, 1, 1, 4, 50); // window size 4
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 5, 0, 1)); // asap ASSERT_TRUE(hf.add_fork(2, 5, 0, 1)); // asap
ASSERT_TRUE(hf.add(3, 10, 100, 2)); // all votes ASSERT_TRUE(hf.add_fork(3, 10, 100, 2)); // all votes
ASSERT_TRUE(hf.add(4, 15, 3)); // default 50% votes ASSERT_TRUE(hf.add_fork(4, 15, 3)); // default 50% votes
hf.init(); hf.init();
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9
@ -386,8 +399,8 @@ TEST(new_blocks, denied)
HardFork hf(db, 1, 0, 1, 1, 4, 50); HardFork hf(db, 1, 0, 1, 1, 4, 50);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 2, 1)); ASSERT_TRUE(hf.add_fork(2, 2, 1));
hf.init(); hf.init();
ASSERT_TRUE(hf.add(mkblock(1), 0)); ASSERT_TRUE(hf.add(mkblock(1), 0));
@ -411,8 +424,8 @@ TEST(new_version, early)
HardFork hf(db, 1, 0, 1, 1, 4, 50); HardFork hf(db, 1, 0, 1, 1, 4, 50);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 4, 1)); ASSERT_TRUE(hf.add_fork(2, 4, 1));
hf.init(); hf.init();
ASSERT_TRUE(hf.add(mkblock(2), 0)); ASSERT_TRUE(hf.add(mkblock(2), 0));
@ -433,9 +446,9 @@ TEST(reorganize, changed)
HardFork hf(db, 1, 0, 1, 1, 4, 50); HardFork hf(db, 1, 0, 1, 1, 4, 50);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 2, 1)); ASSERT_TRUE(hf.add_fork(2, 2, 1));
ASSERT_TRUE(hf.add(3, 5, 2)); ASSERT_TRUE(hf.add_fork(3, 5, 2));
hf.init(); hf.init();
#define ADD(v, h, a) \ #define ADD(v, h, a) \
@ -487,9 +500,9 @@ TEST(get, higher)
HardFork hf(db, 1, 0, 1, 1, 4, 50); HardFork hf(db, 1, 0, 1, 1, 4, 50);
// v h t // v h t
ASSERT_TRUE(hf.add(1, 0, 0)); ASSERT_TRUE(hf.add_fork(1, 0, 0));
ASSERT_TRUE(hf.add(2, 2, 1)); ASSERT_TRUE(hf.add_fork(2, 2, 1));
ASSERT_TRUE(hf.add(3, 5, 2)); ASSERT_TRUE(hf.add_fork(3, 5, 2));
hf.init(); hf.init();
ASSERT_EQ(hf.get_ideal_version(0), 1); ASSERT_EQ(hf.get_ideal_version(0), 1);