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

Check flag on simulation #264

Merged
merged 12 commits into from
Mar 16, 2016
Merged

Check flag on simulation #264

merged 12 commits into from
Mar 16, 2016

Conversation

nikkolasg
Copy link
Contributor

Refers to #260

@nikkolasg nikkolasg self-assigned this Mar 14, 2016
// 1 - check only at root
// 2 - check at each level of the tree
VerifyResponse int

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of giving it as a parameter in the message, you can also use a setter and call that setter in https://github.com/dedis/cothority/blob/master/lib/sda/simulation.go#L43 - which will be somewhere in https://github.com/dedis/cothority/blob/check/protocols/cosi/simulation.go#L83 - if you do that, be sure to call

cs.SimulationBFTree.Node( config )

before, so that the tree and the entityList are set up. This setter has access to all config.toml-parameters, too. So you can directly set any parameter from the simulation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make an example because I don't see how can I access each node in this function.
Moreover, when we will attack the external api issue, a lot of things will be able to pass through that.
Anyway, it's only for simulation anyway, all theses protocols should not be used as something else...!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you go for an example:

#273


ln := tree.ListNodes()
randomNode := ln[rand.Intn(len(ln))]
var idx int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, some serious testing :-)

@ineiti
Copy link
Member

ineiti commented Mar 16, 2016

+1 for merge

@liamsi
Copy link
Member

liamsi commented Mar 16, 2016

Not sure if this related to changes made here, but running the simulation with debug level 3 shows quite some race conditions.

@ineiti
Copy link
Member

ineiti commented Mar 16, 2016

Thanks for catching that - has to be fixed. What simulation were you running?

@liamsi
Copy link
Member

liamsi commented Mar 16, 2016

test_jvss.toml

@nikkolasg
Copy link
Contributor Author

Race conditions on CoSi is removed from another PR now merged into development.
Race conditions for JVSS is to be examined, and there is already an issue for refactoring JVSS. However, I'd say this is not high priority right now ..

@liamsi
Copy link
Member

liamsi commented Mar 16, 2016

Race conditions for JVSS is to be examined, and there is already an issue for refactoring JVSS. However, I'd say this is not high priority right now ..

Agreed.

liamsi added a commit that referenced this pull request Mar 16, 2016
Merged: Check flag on simulation
@liamsi liamsi merged commit c5af548 into development Mar 16, 2016
@liamsi liamsi deleted the check branch March 16, 2016 16:53
@liamsi liamsi mentioned this pull request Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants