Skip to content
This repository was archived by the owner on Apr 20, 2021. It is now read-only.

Comments

Initial PR#74

Closed
hskang9 wants to merge 9 commits intoLayerXcom:masterfrom
hskang9:accumulator/prime_tester
Closed

Initial PR#74
hskang9 wants to merge 9 commits intoLayerXcom:masterfrom
hskang9:accumulator/prime_tester

Conversation

@hskang9
Copy link

@hskang9 hskang9 commented Jan 21, 2019 •
edited
Loading

What I did

resolve #73

I will extend this to have:

  • solve easy answers(e.g. 2,3,5,7)
  • Resolve Fermat liars if they exist

Cute Animal Picture

pic

@hskang9 hskang9 changed the title WIP: Add prime tester to accumulator Add prime tester to accumulator Jan 21, 2019
nrryuya
nrryuya suggested changes Jan 21, 2019
View reviewed changes
contracts/accumulator/isPrime.vy
@private
@constant
def _compare(_a: uint256[8], _b: uint256[8]) -> int128:
for i in range(M_LIST_LENGTH):
Copy link
Member

@nrryuya nrryuya Jan 21, 2019

Choose a reason for hiding this comment

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

I got this error here ;)

line 8: Range only accepts literal (constant) values
    for i in range(M_LIST_LENGTH):
-------------------^

@nrryuya
Copy link
Member

nrryuya commented Jan 22, 2019

@hskang9
Thank you! A few comments.

  • Could you rebase to the latest master branch and create a new PR? This inlined version is WIP and I think it'd be better to have a new directory and file contracts/prime/isPrime.vy because this contract can be used as stand-alone.
  • Tests are always necessary ;) Please add tests at tests/prime/test_isPrime.py.

@nrryuya nrryuya mentioned this pull request Jan 22, 2019
Closed
@nrryuya
Copy link
Member

nrryuya commented Jan 22, 2019 •
edited
Loading

@hskang9
About tests, I'll prepare BigInt.vy which can be used by isPrime.vy in #79

@nrryuya nrryuya mentioned this pull request Jan 22, 2019
Open
@hskang9
Copy link
Author

hskang9 commented Jan 22, 2019

Thank you! @nrryuya now that I have Big integer operations in separate file, I can focus on the tester. I will create WIP to master branch based on the comments received 👍

@hskang9
Copy link
Author

hskang9 commented Jan 22, 2019

Closing this for new PR

@hskang9 hskang9 closed this Jan 22, 2019
@nrryuya
Copy link
Member

nrryuya commented Jan 22, 2019

@hskang9
#79 is still WIP so it's OK to wait for my work or to prepare only test cases (without actually executing them) ;)

@hskang9
Copy link
Author

hskang9 commented Jan 22, 2019 •
edited
Loading

I will put TODO in comment to remove big integer operations after BigInt.vy is developed.

@hskang9 hskang9 changed the title Add prime tester to accumulator Initial commit Jan 23, 2019
@hskang9 hskang9 changed the title Initial commit Initial PR Jan 23, 2019
@hskang9
Copy link
Author

hskang9 commented Jan 26, 2019

This PR is now moved to #81

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@nrryuya nrryuya nrryuya requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RSA Accumulator] Use Fermat Primality Test for verifying prime number

2 participants

@hskang9 @nrryuya