oxen-io
/
oxen-core
mirror of https://github.com/oxen-io/oxen-core.git
2
1
Fork 0
Code Issues Projects Releases Wiki Activity

Filter vote age before querying quorum (#777) 

* Reject new votes received before daemon realises it is syncing

* Fix unit tests and print_vote_* warning
This commit is contained in:
Doyle 2019-08-07 10:52:32 +10:00 committed by GitHub
parent 40be6e58c9
commit 0f7133044c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 120 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/cryptonote_basic/cryptonote_format_utils.cpp
View File

@ -1235,9 +1235,17 @@ namespace cryptonote
{
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, "Incorrect voting group specified");
if (vote)
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, ": %s", (vote->group == service_nodes::quorum_group::validator) ? "validator" : "worker");
{
if (vote->group == service_nodes::quorum_group::validator)
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, ": %s", "validator");
else if (vote->group == service_nodes::quorum_group::worker)
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, ": %s", "worker");
else
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, ": %d", (uint8_t)vote->group);
}
bufPtr += snprintf(bufPtr, bufEnd - bufPtr, ", ");
}
if (vvc.m_invalid_vote_type) bufPtr += snprintf(bufPtr, bufEnd - bufPtr, "Vote type has invalid value: %s, ", vote ? std::to_string((uint8_t)vote->type).c_str() : "??");
if (bufPtr != buf)
{

2
src/cryptonote_basic/verification_context.h
View File

@ -47,6 +47,7 @@ namespace cryptonote
bool m_added_to_pool;
bool m_not_enough_votes;
bool m_incorrect_voting_group;
bool m_invalid_vote_type;
BEGIN_KV_SERIALIZE_MAP()
KV_SERIALIZE(m_verification_failed)
@ -58,6 +59,7 @@ namespace cryptonote
KV_SERIALIZE(m_added_to_pool)
KV_SERIALIZE(m_not_enough_votes)
KV_SERIALIZE(m_incorrect_voting_group)
KV_SERIALIZE(m_invalid_vote_type)
END_KV_SERIALIZE_MAP()
};

15
src/cryptonote_core/service_node_quorum_cop.cpp
View File

