-
Notifications
You must be signed in to change notification settings - Fork 16
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
Setup Script: PR for Issue 183 #242
Conversation
…r-contracts into rahul/deployment-scripts
…l me now, anyway i would have died for a good cause
…s setup script is a mirror of the e2e test:MilestoneLifecycle.t.sol
…i had not separated the logic for milestone manager in 2 separate contract
6fd3fe3
to
e3924e7
Compare
Just as a summary, since not everyone was present when we discussed this and when Felix and I had our session. @saxenism just FYI: we removed the split/view-version of the MilestoneManager, as it did not even help the contract size at all (due to the inheritance usage, the code of the view contract is copied to the MilestoneManager anyways). Code size of MilestoneManager without the split was 25.832 kB, with your split version, it was 25.833 kB (+1 byte). We reverted it, removed an unused function (an unused budget check), and brought the optimizoooor runs down to 5k from 10k, which reduced the contract size enough to pass the limit with 1-2 % of wiggle room. And your scripts work now with the latest changes from main. In the future, we will probably look for a more sustainable solution, as the MilestoneManager is currently the only contract of that size, but for now this is fine. I am currently reviewing the script(s) itself to see if we have all we need. Just tagging @0xNuggan here so you are up to date as well. I will ping everyone once I am happy with everything so the rest can review as well :) |
Great, although I'm not very sure as to what is happening since I'm pretty sure I already did rebase it and prior to that I kept on merging new changes quite often. Anyway, happy to know that it's up to date as of now. Regarding the milestone manager, yeah that's great. Lmk if I can contribute anyhow to the more sustainable solution. Sure, lmk how the review goes I'll change up things accordingly. Thanks. |
This reverts commit 013269e.
This PR handles the issue 183 and introduces two separate scripts. One is the deployment script and the other is a setup script.