Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

client/beefy: add some bounds on enqueued votes #12562

Merged
Merged
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions client/beefy/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,22 @@ use sp_consensus::SyncOracle;
use sp_mmr_primitives::MmrApi;
use sp_runtime::{
generic::OpaqueDigestItemId,
traits::{Block, Header, NumberFor, Zero},
SaturatedConversion,
traits::{Block, ConstU32, Header, NumberFor, Zero},
BoundedVec, SaturatedConversion,
};
use std::{
collections::{BTreeMap, BTreeSet, VecDeque},
fmt::Debug,
marker::PhantomData,
sync::Arc,
};
/// Bound for the number of buffered future voting rounds.
const MAX_BUFFERED_VOTE_ROUNDS: u32 = 20;
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
/// Bound for the number of buffered votes per round number.
const MAX_BUFFERED_VOTES_PER_ROUND: u32 = 1000;
/// Bound for the number of pending justifications - use 2400 - the max number
/// of justifications possible in a single session.
const MAX_BUFFERED_JUSTIFICATIONS: u32 = 2400;

pub(crate) enum RoundAction {
Drop,
Expand Down Expand Up @@ -307,7 +314,13 @@ pub(crate) struct BeefyWorker<B: Block, BE, P, R, N> {
/// BEEFY client metrics.
metrics: Option<Metrics>,
/// Buffer holding votes for future processing.
pending_votes: BTreeMap<NumberFor<B>, Vec<VoteMessage<NumberFor<B>, AuthorityId, Signature>>>,
pending_votes: BTreeMap<
NumberFor<B>,
BoundedVec<
VoteMessage<NumberFor<B>, AuthorityId, Signature>,
ConstU32<MAX_BUFFERED_VOTES_PER_ROUND>,
>,
>,
/// Buffer holding justifications for future processing.
pending_justifications: BTreeMap<NumberFor<B>, BeefyVersionedFinalityProof<B>>,
/// Persisted voter state.
Expand Down Expand Up @@ -482,7 +495,14 @@ where
)?,
RoundAction::Enqueue => {
debug!(target: "beefy", "🥩 Buffer vote for round: {:?}.", block_num);
self.pending_votes.entry(block_num).or_default().push(vote)
if self.pending_votes.len() < MAX_BUFFERED_VOTE_ROUNDS as usize {
acatangiu marked this conversation as resolved.
Show resolved Hide resolved
let votes_vec = self.pending_votes.entry(block_num).or_default();
if votes_vec.try_push(vote).is_err() {
warn!(target: "beefy", "🥩 Buffer vote dropped for round: {:?}", block_num)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just random thought: value in pending_votes map may store duplicated votes, right? So in theory malicious validator may try to send MAX_BUFFERED_VOTES_PER_ROUND duplicate votes to all other validators, blocking round from concluding. If that's true, then it is something that must be tracked during equivocation impl.

Copy link
Contributor

@acatangiu acatangiu Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's correct - added explicit comment for this to #12692

}
} else {
error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num);
}
},
RoundAction::Drop => (),
};
Expand All @@ -508,7 +528,11 @@ where
},
RoundAction::Enqueue => {
debug!(target: "beefy", "🥩 Buffer justification for round: {:?}.", block_num);
self.pending_justifications.entry(block_num).or_insert(justification);
if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS as usize {
self.pending_justifications.entry(block_num).or_insert(justification);
} else {
error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num);
}
},
RoundAction::Drop => (),
};
Expand Down