ethereum-optimism / optimism

Optimism is Ethereum, scaled.
https://optimism.io
MIT License
5.65k stars 3.28k forks source link

`CONTRACT_ADDRESSES` does not include op devnet addresses #7643

Open Arachnid opened 1 year ago

Arachnid commented 1 year ago

chain-constants.ts has a mapping CONTRACT_ADDRESSES used to look up contract addresses for various chains. It doesn't have any values for the op devnet (chain ID 901), and so all of these have to be supplied manually by the caller. It'd be good if these were prepopulated.

tynes commented 1 year ago

Devnet addresses are dynamic and subject to change, you can get them at GET localhost:8080/addresses.json if you use the local devnet setup

Arachnid commented 1 year ago

That's good to know. Wouldn't it be a good idea if devnet addresses were deterministic, though?

Arachnid commented 1 year ago

Running make devnet-up doesn't result in anything serving on port 8080. I've tried the various ports exposed by the containers it starts up, but none of them return anything on /addresses.json.

tynes commented 1 year ago

That's good to know. Wouldn't it be a good idea if devnet addresses were deterministic, though?

It would be nice for integrators but then it would add constraints to the way the system is deployed, if we did deployments with a create3 sort of design then it could work but we don't have time to refactor our deploy scripts to work like that right now.

You can see in the docker-compose file that an artifacts server is created, I will double check locally to ensure that there have been no regressions

https://github.com/ethereum-optimism/optimism/blob/126454e907f3dae4c83914c9379112ae7e7825cc/ops-bedrock/docker-compose.yml#L135-L144

Arachnid commented 1 year ago

Why would you need a create3 design? Addresses should be deterministic as long as the deployment account and the sequence of operations is deterministic.

tynes commented 1 year ago

Why would you need a create3 design? Addresses should be deterministic as long as the deployment account and the sequence of operations is deterministic.

I would prefer to not couple the deployment account and sequence of operations to the correctness of tests. There are some cases in which we would need to change the sequence of operations as we make changes to the smart contracts

This PR fixes the issue where the artifacts are not being served: https://github.com/ethereum-optimism/optimism/pull/7672

Arachnid commented 1 year ago

Thanks!

tynes commented 1 year ago

Thanks!

Perhaps a function in the sdk that automates creating a client for the devnet would be the best abstraction for you. Open to any feedback on what makes it easy to use! Longer term we will be deprecating the sdk because it is a bit heavy to be used on frontends in favor of smaller utilities like https://github.com/base-org/op-viem

tynes commented 1 year ago

Longer term the idea is that you should need a minimal amount of addresses to interact with the system, the SystemConfig has getters for the other L1 contracts but that release has not been rolled out to mainnet yet

Arachnid commented 1 year ago

I want to use the devnet for end-to-end testing, so anything that facilitates that would be ideal. Perhaps a devnet provider in the SDK, that knows how to look up the addresses via the artifact server?