Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement DecodeWithMemTracking #165

Merged
merged 4 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ jobs:
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: nightly
toolchain: nightly-2024-09-11
override: true
- name: Test
uses: actions-rs/cargo@v1
Expand All @@ -77,8 +77,8 @@ jobs:
args: --no-fail-fast
env:
CARGO_INCREMENTAL: '0'
RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests'
RUSTDOCFLAGS: '-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests'
RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests'
RUSTDOCFLAGS: '-Cpanic=abort'
- name: Gather coverage data
id: coverage
uses: actions-rs/[email protected]
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "finality-grandpa"
version = "0.16.2"
version = "0.16.3"
description = "PBFT-based finality gadget for blockchains"
authors = ["Parity Technologies <[email protected]>"]
license = "Apache-2.0"
Expand Down
35 changes: 17 additions & 18 deletions src/fuzz_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ impl FuzzChain {
match hash {
0 => 0,

1 | 2 | 3 => 1,
1..=3 => 1,

4 | 5 | 6 => 2,
7 | 8 | 9 => 2,
4..=6 => 2,
7..=9 => 2,

10 | 11 | 12 => 3,
13 | 14 | 15 => 3,
10..=12 => 3,
13..=15 => 3,

_ => panic!("invalid block hash"),
}
Expand Down Expand Up @@ -122,11 +122,11 @@ impl Chain<Hash, BlockNumber> for FuzzChain {

let full_ancestry: &[Hash] = match block {
0 => &[],
1 | 2 | 3 => &[0],
4 | 5 | 6 => &[0, 1],
7 | 8 | 9 => &[0, 2],
10 | 11 | 12 => &[0, 1, 4],
13 | 14 | 15 => &[0, 2, 7],
1..=3 => &[0],
4..=6 => &[0, 1],
7..=9 => &[0, 2],
10..=12 => &[0, 1, 4],
13..=15 => &[0, 2, 7],
_ => panic!("invalid block hash"),
};

Expand Down Expand Up @@ -170,7 +170,7 @@ impl<'a> RandomnessStream<'a> {
self.half_nibble = false;
}
self.pos += 1;
self.inner.get(self.pos).map(|&b| b)
self.inner.get(self.pos).copied()
}
}

