Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/parsec: dump-graphs output needed info and can replay with DKG #328

Merged
merged 1 commit into from Jul 5, 2019

Conversation

Projects
None yet
3 participants
@jeanphilippeD
Copy link
Contributor

commented Jul 4, 2019

Closes #324

We need additional info to be able to replay the DKG:

  • We need Parsec to be able to recreate DKG part, this depend on the
    output of the injected RNG: Store that output to be able to replay
    it.
  • We need to be able to dump/parse the Dkg Parts and Acks:
    Dump them in the comment only to not add too much noise to the graph
    (very long), and parse them back.
  • Also parse DkgStart.

During replay:

  • We need to create the ReplayRng for Parsec.
  • We need to ignore/not vote for our own DkgMessages as parsec will
    generate them.

Make the fake encrypt used for DKG deterministic so we can replay it.

Notes:
-Adding extra parameter to dump_graph to_file was triggering to many
param warning: Replace with a struct.
-Only write out rng if some values were output to avoid changing
existing dot files. Allow parsing with missing dkg_rng to parse
existing dot files.

Test:
Add new smoke test with a DKG.

feat/parsec: dump-graphs output needed info and can replay with DKG
We need additional info to be able to replay the DKG:
 - We need Parsec to be able to recreate DKG part, this depend on the
   output of the injected RNG: Store that output to be able to replay
   it.
 - We need to be able to dump/parse the Dkg Parts and Acks:
   Dump them in the comment only to not add too much noise to the graph
   (very long), and parse them back.
 - Also parse DkgStart.

During replay:
 - We need to create the ReplayRng for Parsec.
 - We need to ignore/not vote for our own DkgMessages as parsec will
   generate them.

Make the fake encrypt used for DKG deterministic so we can replay it.

Notes:
-Adding extra parameter to dump_graph to_file was triggering to many
 param warning: Replace with a struct.
-Only write out rng if some values were output to avoid changing
 existing dot files. Allow parsing with missing dkg_rng to parse
 existing dot files.

Test:
Add new smoke test with a DKG.

@jeanphilippeD jeanphilippeD requested a review from maqi Jul 4, 2019

@jeanphilippeD jeanphilippeD requested a review from pierrechevalier83 as a code owner Jul 4, 2019

@jeanphilippeD jeanphilippeD requested review from fizyk20 and removed request for pierrechevalier83 Jul 4, 2019

Show resolved Hide resolved src/dev_utils/dot_parser.rs
let parser_vec =
(seq(b"[") * list(parser_u32, seq(b", ")) - seq(b"]")).map(|v| v.into_iter().collect());
let parser = comment_prefix() * seq(b"dkg_rng: ") * parser_vec - next_line();
parser.opt().map(|dkg_rng| dkg_rng.unwrap_or_default())

This comment has been minimized.

Copy link
@maqi

maqi Jul 5, 2019

Member

just to confirm, so in case of failure, there will be a default value to be used?
If so, will it be better to at least put a comment?
I'd prefer a failure or panic in case of parse failure (alarm a file failure directly)

This comment has been minimized.

Copy link
@jeanphilippeD

jeanphilippeD Jul 5, 2019

Author Contributor

This is not an error, this is optional in the dot file.
So if it is not present, empty vector will be used (this is similar to consensus mode).

Show resolved Hide resolved src/dev_utils/pseudo_random.rs Outdated
Show resolved Hide resolved src/dev_utils/record.rs Outdated
Show resolved Hide resolved src/dump_graph.rs Outdated
Show resolved Hide resolved src/mock.rs Outdated

@jeanphilippeD jeanphilippeD added the fixups label Jul 5, 2019

@jeanphilippeD jeanphilippeD dismissed stale reviews from maqi and fizyk20 via cfe6208 Jul 5, 2019

@jeanphilippeD jeanphilippeD force-pushed the jeanphilippeD:replay_dkg branch from 8433f36 to cfe6208 Jul 5, 2019

@jeanphilippeD jeanphilippeD removed the fixups label Jul 5, 2019

@maqi

maqi approved these changes Jul 5, 2019

@jeanphilippeD jeanphilippeD merged commit 3117372 into maidsafe:master Jul 5, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@jeanphilippeD jeanphilippeD deleted the jeanphilippeD:replay_dkg branch Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.