apollo_consensus: use instance fields instead of constants in sim test

[asmaastarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[asmaastarkware]

• apollo_consensus: use instance fields instead of constants in sim test #11035 👈 (View in Graphite)
• apollo_consensus: add faulty nodes to the sim test #10964
• apollo_consensus: update consensus sim test to simulate network msg drops #10869 : 1 other dependent PR (#11016 )
• apollo_consensus: add honest nodes consensus simulation #10868
• main-v0.14.1-committer

This stack of pull requests is managed by Graphite. Learn more about stacking.

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @asmaastarkware and @dafnamatsry).

---

crates/apollo_consensus/src/simulation_test.rs line 117 at r1 (raw file):

    honest_nodes: usize,
    /// Threshold for reaching consensus (2/3 + 1 of total nodes).
    threshold: usize,

Too broad IMO
vote_threshold?

Code quote:

threshold

---

crates/apollo_consensus/src/simulation_test.rs line 604 at r1 (raw file):

fn test_consensus_simulation(keep_ratio: f64, honest_nodes: usize) {
    let seed = rand::thread_rng().gen();
    let total_nodes = 100;

Consider defining the number of honest nodes as a fraction of the total

Code quote:

let total_nodes = 100;

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dafnamatsry).

[dafnamatsry]

@dafnamatsry reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @asmaastarkware).

---

crates/apollo_consensus/src/simulation_test.rs line 479 at r6 (raw file):

            } else {
                round_rng.gen_range(1..total_nodes)
            };

This code is identical to get_leader_index, which you used before.

I think you can either:

• Make get_leader_index accept the seed and total_nodes instead of self.
• Clone self just before this defining the closure and and then you can call it:

let this = self.clone();
let leader_fn = move |r: Round| {
      let idx = this.get_leader_index(r);
      validators[idx]
 };

Code quote:

        let leader_fn = move |r: Round| {
            let round_u64 = u64::from(r);
            let round_seed = seed.wrapping_mul(31).wrapping_add(round_u64);
            let mut round_rng = StdRng::seed_from_u64(round_seed);
            let random_value: f64 = round_rng.gen();
            let idx = if random_value < NODE_0_LEADER_PROBABILITY {
                NODE_UNDER_TEST
            } else {
                round_rng.gen_range(1..total_nodes)
            };

[asmaastarkware]

@asmaastarkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dafnamatsry).

---

crates/apollo_consensus/src/simulation_test.rs line 479 at r6 (raw file):

Previously, dafnamatsry wrote…

This code is identical to get_leader_index, which you used before.

I think you can either:

• Make get_leader_index accept the seed and total_nodes instead of self.
• Clone self just before this defining the closure and and then you can call it:

let this = self.clone();
let leader_fn = move |r: Round| {
      let idx = this.get_leader_index(r);
      validators[idx]
 };

Done.

[dafnamatsry]

@dafnamatsry reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @asmaastarkware).