test: MPC + stream component test#739
Conversation
volovyks
left a comment
There was a problem hiding this comment.
Nice! test_channel_contention should be very useful in our work on backlog and stability.
| crate::metrics::nodes::CONFIGURATION_DIGEST.set(digest); | ||
|
|
||
| let (sign_tx, sign_rx) = mpsc::channel(16384); | ||
| let (sign_tx, sign_rx) = mpsc::channel(if cfg!(test) { 1 } else { 16384 }); |
There was a problem hiding this comment.
I guess this is more of an experiment, but we can make it configurable in tests.
There was a problem hiding this comment.
not related to this PR, but it definitely deserves a constant
There was a problem hiding this comment.
yeah it's just an experiment that I will clean up before removing the draft label
| guard.sign_requests(requests) | ||
| } | ||
|
|
||
| pub async fn rpc_actions(&self, actions: &[RpcAction]) { |
There was a problem hiding this comment.
It is not clear what we are doing with these actions. Are we queuing them? Processing? Adding to a specific block? I'm ok with a bit longer names. That makes the code a bit more readable IMO.
| network[1].mock_streams[0].progress_block_height(1).await; | ||
| network[2].mock_streams[0].progress_block_height(1).await; | ||
|
|
||
| let timeout = Duration::from_secs(10); |
There was a problem hiding this comment.
nit: Such a pattern significantly slows down our tests; I would add a helper that actually waits for N events or specific events with a timeout.
There was a problem hiding this comment.
Wait, doesn't this already do exactly what you ask?
let timeout = Duration::from_secs(10);
let actions = network.assert_actions(1, timeout).await; pub async fn assert_actions(
&self,
threshold_per_node: usize,
timeout: Duration,
) -> HashSet<String> {
let result = tokio::time::timeout(timeout, self.wait_for_actions(threshold_per_node)).await;
if result.is_err() {
self.print_actions().await;
}
result.expect("should produce enough signatures")
}There was a problem hiding this comment.
Yes, but why do we have the timeout? What if it happens faster?
There was a problem hiding this comment.
tokio::time::timeout returns early in that case
wait_for_actions keeps polling at a 100ms interval to check if there are enough actions
|
I'm still experimenting here. Getting some interesting results.
I will take a closer look again next week. |
b24050f to
5fd34dc
Compare
Create a MockStream that connects to the MPC fixture setup. This lets us test the indexer stream <-> MPC glue.
but not with channel capacity?
otherwise it is always the same participant in round 0
adding useful cases and checking if they run in ci
8cd5c90 to
b883f27
Compare
|
The issue from my previous comment was that I didn't use good entropy in requests. Hence, at round 0, all requests went to participant 0 (first 25 requests) and after one timeout in round 2, all requests went to participant 2. That's fixed now and it actually works rather stable, even when adding 1M requests all at once. |
| #[test(tokio::test(flavor = "multi_thread"))] | ||
| async fn test_channel_contention_multiple_blocks_at_once_delayed() { | ||
| // delay should be > ORGANIZE_POSIT_TIMEOUT | ||
| let delay = mpc_node::protocol::signature::organize_posit_timeout() * 3 / 2; |
There was a problem hiding this comment.
Changing this to less than a full posit delay make the test pass. Hence we know we only run into the issue when observations are apart more than a posit timeout (20s).
| let delay = mpc_node::protocol::signature::organize_posit_timeout() * 3 / 2; | |
| let delay = mpc_node::protocol::signature::organize_posit_timeout() / 2; |
|
Somehow I only changed the entropy of the signature requests. This suggests that "unlucky" proposer assignment can cause long delays, too. I tried increasing the timeout and using different seeds. But nothing works except the entropy currently on develop. That one assigned all requests to the same participant in any given round. (round 0, proposer = Participant(0); round 1, proposer = Participant(1); round 2, proposer = Participant(2)) Apparently that's the only way we have no presignature waste 😕 |
|
Fix in #765 |



Create a MockStream that connects to the MPC fixture setup. This lets us test the indexer stream <-> MPC glue.