Skip to content

Commit 71f0314

Browse files
Marcin-RadeckiMarcin-Radecki
authored
A0-4563: Introduce notion of non-direct parents in Extender (#506)
* First working version * fmt * clippy * Bump consensus crate version * Updated readme * Revered changes to unit testing code * Review * fmt * Review * Try to fix cargo audit pipeline
1 parent 5a13d1e commit 71f0314

File tree

13 files changed

+87
-43
lines changed

13 files changed

+87
-43
lines changed

‎.github/workflows/cargo-audit.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ jobs:
2020
- name: Checkout source code
2121
uses: actions/checkout@v4
2222

23+
- name: Install cargo audit
24+
shell: bash
25+
run: |
26+
cargo install cargo-audit --locked
27+
2328
- name: Run `cargo-audit`
2429
uses: actions-rs/audit-check@v1
2530
with:

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ More details are available [in the book][reference-link-implementation-details].
6060
- Import AlephBFT in your crate
6161
```toml
6262
[dependencies]
63-
aleph-bft = "^0.39"
63+
aleph-bft = "^0.40"
6464
```
6565
- The main entry point is the `run_session` function, which returns a Future that runs the
6666
consensus algorithm.

‎consensus/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "aleph-bft"
3-
version = "0.39.1"
3+
version = "0.40.0"
44
edition = "2021"
55
authors = ["Cardinal Cryptography"]
66
categories = ["algorithms", "data-structures", "cryptography", "database"]

‎consensus/src/dag/reconstruction/dag.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ impl<U: UnitWithParents> Dag<U> {
9696
}
9797
let missing_parents = unit
9898
.parents()
99-
.values()
10099
.filter(|parent| !self.dag_units.contains(parent))
101100
.cloned()
102101
.collect();

‎consensus/src/dag/reconstruction/mod.rs

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
1-
use std::collections::HashMap;
2-
31
use crate::{
42
units::{ControlHash, FullUnit, HashFor, Unit, UnitCoord, UnitWithParents, WrappedUnit},
53
Hasher, NodeMap, SessionId,
64
};
5+
use aleph_bft_rmc::NodeCount;
6+
use std::collections::HashMap;
77

88
mod dag;
99
mod parents;
1010

11-
use aleph_bft_types::{Data, MultiKeychain, OrderedUnit, Signed};
11+
use aleph_bft_types::{Data, MultiKeychain, NodeIndex, OrderedUnit, Round, Signed};
1212
use dag::Dag;
1313
use parents::Reconstruction as ParentReconstruction;
1414

1515
/// A unit with its parents represented explicitly.
1616
#[derive(Debug, PartialEq, Eq, Clone)]
1717
pub struct ReconstructedUnit<U: Unit> {
1818
unit: U,
19-
parents: NodeMap<HashFor<U>>,
19+
parents: NodeMap<(HashFor<U>, Round)>,
2020
}
2121

2222
impl<U: Unit> ReconstructedUnit<U> {
@@ -25,7 +25,18 @@ impl<U: Unit> ReconstructedUnit<U> {
2525
match unit.control_hash().combined_hash
2626
== ControlHash::<U::Hasher>::combine_hashes(&parents)
2727
{
28-
true => Ok(ReconstructedUnit { unit, parents }),
28+
true => {
29+
let unit_round = unit.round();
30+
let mut parents_with_rounds = NodeMap::with_size(parents.size());
31+
for (parent_index, hash) in parents.into_iter() {
32+
// we cannot have here round 0 units
33+
parents_with_rounds.insert(parent_index, (hash, unit_round.saturating_sub(1)));
34+
}
35+
Ok(ReconstructedUnit {
36+
unit,
37+
parents: parents_with_rounds,
38+
})
39+
}
2940
false => Err(unit),
3041
}
3142
}
@@ -72,8 +83,33 @@ impl<U: Unit> WrappedUnit<U::Hasher> for ReconstructedUnit<U> {
7283
}
7384

7485
impl<U: Unit> UnitWithParents for ReconstructedUnit<U> {
75-
fn parents(&self) -> &NodeMap<HashFor<Self>> {
76-
&self.parents
86+
fn parents(&self) -> impl Iterator<Item = &HashFor<U>> {
87+
self.parents.values().map(|(hash, _)| hash)
88+
}
89+
90+
fn direct_parents(&self) -> impl Iterator<Item = &HashFor<Self>> {
91+
self.parents
92+
.values()
93+
.filter_map(|(hash, parent_round)| match self.unit.coord().round() {
94+
// round 0 units cannot have non-empty parents
95+
0 => None,
96+
97+
unit_round => {
98+
if unit_round - 1 == *parent_round {
99+
Some(hash)
100+
} else {
101+
None
102+
}
103+
}
104+
})
105+
}
106+
107+
fn parent_for(&self, index: NodeIndex) -> Option<&HashFor<Self>> {
108+
self.parents.get(index).map(|(hash, _)| hash)
109+
}
110+
111+
fn node_count(&self) -> NodeCount {
112+
self.parents.size()
77113
}
78114
}
79115

@@ -89,7 +125,7 @@ impl<D: Data, H: Hasher, K: MultiKeychain> From<ReconstructedUnit<Signed<FullUni
89125
for OrderedUnit<D, H>
90126
{
91127
fn from(unit: ReconstructedUnit<Signed<FullUnit<H, D>, K>>) -> Self {
92-
let parents = unit.parents().values().cloned().collect();
128+
let parents = unit.parents().cloned().collect();
93129
let unit = unit.unpack();
94130
let creator = unit.creator();
95131
let round = unit.round();
@@ -233,7 +269,7 @@ mod test {
233269
assert_eq!(units.len(), 1);
234270
let reconstructed_unit = units.pop().expect("just checked its there");
235271
assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone()));
236-
assert_eq!(reconstructed_unit.parents().item_count(), 0);
272+
assert_eq!(reconstructed_unit.parents().count(), 0);
237273
}
238274
}
239275

@@ -254,15 +290,15 @@ mod test {
254290
match round {
255291
0 => {
256292
assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone()));
257-
assert_eq!(reconstructed_unit.parents().item_count(), 0);
293+
assert_eq!(reconstructed_unit.parents().count(), 0);
258294
}
259295
round => {
260-
assert_eq!(reconstructed_unit.parents().item_count(), 4);
296+
assert_eq!(reconstructed_unit.parents().count(), 4);
261297
let parents = dag
262298
.get((round - 1) as usize)
263299
.expect("the parents are there");
264300
for (parent, reconstructed_parent) in
265-
parents.iter().zip(reconstructed_unit.parents().values())
301+
parents.iter().zip(reconstructed_unit.parents())
266302
{
267303
assert_eq!(&parent.hash(), reconstructed_parent);
268304
}

‎consensus/src/dag/reconstruction/parents.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ mod test {
237237
assert_eq!(units.len(), 1);
238238
let reconstructed_unit = units.pop().expect("just checked its there");
239239
assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone()));
240-
assert_eq!(reconstructed_unit.parents().item_count(), 0);
240+
assert_eq!(reconstructed_unit.parents().count(), 0);
241241
}
242242
}
243243

