Don't relay expired votes; add vote tolerance before disconnection

Update the way votes are relayed to fix superfluous p2p disconnections:
- check that votes are actually not older than VOTE_LIFETIME before
  relaying.
- Fix off-by-one error in incoming vote handling (it was rejecting on >=
  maximum age instead of > maximum age).
- Add a 5-block buffer to the incoming vote tolerance handling: don't
  accept a vote that is too new or too old, but also don't trigger a
  disconnection if it is within 5 blocks of where it would be
  acceptable so that relays from slightly out of sync peers don't
  trigger p2p disconnections.
This commit is contained in:
Jason Rhinelander 2019-06-30 13:59:45 -03:00
parent c1d4d3b347
commit 192ea32326
5 changed files with 26 additions and 25 deletions

View File

@ -1430,7 +1430,7 @@ namespace cryptonote
//-----------------------------------------------------------------------------------------------
bool core::relay_service_node_votes()
{
std::vector<service_nodes::quorum_vote_t> relayable_votes = m_quorum_cop.get_relayable_votes();
std::vector<service_nodes::quorum_vote_t> relayable_votes = m_quorum_cop.get_relayable_votes(get_current_blockchain_height());
uint8_t hf_version = get_blockchain_storage().get_current_hard_fork_version();
if (hf_version < cryptonote::network_version_11_infinite_staking)
{

View File

@ -138,10 +138,9 @@ namespace service_nodes
m_vote_pool.set_relayed(relayed_votes);
}
std::vector<quorum_vote_t> quorum_cop::get_relayable_votes()
std::vector<quorum_vote_t> quorum_cop::get_relayable_votes(uint64_t current_height)
{
std::vector<quorum_vote_t> result = m_vote_pool.get_relayable_votes();
return result;
return m_vote_pool.get_relayable_votes(current_height);
}
static int find_index_in_quorum_group(std::vector<crypto::public_key> const &group, crypto::public_key const &my_pubkey)
@ -370,14 +369,7 @@ namespace service_nodes
{
process_quorums(block);
// Since our age checks for state change votes is now (age >=
// STATE_CHANGE_VOTE_LIFETIME_IN_BLOCKS) where age is
// get_current_blockchain_height() which gives you the height that you are
// currently mining for, i.e. (height + 1).
// Otherwise peers will silently drop connection from each other when they
// go around P2Ping votes due to passing around old votes
uint64_t const height = cryptonote::get_block_height(block) + 1;
uint64_t const height = cryptonote::get_block_height(block) + 1; // chain height = new top block height + 1
m_vote_pool.remove_expired_votes(height);
m_vote_pool.remove_used_votes(txs, block.major_version);
}

View File

@ -81,7 +81,7 @@ namespace service_nodes
void blockchain_detached(uint64_t height) override;
void set_votes_relayed (std::vector<quorum_vote_t> const &relayed_votes);
std::vector<quorum_vote_t> get_relayable_votes();
std::vector<quorum_vote_t> get_relayable_votes(uint64_t current_height);
bool handle_vote (quorum_vote_t const &vote, cryptonote::vote_verification_context &vvc);
static int64_t calculate_decommission_credit(const service_node_info &info, uint64_t current_height);

View File

@ -319,15 +319,22 @@ namespace service_nodes
// NOTE: Validate vote age
//
{
uint64_t delta_height = latest_height - vote.block_height;
if (vote.block_height < latest_height && delta_height >= VOTE_LIFETIME)
// If we get an incoming vote that is outside the acceptable range by this many votes then
// just ignore it instead of dropping the connection; the sending side could be a couple
// blocks out of sync and sending something that it thinks is legit.
constexpr uint64_t VOTE_BUFFER = 5;
bool height_in_buffer = false;
if (latest_height > vote.block_height + VOTE_LIFETIME)
{
height_in_buffer = latest_height <= vote.block_height + (VOTE_LIFETIME + VOTE_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 + VOTE_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;
@ -336,7 +343,7 @@ namespace service_nodes
if (vvc.m_invalid_block_height)
{
result = false;
vvc.m_verification_failed = true;
vvc.m_verification_failed = !height_in_buffer;
return result;
}
}
@ -454,28 +461,30 @@ namespace service_nodes
}
template <typename T>
static void append_relayable_votes(std::vector<quorum_vote_t> &result, const T &pool, const time_t now, const time_t threshold) {
static void append_relayable_votes(std::vector<quorum_vote_t> &result, const T &pool, const uint64_t max_last_sent, uint64_t min_height) {
for (const auto &pool_entry : pool)
for (const auto &vote_entry : pool_entry.votes)
if (now > (time_t)vote_entry.time_last_sent_p2p + threshold)
if (vote_entry.vote.block_height >= min_height && vote_entry.time_last_sent_p2p <= max_last_sent)
result.push_back(vote_entry.vote);
}
std::vector<quorum_vote_t> voting_pool::get_relayable_votes() const
std::vector<quorum_vote_t> voting_pool::get_relayable_votes(uint64_t height) const
{
CRITICAL_REGION_LOCAL(m_lock);
// TODO(doyle): Rate-limiting: A better threshold value that follows suite with transaction relay time back-off
#if defined(LOKI_ENABLE_INTEGRATION_TEST_HOOKS)
const time_t TIME_BETWEEN_RELAY = 0;
constexpr uint64_t TIME_BETWEEN_RELAY = 0;
#else
const time_t TIME_BETWEEN_RELAY = 60 * 2;
constexpr uint64_t TIME_BETWEEN_RELAY = 60 * 2;
#endif
time_t const now = time(nullptr);
const uint64_t max_last_sent = static_cast<uint64_t>(time(nullptr)) - TIME_BETWEEN_RELAY;
const uint64_t min_height = height > VOTE_LIFETIME ? height - VOTE_LIFETIME : 0;
std::vector<quorum_vote_t> result;
append_relayable_votes(result, m_obligations_pool, now, TIME_BETWEEN_RELAY);
append_relayable_votes(result, m_checkpoint_pool, now, TIME_BETWEEN_RELAY);
append_relayable_votes(result, m_obligations_pool, max_last_sent, min_height);
append_relayable_votes(result, m_checkpoint_pool, max_last_sent, min_height);
return result;
}

View File

@ -138,7 +138,7 @@ namespace service_nodes
void set_relayed (const std::vector<quorum_vote_t>& votes);
void remove_expired_votes(uint64_t height);
void remove_used_votes (std::vector<cryptonote::transaction> const &txs, uint8_t hard_fork_version);
std::vector<quorum_vote_t> get_relayable_votes () const;
std::vector<quorum_vote_t> get_relayable_votes (uint64_t height) const;
private:
std::vector<pool_vote_entry> *find_vote_pool(const quorum_vote_t &vote, bool create_if_not_found = false);