Expand Down Expand Up @@ -334,7 +334,7 @@ pub fn execute_fuzzed_vote(data: &[u8]) {

// Import precommits.
for (i, &voter) in voters().iter().enumerate() {
let round = &mut rounds[i as usize];
let round = &mut rounds[i];

// Import enough precommits (including our own) to reach supermajority.
let k = match stream.read_byte() {
Expand All @@ -349,7 +349,7 @@ pub fn execute_fuzzed_vote(data: &[u8]) {

// Start tracking completability and estimate.
let mut completable = round.state().completable;
let mut last_estimate = round.state().estimate.clone();
let mut last_estimate = round.state().estimate;

// Import the remaining precommits.
for j in omit {
Expand Down Expand Up @@ -384,7 +384,7 @@ pub fn execute_fuzzed_vote(data: &[u8]) {

// Now (re-)import _all_ prevotes, checking the prevote-ghost along the way.
for &v in voters().iter() {
let old_ghost = round.state().prevote_ghost.clone().expect("supermajority seen");
let old_ghost = round.state().prevote_ghost.expect("supermajority seen");

let vote = prevotes[v as usize].clone();
let result = round.import_prevote(&FuzzChain, vote, v, v).unwrap();
Expand Down Expand Up @@ -443,7 +443,7 @@ pub fn execute_fuzzed_graph(data: &[u8]) {

graph.insert(target_hash, target_number, new_prevote(), &FuzzChain).unwrap();

let new_prevote_ghost = graph.find_ghost(prevote_ghost.clone(), |v| v.prevote >= T);
let new_prevote_ghost = graph.find_ghost(prevote_ghost, |v| v.prevote >= T);
if let Some(old_ghost) = prevote_ghost {
let new_ghost = new_prevote_ghost.expect("ghost does not disappear with more votes.");
check_prevote_ghost(old_ghost, new_ghost);
Expand Down Expand Up @@ -482,8 +482,7 @@ pub fn execute_fuzzed_graph(data: &[u8]) {

// The already calculated prevote ghost should not change as a result of
// adding precommit weights.
let new_prevote_ghost =
graph.find_ghost(Some(prevote_ghost.clone()), |v| v.prevote >= T).unwrap();
let new_prevote_ghost = graph.find_ghost(Some(prevote_ghost), |v| v.prevote >= T).unwrap();
assert_eq!(new_prevote_ghost, prevote_ghost, "prevote ghost changed");

// The number of voters who did not yet cast a vote.
Expand Down Expand Up @@ -523,7 +522,7 @@ pub fn execute_fuzzed_graph(data: &[u8]) {
check_estimate(old_estimate, new_estimate);
}

estimate = new_estimate.clone();
estimate = new_estimate;
completable = newly_completable;
}

Expand Down
16 changes: 8 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ mod bitfield;
mod bridge_state;
#[cfg(any(test, feature = "fuzz-helpers"))]
pub mod fuzz_helpers;
#[cfg(any(test))]
#[cfg(test)]
mod testing;
mod weights;
#[cfg(not(feature = "std"))]
Expand Down Expand Up @@ -73,7 +73,7 @@ mod std {

use crate::{std::vec::Vec, voter_set::VoterSet};
#[cfg(feature = "derive-codec")]
use parity_scale_codec::{Decode, Encode};
use parity_scale_codec::{Decode, DecodeWithMemTracking, Encode};
use round::ImportResult;
#[cfg(feature = "derive-codec")]
use scale_info::TypeInfo;
Expand All @@ -83,7 +83,7 @@ const LOG_TARGET: &str = "grandpa";

/// A prevote for a block and its ancestors.
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, TypeInfo))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, DecodeWithMemTracking, TypeInfo))]
pub struct Prevote<H, N> {
/// The target block's hash.
pub target_hash: H,
Expand All @@ -100,7 +100,7 @@ impl<H, N> Prevote<H, N> {

/// A precommit for a block and its ancestors.
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, TypeInfo))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, DecodeWithMemTracking, TypeInfo))]
pub struct Precommit<H, N> {
/// The target block's hash.
pub target_hash: H,
Expand Down Expand Up @@ -209,7 +209,7 @@ pub trait Chain<H: Eq, N: Copy + BlockNumberOps> {

/// An equivocation (double-vote) in a given round.
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, TypeInfo))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, DecodeWithMemTracking, TypeInfo))]
pub struct Equivocation<Id, V, S> {
/// The round number equivocated in.
pub round_number: u64,
Expand Down Expand Up @@ -273,7 +273,7 @@ impl<H, N: Copy, S, Id> SignedMessage<H, N, S, Id> {
/// A commit message which is an aggregate of precommits.
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(any(feature = "std", test), derive(Debug))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, TypeInfo))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, DecodeWithMemTracking, TypeInfo))]
pub struct Commit<H, N, S, Id> {
/// The target block's hash.
pub target_hash: H,
Expand All @@ -299,7 +299,7 @@ pub struct SignedPrevote<H, N, S, Id> {
/// A signed precommit message.
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(any(feature = "std", test), derive(Debug))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, TypeInfo))]
#[cfg_attr(feature = "derive-codec", derive(Encode, Decode, DecodeWithMemTracking, TypeInfo))]
pub struct SignedPrecommit<H, N, S, Id> {
/// The precommit message which has been signed.
pub precommit: Precommit<H, N>,
Expand Down Expand Up @@ -357,7 +357,7 @@ impl<H, N, S, Id> From<CompactCommit<H, N, S, Id>> for Commit<H, N, S, Id> {
precommits: commit
.precommits
.into_iter()
.zip(commit.auth_data.into_iter())
.zip(commit.auth_data)
.map(|(precommit, (signature, id))| SignedPrecommit { precommit, signature, id })
.collect(),
}
Expand Down
10 changes: 5 additions & 5 deletions src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@ pub struct Round<Id: Ord + Eq, H: Ord + Eq, N, Signature> {
prevote: VoteTracker<Id, Prevote<H, N>, Signature>, // tracks prevotes that have been counted
precommit: VoteTracker<Id, Precommit<H, N>, Signature>, // tracks precommits
historical_votes: HistoricalVotes<H, N, Signature, Id>,
prevote_ghost: Option<(H, N)>, // current memoized prevote-GHOST block
prevote_ghost: Option<(H, N)>, // current memoized prevote-GHOST block
precommit_ghost: Option<(H, N)>, // current memoized precommit-GHOST block
finalized: Option<(H, N)>, // best finalized block in this round.
estimate: Option<(H, N)>, // current memoized round-estimate
completable: bool, // whether the round is completable
finalized: Option<(H, N)>, // best finalized block in this round.
estimate: Option<(H, N)>, // current memoized round-estimate
completable: bool, // whether the round is completable
}

/// Result of importing a Prevote or Precommit.
Expand Down Expand Up @@ -580,7 +580,7 @@ where
return
}