@@ -258,15 +258,15 @@ mod test {
258258
match round {
259259
0 => {
260260
assert_eq!(reconstructed_unit, ReconstructedUnit::initial(unit.clone()));
261-
assert_eq!(reconstructed_unit.parents().item_count(), 0);
261+
assert_eq!(reconstructed_unit.parents().count(), 0);
262262
}
263263
round => {
264-
assert_eq!(reconstructed_unit.parents().item_count(), 4);
264+
assert_eq!(reconstructed_unit.parents().count(), 4);
265265
let parents = dag
266266
.get((round - 1) as usize)
267267
.expect("the parents are there");
268268
for (parent, reconstructed_parent) in
269-
parents.iter().zip(reconstructed_unit.parents().values())
269+
parents.iter().zip(reconstructed_unit.parents())
270270
{
271271
assert_eq!(&parent.hash(), reconstructed_parent);
272272
}
@@ -371,11 +371,11 @@ mod test {
371371
assert!(requests.is_empty());
372372
assert_eq!(units.len(), 1);
373373
let reconstructed_unit = units.pop().expect("just checked its there");
374-
assert_eq!(reconstructed_unit.parents().item_count(), 4);
374+
assert_eq!(reconstructed_unit.parents().count(), 4);
375375
for (coord, parent_hash) in parent_hashes {
376376
assert_eq!(
377377
Some(&parent_hash),
378-
reconstructed_unit.parents().get(coord.creator())
378+
reconstructed_unit.parent_for(coord.creator())
379379
);
380380
}
381381
}

‎consensus/src/dissemination/responder.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ impl<H: Hasher, D: Data, MK: MultiKeychain> Responder<H, D, MK> {
5757
.map(|unit| {
5858
let parents = unit
5959
.parents()
60-
.values()
6160
.map(|parent_hash| {
6261
units
6362
.unit(parent_hash)
@@ -270,8 +269,8 @@ mod test {
270269
match response {
271270
Response::Parents(response_hash, parents) => {
272271
assert_eq!(response_hash, requested_unit.hash());
273-
assert_eq!(parents.len(), requested_unit.parents().size().0);
274-
for (parent, parent_hash) in zip(parents, requested_unit.parents().values()) {
272+
assert_eq!(parents.len(), requested_unit.parents().count());
273+
for (parent, parent_hash) in zip(parents, requested_unit.parents()) {
275274
assert_eq!(&parent.as_signable().hash(), parent_hash);
276275
}
277276
}

‎consensus/src/extension/election.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::collections::HashMap;
33
use crate::{
44
extension::units::Units,
55
units::{HashFor, UnitWithParents},
6-
Hasher, NodeCount, NodeIndex, NodeMap, Round,
6+
Hasher, NodeCount, NodeIndex, Round,
77
};
88

99
fn common_vote(relative_round: Round) -> bool {
@@ -49,11 +49,11 @@ impl<U: UnitWithParents> CandidateElection<U> {
4949

5050
fn parent_votes(
5151
&mut self,
52-
parents: &NodeMap<HashFor<U>>,
52+
parents: Vec<HashFor<U>>,
5353
) -> Result<(NodeCount, NodeCount), CandidateOutcome<U::Hasher>> {
5454
let (mut votes_for, mut votes_against) = (NodeCount(0), NodeCount(0));
55-
for parent in parents.values() {
56-
match self.votes.get(parent).expect("units are added in order") {
55+
for parent in parents {
56+
match self.votes.get(&parent).expect("units are added in order") {
5757
true => votes_for += NodeCount(1),
5858
false => votes_against += NodeCount(1),
5959
}
@@ -63,11 +63,11 @@ impl<U: UnitWithParents> CandidateElection<U> {
6363

6464
fn vote_from_parents(
6565
&mut self,
66-
parents: &NodeMap<HashFor<U>>,
66+
parents: Vec<HashFor<U>>,
67+
threshold: NodeCount,
6768
relative_round: Round,
6869
) -> Result<bool, CandidateOutcome<U::Hasher>> {
6970
use CandidateOutcome::*;
70-
let threshold = parents.size().consensus_threshold();
7171
// Gather parents' votes.
7272
let (votes_for, votes_against) = self.parent_votes(parents)?;
7373
assert!(votes_for + votes_against >= threshold);
@@ -105,9 +105,13 @@ impl<U: UnitWithParents> CandidateElection<U> {
105105
let vote = match relative_round {
106106
0 => unreachable!("just checked that voter and election rounds are not equal"),
107107
// Direct descendands vote for, all other units of that round against.
108-
1 => voter.parents().get(self.candidate_creator) == Some(&self.candidate_hash),
108+
1 => voter.parent_for(self.candidate_creator) == Some(&self.candidate_hash),
109109
// Otherwise we compute the vote based on the parents' votes.
110-
_ => self.vote_from_parents(voter.parents(), relative_round)?,
110+
_ => {
111+
let threshold = voter.node_count().consensus_threshold();
112+
let direct_parents = voter.direct_parents().cloned().collect();
113+
self.vote_from_parents(direct_parents, threshold, relative_round)?
114+
}
111115
};
112116
self.votes.insert(voter.hash(), vote);
113117
Ok(())
@@ -360,7 +364,7 @@ mod test {
360364

361365
#[test]
362366
#[ignore]
363-
// TODO(A0-4563) Uncomment after changes to parent voting code
367+
// TODO(A0-4559) Uncomment
364368
fn given_minimal_dag_with_orphaned_node_when_electing_then_orphaned_node_is_not_head() {
365369
use ElectionResult::*;
366370
let mut units = Units::new();

‎consensus/src/extension/extender.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ mod test {
101101

102102
#[test]
103103
#[ignore]
104-
// TODO(A0-4563) Uncomment after changes to parent voting code
104+
// TODO(A0-4559) Uncomment
105105
fn given_minimal_dag_with_orphaned_node_when_producing_batches_have_correct_length() {
106106
let mut extender = Extender::new();
107107
let n_members = NodeCount(4);

‎consensus/src/extension/units.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ impl<U: UnitWithParents> Units<U> {
6464
.expect("head is picked among units we have"),
6565
);
6666
while let Some(u) = queue.pop_front() {
67-
for u_hash in u.parents().clone().into_values() {
68-
if let Some(v) = self.units.remove(&u_hash) {
67+
for u_hash in u.parents() {
68+
if let Some(v) = self.units.remove(u_hash) {
6969
queue.push_back(v);
7070
}
7171
}

‎consensus/src/testing/dag.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,16 @@ impl DagFeeder {
139139

140140
fn on_reconstructed_unit(&mut self, unit: ReconstructedUnit) {
141141
let h = unit.hash();
142-
let parents = unit.parents();
142+
let parents: HashSet<_> = unit.parents().cloned().collect();
143143
let expected_hashes: HashSet<_> = self
144144
.units_map
145145
.get(&h)
146146
.expect("we have the unit")
147147
.parent_hashes()
148148
.into_iter()
149149
.collect();
150-
assert_eq!(parents.item_count(), expected_hashes.len());
151-
for (_, hash) in parents {
152-
assert!(expected_hashes.contains(hash));
153-
}
150+
151+
assert_eq!(parents, expected_hashes);
154152
self.result.push(unit.clone());
155153
self.store.insert(unit);
156154
}

‎consensus/src/units/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ pub trait WrappedUnit<H: Hasher>: Unit<Hasher = H> {
215215
}
216216

217217
pub trait UnitWithParents: Unit {
218-
fn parents(&self) -> &NodeMap<HashFor<Self>>;
218+
fn parents(&self) -> impl Iterator<Item = &HashFor<Self>>;
219+
fn direct_parents(&self) -> impl Iterator<Item = &HashFor<Self>>;
220+
fn parent_for(&self, index: NodeIndex) -> Option<&HashFor<Self>>;
221+
fn node_count(&self) -> NodeCount;
219222
}
220223

221224
impl<H: Hasher, D: Data> Unit for FullUnit<H, D> {

0 commit comments

Comments
 (0)