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

GodMode import on MCD.sol causes issues on spells #40

Open
amusingaxl opened this issue Mar 26, 2024 · 1 comment
Open

GodMode import on MCD.sol causes issues on spells #40

amusingaxl opened this issue Mar 26, 2024 · 1 comment

Comments

@amusingaxl
Copy link
Contributor

amusingaxl commented Mar 26, 2024

This issue can be spotted in the wild throughout different spells:

2024-03-26: https://etherscan.io/address/0xcD672aCc9885796a19b4bAf03Dba46c8cdB0882B#code
2023-07-14: https://etherscan.io/address/0x402D46A20C849390Da96CeB0C3c04832D29e87d7#code
2023-05-01: https://etherscan.io/address/0x77107F74bf30250aFFada0fbD09fa517658B4916#code

The new-ish pattern of self-contained libs in spells for onboarding new modules usually import some contracts that were initially meant for testing purposes only.

This has an undesired side effect of causing the spell deployment to pull a lot of extraneous dependencies, that not only bloat the contract verification, but also inflate the contract deployed bytecode size.

While it's hard to compare directly, because different spells have different bytecode size, comparing the transactions that created the spell for 2024-03-26 against the one of 2024-03-08 shows us that the former costed around $4,800,761$ gas, while the latter costed $2,228,419$. That's a $115\%$ difference! Part of that can be explained by the imported but unused dependencies.

We should probably refactor MCD.sol and split it in 2 separate files, one meant to be used for testing and the other meant to be used in contracts that will actually be deployed.

@SidestreamColdMelon
Copy link
Collaborator

I'm actually not sure if it's strictly dss-test-related problem. Looking at the repository name, it might not be dss-test responsibility to provide reusable primitives useful outside of the tests. If a primitive is indeed useful, it should either be manually copied out and stripped from irrelevant functions and imports or another place should be found (like the exec lib)

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

No branches or pull requests

2 participants