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

Polish deploy-multi-chain script #744

Merged

Conversation

smol-ninja
Copy link
Member

@smol-ninja smol-ninja commented Dec 13, 2023

Closes #743

@smol-ninja
Copy link
Member Author

smol-ninja commented Dec 13, 2023

@andreivladbrg let me know if looks good.

I had to change MAINNET_RPC_URL with RPC_URL_MAINNET in foundry.toml because fork test was failing as we use RPC_URL_MAINNET in github secret.

@smol-ninja smol-ninja force-pushed the chore/polish-deployment-script branch from a0b9658 to f512de7 Compare December 13, 2023 22:36
@smol-ninja smol-ninja force-pushed the chore/polish-deployment-script branch from f512de7 to 7706379 Compare December 13, 2023 22:47
@andreivladbrg
Copy link
Member

I had to change MAINNET_RPC_URL with RPC_URL_MAINNET in foundry.toml because fork test was failing as we use RPC_URL_MAINNET in github secret.

github secrets can be updated as well, my intention is to switch from RPC_URL_{CHAIN_NAME} to {CHAIN_NAME}_RPC_URL because it is easier to group them and to alphabetically order in .env.

@smol-ninja
Copy link
Member Author

Agree. I've changed it back 083d6d7

Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

I've added the comptrollers here with the intention of adding a deploy core 3 script that does not deploy the comptroller. Can you also verify if there have been any changes between release and staging in src/SablierV2Comptroller or src/interfaces/ISablierV2Comptroller?

shell/deploy-multi-chains.sh Outdated Show resolved Hide resolved
build: use deploy core 3 scripts
@andreivladbrg
Copy link
Member

pushed two commits, lmk if it looks good now

@smol-ninja
Copy link
Member Author

LGTM!

@andreivladbrg andreivladbrg merged commit 9991191 into build/multi-chain-script Dec 14, 2023
9 of 10 checks passed
@andreivladbrg andreivladbrg deleted the chore/polish-deployment-script branch December 14, 2023 13:05
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