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

Deployed contract addresses saved to file #200

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

0xJem
Copy link
Contributor

@0xJem 0xJem commented Feb 8, 2022

This PR throws out the previous approach of storing the deployment files (any time a contract was updated, it would invalidate the old file and require a manual update of the constants in olympus-frontend).

Instead, we now write the addresses into a JSON file in addresses/addresses.json. This can be used by a local dev, but can also be passed to the frontend and ingested dynamically. Magic!

@0xJem 0xJem force-pushed the dynamic-contract-addresses branch from 6b6341b to 918da04 Compare March 16, 2022 06:07
@OlympusDAO OlympusDAO deleted a comment from 0xJem Mar 24, 2022
@OlympusDAO OlympusDAO deleted a comment from 0xJem Mar 24, 2022
@OlympusDAO OlympusDAO deleted a comment from 0xJem Mar 24, 2022
@ghost
Copy link

ghost commented Mar 31, 2022

We really need a sitdown on this topic. I would leave this open simply for this reason. I've been writing mini util with importable addresses in /test/utils on the allocator branch, and was thinking about directly writing. I don't see why we should rely on something like hardhat-deploy to do stuff for us when we can just code it ourselves.

@philipjonsen
Copy link

Yes, smart contracts are the root of all DAO projects. But integrating and and working could with them could be better, before any broader applications this should be checked one, five or 10-100 times by Security professionals.

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.

2 participants