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

Optimisations, CI workflow, & deploy script #190

Closed
wants to merge 3 commits into from

Conversation

diego-G
Copy link
Contributor

@diego-G diego-G commented Jun 1, 2023

Closes #189 , #188 , #133

mariogutval and others added 3 commits June 1, 2023 16:30
* For loops and function arguments optimization
* Substitute require statements with custom errors
* Add deploy script for CMTAT

---------

Co-authored-by: Diego G <[email protected]>
@rya-sge
Copy link
Collaborator

rya-sge commented Jun 2, 2023

Closes #131, #61

@rya-sge rya-sge self-requested a review June 2, 2023 15:33
@rya-sge
Copy link
Collaborator

rya-sge commented Jun 5, 2023

Thank you very much for your great work.

For the custom errors, I saw that currently there is no support of it with truffle/ganache for the tests ("expectRevert.unspecified").
I am not sure if it is the good idea to use it if we do not have a way to test the result.
trufflesuite/truffle#5753

@diego-G
Copy link
Contributor Author

diego-G commented Jun 5, 2023

Thank you very much for your great work.

For the custom errors, I saw that currently there is no support of it with truffle/ganache for the tests ("expectRevert.unspecified"). I am not sure if it is the good idea to use it if we do not have a way to test the result. trufflesuite/truffle#5753

Yes, we already discussed the same topic internally. We preferred to include the optimisation because it's significant enough. This TODO in the code should be addressed as soon as Truffle implements the feature. I will open an issue to remember it.

@rya-sge
Copy link
Collaborator

rya-sge commented Jun 6, 2023

Ok perfect, thank you for your work

Perhaps it will be possible to test that with Foundry if one day the actual tests are updated.

I think we can add the support of custom errors for the future version 2.4.

The version 2.3 will be an audited version, recommended by the CMTA, and I am not really comfortable by adding a functionality which is not audited and not entirely tested.

So here the "planning":

  1. Release 2.3 in the next weeks.
  2. Cherry-pick the different commits or merge the PR
  3. Complete the tests when it is available with Truffle or use Foundry

@rya-sge
Copy link
Collaborator

rya-sge commented Jun 6, 2023

Commit 4cb6320 is included in pull/193

@diego-G diego-G closed this Jul 25, 2023
@diego-G diego-G deleted the feature/optimisations-ci-deploy branch July 25, 2023 08:31
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.

Add CI Github action
3 participants