Jump to conversation
Unresolved conversations (3)
@davidrusu davidrusu Jan 19, 2023
Nice to see some randomized testing, but can we modify this to be a quickcheck test?
src/mvba/consensus.rs
b00f
@davidrusu davidrusu Jan 19, 2023
I think we should continue in a loop forever going back to the first proposer. It could be that we hit the first guy too early. Now that some time has passed their proposal had enough time to reach the other nodes.
src/mvba/consensus.rs
davidrusu b00f
@davidrusu davidrusu Jan 19, 2023
I think this should return an error. The caller is trying to vote twice. The caller can use the error as a signal to buffer their vote and try again after this instance completes. We do something like this in safe_network for membership.
src/mvba/abba/mod.rs
davidrusu b00f
Resolved conversations (6)
@davidrusu davidrusu Jan 19, 2023
```suggestion if votes.values().any(|v| v.value) { // if there is some uj = 1 then // v ← 1; ρ ← ρj self.v = Some(true); } else { // else // v ← 0; ρ ← ⊥ self.v = Some(false); } ```
Outdated
src/mvba/mvba/mod.rs
@davidrusu davidrusu Jan 19, 2023
Can you add a comment explaining why we are pre-voting again after we've decided?
Outdated
src/mvba/consensus.rs
davidrusu b00f
@davidrusu davidrusu Jan 19, 2023
We need to protect against the case where the initiator is faulty and refuses to send the request. How about we make this a broadcast instead of a direct message?
Outdated
src/mvba/consensus.rs
b00f
@davidrusu davidrusu Jan 19, 2023
Can you add a comment explaining why we check for an ABBA decision here? It was not obvious why this vcbc message should be connected to an abba decision
Outdated
src/mvba/consensus.rs
davidrusu b00f
@davidrusu davidrusu Jan 19, 2023
No need to manually clear self.outgoing, `std::mem::take` will replace it with an empty vec. ```suggestion pub fn take_outgoings(&mut self) -> Vec<Outgoing> { std::mem::take(&mut self.outgoings) } ```
Outdated
src/mvba/broadcaster.rs
@davidrusu davidrusu Jan 19, 2023
We can't have panics. ```suggestion pub fn decided_value(&self) -> Option<bool> { self.decided_value.as_ref().map(|v| match v.value { Value::One => true, Value::Zero => false, }) } ```
Outdated
src/mvba/abba/mod.rs