self.completable = self.estimate.clone().map_or(false, |(b_hash, b_num)| {
self.completable = self.estimate.clone().is_some_and(|(b_hash, b_num)| {
b_hash != g_hash || {
// round-estimate is the same as the prevote-ghost.
// this round is still completable if no further blocks
Expand Down
10 changes: 5 additions & 5 deletions src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub mod chain {
}

pub fn last_finalized(&self) -> (&'static str, u32) {
self.finalized.clone()
self.finalized
}

pub fn set_last_finalized(&mut self, last_finalized: (&'static str, u32)) {
Expand All @@ -99,7 +99,7 @@ pub mod chain {
return Some((leaf, leaf_number))
}

if let Ok(_) = self.ancestry(base, leaf) {
if self.ancestry(base, leaf).is_ok() {
return Some((leaf, leaf_number))
}
}
Expand Down Expand Up @@ -194,7 +194,7 @@ pub mod environment {
F: FnOnce(&mut DummyChain) -> U,
{
let mut chain = self.chain.lock();
f(&mut *chain)
f(&mut chain)
}

/// Stream of finalized blocks.
Expand All @@ -208,7 +208,7 @@ pub mod environment {

/// Get the last completed and concluded rounds.
pub fn last_completed_and_concluded(&self) -> (u64, u64) {
self.last_completed_and_concluded.lock().clone()
*self.last_completed_and_concluded.lock()
}
}

Expand Down Expand Up @@ -295,7 +295,7 @@ pub mod environment {
let mut chain = self.chain.lock();

let last_finalized = chain.last_finalized();
if number as u32 <= last_finalized.1 {
if number <= last_finalized.1 {
panic!("Attempted to finalize backwards")
}

Expand Down
2 changes: 1 addition & 1 deletion src/vote_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ where
return Some((hash, number))
}
// Not enough weight, check the parent block.
match node.ancestors.get(0) {
match node.ancestors.first() {
None => return None,
Some(a) => {
hash = a.clone();
Expand Down
14 changes: 7 additions & 7 deletions src/voter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,12 @@ where
/// given block and includes a set of precommits as proof.
///
/// - When a round is completable and we precommitted we start a commit timer
/// and start accepting commit messages;
/// and start accepting commit messages;
/// - When we receive a commit message if it targets a block higher than what
/// we've finalized we validate it and import its precommits if valid;
/// we've finalized we validate it and import its precommits if valid;
/// - When our commit timer triggers we check if we've received any commit
/// message for a block equal to what we've finalized, if we haven't then we
/// broadcast a commit.
/// message for a block equal to what we've finalized, if we haven't then we
/// broadcast a commit.
///
/// Additionally, we also listen to commit messages from rounds that aren't
/// currently running, we validate the commit and dispatch a finalization
Expand Down Expand Up @@ -1173,7 +1173,7 @@ mod tests {

pool.spawner().spawn(routing_task.map(|_| ())).unwrap();

pool.run_until(future::join_all(finalized_streams.into_iter()));
pool.run_until(future::join_all(finalized_streams));
}

#[test]
Expand Down Expand Up @@ -1242,7 +1242,7 @@ mod tests {
);

pool.spawner().spawn(routing_task.map(|_| ())).unwrap();
pool.run_until(future::join_all(finalized_streams.into_iter()));
pool.run_until(future::join_all(finalized_streams));

assert_eq!(voter_state.get().best_round, (2, expected_round_state.clone()));
}
Expand Down Expand Up @@ -1459,7 +1459,7 @@ mod tests {
let voters = VoterSet::new((0..3).map(|i| (Id(i), 1u64))).expect("nonempty");
let total_weight = voters.total_weight();
let threshold_weight = voters.threshold();
let voter_ids: HashSet<Id> = (0..3).map(|i| Id(i)).collect();
let voter_ids: HashSet<Id> = (0..3).map(Id).collect();

let (network, routing_task) = testing::environment::make_network();
let mut pool = LocalPool::new();
Expand Down
8 changes: 4 additions & 4 deletions src/voter/voting_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ where

let voting = if round_data.voter_id.as_ref() == Some(votes.primary_voter().0) {
Voting::Primary
} else if round_data.voter_id.as_ref().map_or(false, |id| votes.voters().contains(id)) {
} else if round_data.voter_id.as_ref().is_some_and(|id| votes.voters().contains(id)) {
Voting::Yes
} else {
Voting::No
Expand Down Expand Up @@ -214,7 +214,7 @@ where

// or it must be finalized in the current round
let finalized_in_current_round =
self.finalized().map_or(false, |(_, current_round_finalized)| {
self.finalized().is_some_and(|(_, current_round_finalized)| {
last_round_estimate <= *current_round_finalized
});

Expand Down Expand Up @@ -630,7 +630,7 @@ where
let should_precommit = {
// we wait for the last round's estimate to be equal to or
// the ancestor of the current round's p-Ghost before precommitting.
self.votes.state().prevote_ghost.as_ref().map_or(false, |p_g| {
self.votes.state().prevote_ghost.as_ref().is_some_and(|p_g| {
p_g == &last_round_estimate ||
self.env
.is_equal_or_descendent_of(last_round_estimate.0, p_g.0.clone())
Expand Down Expand Up @@ -710,7 +710,7 @@ where
(last_prevote_g.1 - to_sub).as_()
};

if ancestry.get(offset).map_or(false, |b| b == p_hash) {
if ancestry.get(offset) == Some(p_hash) {
p_hash.clone()
} else {
last_round_estimate.0
Expand Down
Loading