search

cryptimeleon / incentive-system

A cryptographic incentive system.
Apache License 2.0
3 stars 0 forks source link

Feature/spend deduct crypto #28

Closed this-kramer closed 3 years ago

this-kramer commented 3 years ago

Reason for this PR:

Changes in this PR:

Checklist:

Select whatever applies:

JanBobolz commented 3 years ago

Just as a small remark: I personally like Patrick's code comments like // modify precommitment 0 using homorphism trick and randomly chosen exponent.

It gives much more meaning to what's happening in the code. You have sectioned your code using comments too, which is good, but your comments could be more cryptographically relevant (the current ones seem to be mostly "compute this. Now compute this").

But to be honest, this is just a matter of personal preference. Don't rewrite everything brecause of this. It's been easily readable (for me) as is.

this-kramer commented 3 years ago

Just as a small remark: I personally like Patrick's code comments like // modify precommitment 0 using homorphism trick and randomly chosen exponent.

It gives much more meaning to what's happening in the code. You have sectioned your code using comments too, which is good, but your comments could be more cryptographically relevant (the current ones seem to be mostly "compute this. Now compute this").

But to be honest, this is just a matter of personal preference. Don't rewrite everything brecause of this. It's been easily readable (for me) as is.

I fixed all of your comments (and learned a lot about math and craco :smiley:). I will go through the code and sprinkle more knowledge using comments next week. Feel free to review my changes before that :)

this-kramer commented 3 years ago

Looks good to me, altough I do not understand why some of the classes are marked as completely changed though I am 100% sure to have already seen them in my last review.

You always see the changes of this PR, not changes made since the last review. For that, see the resolved converstations.