Conversation
|
replaces #451 after a rebase and making it run on top of all the latest refactors Somehow, the normal test build is now way slower when generating triples. Running them with --release let's me successfully finish the triple test in 2s (after building) while in debug, 180s is not enough of a timeout. Still looking into this... |
Fixed with [profile.dev.package.curve25519-dalek]
opt-level = 3This will increase build time, so it's not entirely for free... |
ChaoticTempest
left a comment
ChaoticTempest
left a comment
There was a problem hiding this comment.
nice work, this will be great to add in since we'll have a way to directly test our changes more easily
integration-tests/tests/cases/mpc.rs
Outdated
| .expect("should have enough presignatures eventually"); | ||
|
|
||
| let mut conn = network.redis_container.pool().get().await.unwrap(); | ||
| let mut data = BTreeMap::new(); |
There was a problem hiding this comment.
was there something we needed to do with data after collecting the shares?
There was a problem hiding this comment.
No, we just collect it to then write it out to a file if WRITE_OUTPUT_TO_FILES is true
It might be better to put the entire thing inside the if WRITE_OUTPUT_TO_FILES { } block to make that clearer. I will do so
| #[derive(Clone)] | ||
| pub struct NodeStateWatcher { | ||
| watcher: watch::Receiver<NodeStatus>, | ||
| // Note: this gives access to the private key share and should not be |
There was a problem hiding this comment.
@ppca @auto-mausx, be aware of this flag. Make sure we never use it to create production builds.
| pub fn load(num_nodes: u32) -> Self { | ||
| let data = match num_nodes { | ||
| 3 => include_str!("./3_nodes.json"), | ||
| 5 => include_str!("./5_nodes.json"), |
There was a problem hiding this comment.
3 nodes setup has triples in the JSON, but 5 nodes setup does not. Is that intentional? Should we represent that in the file name or elsewhere?
There was a problem hiding this comment.
It's semi-intentional :)
Eventually, both files should be complete with key shares, triples and presignatures. Right now, 5 nodes setup only has key shares. That's just enough for the current tests.
I still would like to commit it this way. This shows that the framework allows adding data gradually and have tests succeed as long as the data we actually need is available. This makes it convenient to add more tests and generate data as needed. (Earlier iterations were less flexible and would crash if an incomplete data file is included.)
integration-tests/tests/cases/mpc.rs
Outdated
| /// Use this toggle locally to regenerate hard-coded inputs such as key shares, | ||
| /// triples, and presignatures. | ||
| /// You might have to create the directory `integrations-tests/tmp` first. | ||
| const WRITE_OUTPUT_TO_FILES: bool = true; |
There was a problem hiding this comment.
Will this generate a file each time I run the tests? Should it be false by default?
There was a problem hiding this comment.
Yes, good catch, will set it to false
|
|
||
| drop(actions); | ||
| tokio::time::sleep(interval).await; | ||
| } |
There was a problem hiding this comment.
Should we check the emitted action here?
There was a problem hiding this comment.
Right now, I only check that there is exactly one action.
I think it would be better to check the type of it (RpcAction::Publish), too. For now, it's redundant, as there are no other RpcAction variants. But I will add it anyway, it will make sense to have it in the future.
Removing the dependency makes the code more modular and testable. The NearClient is used to read the governance smart contract on-chain state. Currently, this is on near but it may change in the future. Aside from what we call the ConsensusProtocol, the MPC protocols don't need to know about the governance. Thus, I added a new parameter to the consensus protocol. Now we can test other protocol without a NearClient. Finally, I replace the NearClient parameter with a generic type parameter, to allow mocking it in tests.
Testing framework for testing the MPC component without a real network. These tests create a test fixture with an in-memory node-to-node network as well as a in-memory shared governance contract proxy. With that, MPC testing should be almost entirely deterministic. The only external dependency a test fixture still needs is a redis container. This separation of the MPC component from the rest of the system is a first move in a general new direction of testing. The long-term goal is to replace the few integration tests that run in CI today with a much larger number of tests that are more light-weight and deerministic. While the component tests for MPC are still few and simple, the setup lays down a foundation for many more tests. The new framework gives total control for the order in which messages are received, including malicious messages. Adding such tests will come in a separate PR to not make this PR even larger. (key sharing, triple generation, presignature genreation, signature)
This improves the time for `test_basic_generate_triples` from ~320s down to ~16s on my machine.
Reading it dynamically from the filesystem makes problems in CI. Let's just include it as static data into the binary, to avoid such problems.
- don't generate files by default - don't collect data if we don't print or use it - assert on rpc action type for a generated signature
jakmeier
force-pushed
the
mpc-testing-v2.1
branch
from
463d2f6 to
bc76bed
Compare
|
I rebased to resolve merge conflicts. Please approve again so I can merge it :) @volovyks @ChaoticTempest |
3 participants
Testing framework for testing the MPC component without a real network.
These tests create a test fixture with an in-memory node-to-node network
as well as a in-memory shared governance contract proxy. With that, MPC
testing should be almost entirely deterministic.
The only external dependency a test fixture still needs is a redis
container.
This separation of the MPC component from the rest of the system is a
first move in a general new direction of testing. The long-term goal
is to replace the few integration tests that run in CI today with a much
larger number of tests that are more light-weight and deerministic.
While the component tests for MPC are still few and simple, the setup
lays down a foundation for many more tests. The new framework gives
total control for the order in which messages are received, including
malicious messages. Adding such tests will come in a separate PR to
not make this PR even larger.
(key sharing, triple generation, presignature genreation, signature)