@ -425,22 +425,25 @@ namespace service_nodes
return true;
}
uint64_t const latest_height = std::max(m_core.get_current_blockchain_height(), m_core.get_target_blockchain_height());
if (!verify_vote_age(vote, latest_height, vvc))
return false;
std::shared_ptr<const testing_quorum> quorum = m_core.get_testing_quorum(vote.type, vote.block_height);
if (!quorum)
{
// TODO(loki): Fatal error
vvc.m_invalid_block_height = true;
LOG_ERROR("Quorum state for vote height " << vote.block_height << " was not cached in daemon!");
return false;
}
uint64_t latest_height = std::max(m_core.get_current_blockchain_height(), m_core.get_target_blockchain_height());
std::vector<pool_vote_entry> votes = m_vote_pool.add_pool_vote_if_unique(latest_height, vote, vvc, *quorum);
bool result = !vvc.m_verification_failed;
if (!verify_vote_against_quorum(vote, vvc, *quorum))
return false;
std::vector<pool_vote_entry> votes = m_vote_pool.add_pool_vote_if_unique(vote, vvc);
if (!vvc.m_added_to_pool) // NOTE: Not unique vote
return result;
return true;
bool result = true;
switch(vote.type)
{
default:

163
src/cryptonote_core/service_node_voting.cpp
View File

@ -316,12 +316,53 @@ namespace service_nodes
return result;
}
bool verify_vote(const quorum_vote_t& vote, uint64_t latest_height, cryptonote::vote_verification_context &vvc, const service_nodes::testing_quorum &quorum)
bool verify_vote_age(const quorum_vote_t& vote, uint64_t latest_height, cryptonote::vote_verification_context &vvc)
{
bool result = true;
bool height_in_buffer = false;
if (latest_height > vote.block_height + VOTE_LIFETIME)
{
height_in_buffer = latest_height <= vote.block_height + (VOTE_LIFETIME + VERIFY_HEIGHT_BUFFER);
LOG_PRINT_L1("Received vote for height: " << vote.block_height << ", is older than: " << VOTE_LIFETIME
<< " blocks and has been rejected.");
vvc.m_invalid_block_height = true;
}
else if (vote.block_height > latest_height)
{
height_in_buffer = vote.block_height <= latest_height + VERIFY_HEIGHT_BUFFER;
LOG_PRINT_L1("Received vote for height: " << vote.block_height << ", is newer than: " << latest_height
<< " (latest block height) and has been rejected.");
vvc.m_invalid_block_height = true;
}
if (vvc.m_invalid_block_height)
{
vvc.m_verification_failed = !height_in_buffer;
result = false;
}
return result;
}
bool verify_vote_against_quorum(const quorum_vote_t &vote, cryptonote::vote_verification_context &vvc, const service_nodes::testing_quorum &quorum)
{
bool result = true;
if (vote.group == quorum_group::invalid)
if (vote.type >= quorum_type::count)
{
vvc.m_invalid_vote_type = true;
result = false;
else if (vote.group == quorum_group::validator)
}
if (vote.group > quorum_group::worker || vote.group < quorum_group::validator)
{
vvc.m_incorrect_voting_group = true;
result = false;
}
if (!result)
return result;
if (vote.group == quorum_group::validator)
result = bounds_check_validator_index(quorum, vote.index_in_group, &vvc);
else
result = bounds_check_worker_index(quorum, vote.index_in_group, &vvc);
@ -329,92 +370,59 @@ namespace service_nodes
if (!result)
return result;
//
// NOTE: Validate vote age
//
crypto::public_key key = crypto::null_pkey;
crypto::hash hash = crypto::null_hash;
switch(vote.type)
{
bool height_in_buffer = false;
if (latest_height > vote.block_height + VOTE_LIFETIME)
default:
{
height_in_buffer = latest_height <= vote.block_height + (VOTE_LIFETIME + VERIFY_HEIGHT_BUFFER);
LOG_PRINT_L1("Received vote for height: " << vote.block_height << ", is older than: " << VOTE_LIFETIME
<< " blocks and has been rejected.");
vvc.m_invalid_block_height = true;
}
else if (vote.block_height > latest_height)
{
height_in_buffer = vote.block_height <= latest_height + VERIFY_HEIGHT_BUFFER;
LOG_PRINT_L1("Received vote for height: " << vote.block_height << ", is newer than: " << latest_height
<< " (latest block height) and has been rejected.");
vvc.m_invalid_block_height = true;
}
LOG_PRINT_L1("Unhandled vote type with value: " << (int)vote.type);
assert("Unhandled vote type" == 0);
return false;
};
if (vvc.m_invalid_block_height)
case quorum_type::obligations:
{
result = false;
vvc.m_verification_failed = !height_in_buffer;
return result;
}
}
{
crypto::public_key key = crypto::null_pkey;
crypto::hash hash = crypto::null_hash;
switch(vote.type)
{
default:
if (vote.group != quorum_group::validator)
{
LOG_PRINT_L1("Unhandled vote type with value: " << (int)vote.type);
assert("Unhandled vote type" == 0);
return false;
};
case quorum_type::obligations:
LOG_PRINT_L1("Vote received specifies incorrect voting group, expected vote from validator");
vvc.m_incorrect_voting_group = true;
result = false;
}
else
{
if (vote.group != quorum_group::validator)
{
LOG_PRINT_L1("Vote received specifies incorrect voting group, expected vote from validator");
vvc.m_incorrect_voting_group = true;
return false;
}
key = quorum.validators[vote.index_in_group];
hash = make_state_change_vote_hash(vote.block_height, vote.state_change.worker_index, vote.state_change.state);
bool result = bounds_check_worker_index(quorum, vote.state_change.worker_index, &vvc);
if (!result)
return result;
result = bounds_check_worker_index(quorum, vote.state_change.worker_index, &vvc);
}
break;
}
break;
case quorum_type::checkpointing:
case quorum_type::checkpointing:
{
if (vote.group != quorum_group::worker)
{
LOG_PRINT_L1("Vote received specifies incorrect voting group, expected vote from worker");
vvc.m_incorrect_voting_group = true;
result = false;
}
else
{
if (vote.group != quorum_group::worker)
{
LOG_PRINT_L1("Vote received specifies incorrect voting group, expected vote from worker");
vvc.m_incorrect_voting_group = true;
return false;
}
key = quorum.workers[vote.index_in_group];
hash = vote.checkpoint.block_hash;
}
break;
}
//
// NOTE: Validate vote signature
//
result = crypto::check_signature(hash, key, vote.signature);
if (!result)
{
vvc.m_signature_not_valid = true;
return result;
}
break;
}
result = true;
if (!result)
return result;
result = crypto::check_signature(hash, key, vote.signature);
if (!result)
vvc.m_signature_not_valid = true;
return result;
}
@ -514,17 +522,8 @@ namespace service_nodes
return false;
}
std::vector<pool_vote_entry> voting_pool::add_pool_vote_if_unique(uint64_t latest_height,
const quorum_vote_t& vote,
cryptonote::vote_verification_context& vvc,
const service_nodes::testing_quorum &quorum)
std::vector<pool_vote_entry> voting_pool::add_pool_vote_if_unique(const quorum_vote_t& vote, cryptonote::vote_verification_context& vvc)
{
if (!verify_vote(vote, latest_height, vvc, quorum))
{
LOG_PRINT_L1("Signature verification failed for deregister vote");
return {};
}
CRITICAL_REGION_LOCAL(m_lock);
auto *votes = find_vote_pool(vote, /*create_if_not_found=*/ true);
if (!votes)

8
src/cryptonote_core/service_node_voting.h
View File

@ -105,7 +105,8 @@ namespace service_nodes
bool verify_checkpoint (cryptonote::checkpoint_t const &checkpoint, service_nodes::testing_quorum const &quorum);
bool verify_tx_state_change (const cryptonote::tx_extra_service_node_state_change& state_change, uint64_t latest_height, cryptonote::tx_verification_context& vvc, const service_nodes::testing_quorum &quorum, uint8_t hf_version);
bool verify_vote (const quorum_vote_t& vote, uint64_t latest_height, cryptonote::vote_verification_context &vvc, const service_nodes::testing_quorum &quorum);
bool verify_vote_age (const quorum_vote_t& vote, uint64_t latest_height, cryptonote::vote_verification_context &vvc);
bool verify_vote_against_quorum (const quorum_vote_t& vote, cryptonote::vote_verification_context &vvc, const service_nodes::testing_quorum &quorum);
crypto::signature make_signature_from_vote (quorum_vote_t const &vote, const crypto::public_key& pub, const crypto::secret_key& sec);
crypto::signature make_signature_from_tx_state_change(cryptonote::tx_extra_service_node_state_change const &state_change, crypto::public_key const &pub, crypto::secret_key const &sec);
@ -132,10 +133,7 @@ namespace service_nodes
struct voting_pool
{
// return: The vector of votes if the vote is valid (and even if it is not unique) otherwise nullptr
std::vector<pool_vote_entry> add_pool_vote_if_unique(uint64_t latest_height,
const quorum_vote_t& vote,
cryptonote::vote_verification_context& vvc,
const service_nodes::testing_quorum &quorum);
std::vector<pool_vote_entry> add_pool_vote_if_unique(const quorum_vote_t &vote, cryptonote::vote_verification_context &vvc);
// TODO(loki): Review relay behaviour and all the cases when it should be triggered
void set_relayed (const std::vector<quorum_vote_t>& votes);

22
tests/unit_tests/service_nodes.cpp
View File

@ -119,6 +119,16 @@ TEST(service_nodes, staking_requirement)
}
}
static bool verify_vote(service_nodes::quorum_vote_t const &vote,
uint64_t latest_height,
cryptonote::vote_verification_context &vvc,
service_nodes::testing_quorum const &quorum)
{
bool result = service_nodes::verify_vote_age(vote, latest_height, vvc);
result &= service_nodes::verify_vote_against_quorum(vote, vvc, quorum);
return result;
}
TEST(service_nodes, vote_validation)
{
// Generate a quorum and the voter
@ -142,7 +152,7 @@ TEST(service_nodes, vote_validation)
service_nodes::quorum_vote_t valid_vote = service_nodes::make_state_change_vote(block_height, voter_index, 1 /*worker_index*/, service_nodes::new_state::decommission, service_node_voter.pub, service_node_voter.sec);
{
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(valid_vote, block_height, vvc, state);
bool result = verify_vote(valid_vote, block_height, vvc, state);
if (!result)
printf("%s\n", cryptonote::print_vote_verification_context(vvc, &valid_vote));
@ -156,7 +166,7 @@ TEST(service_nodes, vote_validation)
vote.signature = service_nodes::make_signature_from_vote(vote, service_node_voter.pub, service_node_voter.sec);
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(vote, block_height, vvc, state);
bool result = verify_vote(vote, block_height, vvc, state);
ASSERT_FALSE(result);
}
@ -167,7 +177,7 @@ TEST(service_nodes, vote_validation)
vote.signature = service_nodes::make_signature_from_vote(vote, service_node_voter.pub, service_node_voter.sec);
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(vote, block_height, vvc, state);
bool result = verify_vote(vote, block_height, vvc, state);
ASSERT_FALSE(result);
}
@ -178,21 +188,21 @@ TEST(service_nodes, vote_validation)
vote.signature = {};
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(vote, block_height, vvc, state);
bool result = verify_vote(vote, block_height, vvc, state);
ASSERT_FALSE(result);
}
// Vote too old
{
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(valid_vote, 1 /*latest_height*/, vvc, state);
bool result = verify_vote(valid_vote, 1 /*latest_height*/, vvc, state);
ASSERT_FALSE(result);
}
// Vote too far in the future
{
cryptonote::vote_verification_context vvc = {};
bool result = service_nodes::verify_vote(valid_vote, block_height + 10000, vvc, state);
bool result = verify_vote(valid_vote, block_height + 10000, vvc, state);
ASSERT_FALSE(result);
}
}