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: Deploy v1.0.0 #139

Merged
merged 5 commits into from
Feb 8, 2024
Merged

feat: Deploy v1.0.0 #139

merged 5 commits into from
Feb 8, 2024

Conversation

adam-alchemy
Copy link
Contributor

Commit the deployment artifacts from v1.0.0 to /deployments.

Uses the LightAccount deployment tracker format, except on a per-network basis. This provides more context than JSON, by noting the salt and script run artifact, as well as providing an explorer link for convenience. We should revisit this tracking format if we find a better organization.

Also moves the existing alpha deployments to this new format for Polygon, Sepolia, and Base-Sepolia.

@adam-alchemy adam-alchemy requested a review from jaypaik February 7, 2024 22:38
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why there are two for sepolia

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I ran the script regularly, and it tried to submit all of the transactions at once. However, on the RPC side it didn't want to accept the 4th and 5th transactions (staking the factory and deploying SessionKeyPlugin) because the nonce was too far ahead, so it caused an error and the script stopped. I had to re-run the script after setting the env vars for the existing deployed addresses.

For all of the other networks, I used the --slow option (docs here), which delays sending the transaction until the previous one is confirmed. This prevented the too many pending transactions error that I ran into with Sepolia, and keep a single run of the script in the broadcast folder. I should document this behavior for the future.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change of tracking deployments this way.

Thoughts on following Alchemy's convention of naming for the chain-networks for the filenames and maybe making the structure flatter? For example:

deployments/arb-mainnet.md
deployments/eth-goerli.md
deployments/polygon-mainnet.md

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed the files the Alchemy naming convention, for now I kept the split between testnets and mainnets but we can drop that in the future if we'd like.

@jaypaik jaypaik merged commit 74fe1bf into develop Feb 8, 2024
3 checks passed
@jaypaik jaypaik deleted the adam/deploy-v1.0.0 branch February 8, 2024 18:20
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.

3 participants