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.

Sign up for GitHub

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

Jump to bottom

Audit/final commit fixes #1196

Merged
merged 64 commits into from
Jul 11, 2024
Merged

Audit/final commit fixes #1196

merged 64 commits into from
Jul 11, 2024

Conversation

ThomasPiellard
Copy link
Collaborator

Description

Branch based on the final commit of the Openzeppelin audit. The branch of the final commit has been kept so it could be referenced in the final report.

This branch contains some fixes after internal review, and also fixes the conflicts with gnark's master branch.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@ThomasPiellard
fix: fixed l-02
c171e57
@ThomasPiellard
style: divideByXMinusOne could -> divideByZH
4bdf2bf
@ThomasPiellard
style: verify_opening_linearised_polynomial -> compute_opening_linear…
123f49b 
…ised_polynomial
@ThomasPiellard
style: lagrangeOne -> lagrangeZero
1b1b0e7
@ThomasPiellard
fix: fixed L-04
5564595
@ThomasPiellard
fix: fixed M-04
8fa4348
@ThomasPiellard
fix: fixed L-04
e3fb37f
@ThomasPiellard
fix: fixed L-05
74ab1eb
@ThomasPiellard
fix: fixed some comments
b28e81e
@ThomasPiellard
fix: fixed comment solidity
77197b8
@ThomasPiellard
fix: fixed comment solidity
c82f17e
@ThomasPiellard
fix: fixed comment solidity
c1a4ac7
@ThomasPiellard
fix: fixed comment solidity
5feb98d
@ThomasPiellard
fix: fixed comment unmarshal
a2cd72b
@ThomasPiellard
fix: fixed comment prove
a5118c4
@ThomasPiellard
fix: used size in domain instead of recomputing it
313b88d
@ThomasPiellard
style: removed redundant if statement
83eea72
@ThomasPiellard
style: reuse variable
d16fc2b
@ThomasPiellard
feat: simplify loop constraint index
6fb9262
@ThomasPiellard
fix: remove redundant select
6a34ab1
@ThomasPiellard
fix: remove redundant computation
b596adc
@ThomasPiellard
fix: removed redundant computation pointer update
123d950
@ThomasPiellard
fix: fixed comment
5a1321a
@ThomasPiellard
fix: fixed comment
f7895c0
@ThomasPiellard
fix: fixed fold_h comment
5aa8b16
@ThomasPiellard
fix: fixed comment
6cee007
@ThomasPiellard
style: renamed H commitments constants
5512147
@ThomasPiellard
style: cleaner computation nb BSB commitments
8662c56
@ThomasPiellard
style: SHA2 constant to replace 0x2
95b6bc0
@ThomasPiellard
style: constant for fixed size proof
7c51024
@ThomasPiellard
Merge pull request Consensys#1 from ThomasPiellard/audit/N-04
da9fa2c 
Audit/n 04
@ThomasPiellard
Merge pull request Consensys#3 from ThomasPiellard/audit/N-02
f14e0dd 
Audit/n 02
@ThomasPiellard
Merge pull request Consensys#4 from ThomasPiellard/audit/N-03
987cb4f 
style: removed unused variable
@ThomasPiellard
Merge pull request Consensys#8 from ThomasPiellard/audit/L-05
969e97b 
fix: fixed L-05
@ThomasPiellard
Merge pull request Consensys#6 from ThomasPiellard/audit/L-07
a6d7ed7 
Audit/l 07
@ThomasPiellard
Merge pull request Consensys#7 from ThomasPiellard/audit/L-06
3018617 
Audit/l 06
@ThomasPiellard
Merge branch 'audit/base' into audit/L-08
3b6e77a
@ThomasPiellard
Merge pull request Consensys#5 from ThomasPiellard/audit/L-08
3330ad0 
Audit/l 08
@ThomasPiellard
Merge pull request Consensys#2 from ThomasPiellard/audit/N-01
d1e7512 
style: cleaner computation nb BSB commitments
@ThomasPiellard
Merge pull request Consensys#13 from ThomasPiellard/audit/N-05
54823b5 
style: fixed typos
@ThomasPiellard
Merge pull request Consensys#14 from ThomasPiellard/audit/H-01
08becf5 
Audit/h 01
@ThomasPiellard
Merge branch 'audit/base' into audit/L-02
758c67e
@ThomasPiellard
Merge pull request Consensys#10 from ThomasPiellard/audit/L-02
872a106 
fix: fixed l-02
@ThomasPiellard
Merge pull request Consensys#11 from ThomasPiellard/audit/M-04
e23bfe6 
fix: fixed M-04
@ThomasPiellard
style: fixed wrong EcMul comment
e4214df
@ThomasPiellard
fix: fixed typo PROOF_H_0_X -> PROOF_H_0_COM_X
46dc9bb
@ThomasPiellard
feat: simplified offset computation
171ec34
@ThomasPiellard
Merge branch 'master' into audit/final-commit-fixes
5039222
@ivokub ivokub self-requested a review July 11, 2024 11:10
ivokub
ivokub requested changes Jul 11, 2024
View reviewed changes
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Some conflicts when merging other changes. Otherwise looks good.

backend/plonk/bn254/solidity.go Outdated Show resolved Hide resolved
backend/plonk/bn254/solidity.go Outdated Show resolved Hide resolved
backend/plonk/bn254/solidity.go Outdated Show resolved Hide resolved
@ivokub ivokub self-requested a review July 11, 2024 12:07
ivokub
ivokub approved these changes Jul 11, 2024
View reviewed changes
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

All good now! Great work!

@ThomasPiellard ThomasPiellard marked this pull request as ready for review July 11, 2024 12:14
@ThomasPiellard ThomasPiellard merged commit 065027a into Consensys:master Jul 11, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivokub ivokub ivokub approved these changes

@gbotrel gbotrel gbotrel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ThomasPiellard @ivokub @gbotrel