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

Lelantus improvements #1012

Merged
merged 30 commits into from
Apr 14, 2021
Merged

Lelantus improvements #1012

merged 30 commits into from
Apr 14, 2021

Conversation

levonpetrosyan93
Copy link
Contributor

No description provided.

@reubenyap reubenyap requested a review from psolstice April 2, 2021 06:34
@lgtm-com
Copy link

lgtm-com bot commented Apr 2, 2021

This pull request introduces 3 alerts when merging a4bf27b into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 5, 2021

This pull request introduces 3 alerts when merging 7a9372b into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

psolstice
psolstice previously approved these changes Apr 5, 2021
@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2021

This pull request introduces 3 alerts when merging b2bfb13 into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

@@ -27,6 +28,8 @@ struct NthPower {

void go_next() {
pow *= num;
if (pow == Scalar(uint64_t(1)))
throw std::invalid_argument("NthPower resulted 1");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this has DoS potential because NthPower is used in muptiple places and not only with challenge value. I.e. attacker can set some value in proof to 1 and if the exception is not properly caught it'll crash all the nodes. It's better to move the check to higher level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have put this call in try-catch in verifier side. I think that is enoygh for preventing such risks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will doublecheck to verify that I did not missed it somewhere.

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2021

This pull request introduces 3 alerts when merging 86cdb95 into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2021

This pull request introduces 3 alerts when merging 532fe71 into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2021

This pull request introduces 3 alerts when merging 905dee5 into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

psolstice
psolstice previously approved these changes Apr 10, 2021
@lgtm-com
Copy link

lgtm-com bot commented Apr 13, 2021

This pull request introduces 3 alerts when merging 0e6190e into b39fa5f - view on LGTM.com

new alerts:

  • 2 for Commented-out code
  • 1 for Declaration hides variable

@reubenyap reubenyap merged commit 72d824c into master Apr 14, 2021
@reubenyap reubenyap deleted the lelantus-improvements branch April 14, 2021 13:25
@levonpetrosyan93 levonpetrosyan93 restored the lelantus-improvements branch April 15, 2021 10:32
@levonpetrosyan93 levonpetrosyan93 deleted the lelantus-improvements branch April 15, 2021 10:33
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.

4 participants