unlynx_v2 the big and great cleanup #14
Conversation
Changed shuffle proofs to account for any possible use for them (e.g.…
… refactor of the remaining proofs
…el have been correctly added
…o not use the global variable Groups
app/client.go
Outdated
| if err != nil { | ||
| log.Fatal("Service did not start.", err) | ||
| } | ||
|
|
||
| grp, aggr, err := client.SendSurveyResultsQuery(*surveyID) | ||
| if err != nil { | ||
| log.Fatal("Service could not output the results.") | ||
| log.Fatal("Service could not output the results.", err) |
There was a problem hiding this comment.
Is this really Fatal, or should this just return the error? Fatal really means: "the internal state of the conode is inconsistent and every further action might make it worse". While here I think it should only be: "oups, something went wrong. Let's discard what happened and wait for a new request."
There was a problem hiding this comment.
Same thing for all Fatals in the rest of the code...
There was a problem hiding this comment.
I am checking all log.Fatals now. I will add a commit after reviewing all of them
data/handle_data.go
Outdated
| @@ -17,7 +17,7 @@ import ( | |||
| ) | |||
|
|
|||
| // Groups identifies all different groups to be added to the test data file | |||
| var Groups [][]int64 | |||
| //var Groups [][]int64 | |||
There was a problem hiding this comment.
Style: remove commented out code sections.
data/handle_data.go
Outdated
| @@ -177,11 +185,18 @@ func WriteDataToFile(filename string, testData map[string][]libunlynx.DpClearRes | |||
| } | |||
|
|
|||
| writer := bufio.NewWriter(fileHandle) | |||
| if err := writer.Flush(); err != nil { | |||
| log.Fatal(err) | |||
There was a problem hiding this comment.
like above: it's much cleaner to return an error and let the caller decide what should happen...
|
Another downside of huge PRs: I don't even know where to start ;) Is there something specific that you would like me to look at? |
Co-Authored-By: JoaoAndreSa <jagsousa1992@gmail.com>
|
Apparently the previous bug in issue #11 was not fixed. I changed the service so that it waits for all the nodes to get the survey before performing the remaining operations/protocols: 083a047. This time I am sure it will work... @ineiti you told me something about a synchronous broadcast that waits until all nodes have gotten the message, but I only found something of that kind for the protocols: |
|
In fact it's the https://github.com/dedis/cothority/blob/master/skipchain/skipchain.go |
I finished the big refactoring cleanup of Unlynx (let us call it v2 :P). I already merged it with master, but @ineiti if you see something that have missed or wrongly merged please let me know.
Since there are quite a lot of commits here is a list of the main changes:
1. Refactor of packages: I changed some of the packages:
2. Cleanup: Deleted some unused code and added some more logs to catch all those naughty errors
3. Proofs: Homogenised and added hook functions for the proofs to define what to do with them (e.g. in unlynx we just create the proofs while in drynx we create them and send them to the skipchain).
4. Removed the PARALLELIZE flag: The parallelization is now by default.
5. Added a new protocol: shuffling+ddt protocol (does the shuffling and ddt in one single round)
6. Fixed #11 : just added the same
for el == nil {...}in the HandleSurveyResponseQuery() to the HandleSurveyResultsQuery()Please take a look at it and let me know if I should add or change things. This is the perfect time to do it (before I send it to master and create a new release).