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

Modularize contracts package #92

Merged
merged 3 commits into from
Dec 11, 2023
Merged

Modularize contracts package #92

merged 3 commits into from
Dec 11, 2023

Conversation

0xjei
Copy link
Member

@0xjei 0xjei commented Dec 4, 2023

Description

This PR is intended to bring about the necessary improvements as stated in proposal #66.

To Do

  • Create a Constants.sol file for the SNARK_SCALAR_FIELD and MAX_DEPTH constants.
  • Update IMT implementations according to the proposal.
  • Update tests
  • Run benchmarks

Related Issue

See #66

Does this introduce a breaking change?

  • Yes
  • No

Other information

None

@0xjei 0xjei self-assigned this Dec 4, 2023
@0xjei
Copy link
Member Author

0xjei commented Dec 4, 2023

Before

Screenshot from 2023-12-04 11-57-56

After

image

After #93 from @cedoor

image

@0xjei 0xjei requested a review from cedoor December 4, 2023 14:37
@0xjei 0xjei marked this pull request as ready for review December 4, 2023 14:37
@cedoor
Copy link
Member

cedoor commented Dec 5, 2023

Hey @RayHuang880301 Could you also take a look at this PR to see if it's what you expected?

What is not clear to me yet is point 2 of your message:

For developers who want to use internal libraries to save run time gas costs, they can directly inherit IncrementalBinaryTreeInternal.sol to their main contract.

Libraries, from what I know, cannot be inherited. So, how could you use an internal function of a library in a contract?

@RayHuang880301
Copy link

Hey @0xjei @cedoor
Thank you very much for your work. It seems that this is exactly what I expected. 👍

Libraries, from what I know, cannot be inherited. So, how could you use an internal function of a library in a contract?

Because a library is used here instead of a contract, so as you said, import should be used here instead of inherit. It looks good to me now. Thank you!

@0xjei
Copy link
Member Author

0xjei commented Dec 11, 2023

Hey @0xjei @cedoor Thank you very much for your work. It seems that this is exactly what I expected. 👍

Libraries, from what I know, cannot be inherited. So, how could you use an internal function of a library in a contract?

Because a library is used here instead of a contract, so as you said, import should be used here instead of inherit. It looks good to me now. Thank you!

Thank you @RayHuang880301 :) @cedoor, green lights

@cedoor cedoor merged commit f24903e into main Dec 11, 2023
2 checks passed
@cedoor cedoor deleted the feat/modular-contracts branch December 11, 2023 12:42
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.

3 participants