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

feat: Prefix all addresses with l1 or l2 depending on chain #3

Open
roninjin10 opened this issue Sep 5, 2023 · 5 comments
Open

feat: Prefix all addresses with l1 or l2 depending on chain #3

roninjin10 opened this issue Sep 5, 2023 · 5 comments

Comments

@roninjin10
Copy link
Contributor

  • To make it clear what layer a contract is on prefix all of them with l1 or l2.
  • Additionally consider adding predeployed l2 addresses even though they are always the same

image

@roninjin10
Copy link
Contributor Author

An alternative is to call this folder L1Addresses instead of addresses

@protolambda
Copy link
Contributor

Currently they are all L1, and they will likely always be L1, as the L2 protocol contracts all have their own standard predeploy address (0x42000...00\d\d\d), the same per chain, and that thus does not have to be hosted in this repository.

@roninjin10
Copy link
Contributor Author

@protolambda it's pretty inconvenient to not have them here though. For example, I want viem and ethers to use this as the source of truth and instead they are forced to use a different package. one package for l1 addresses and a different package for l2 addresses

@roninjin10
Copy link
Contributor Author

Same thing for folks just trying to find contract addresses. Many folks like myself will land here looking for addresses and be confused when they can't find the l2 ones. Is there any good reason to not include them?

@Sabnock01
Copy link
Contributor

Sabnock01 commented Oct 7, 2023

A reasonable compromise here in my opinion would be to change the folder to be L1Addresses suggested earlier without renaming anything in all the individual *.json files but also adding a L2Addresses/predeploys.json file also for reasons stated earlier and outlined here here as well.

See #21

Fine with closing if we discover an alternative solution we prefer.

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 a pull request may close this issue.

3 participants