Polish deploy-multi-chain script

[smol-ninja]

There is one major issue with deployment:

1. SablierV2LockupDynamic has a contract size of 25.224 kB
2. SablierV2NFTDescriptor has a contract size of 25.661 kB

Since both of them are above 24576 bytes, they fail to deploy. Failed Tx #1 and Tx #2

Changes with this PR

.evn.example

• Rename "BINANCE_RPC_URL" to "BSC_RPC_URL"
• Add "SEPOLIA_RPC_URL" and "SEPOLIA_ADMIN"

shell/deploy-multi-chains.sh

• Remove redundant "GOERLI_CHAIN_ID"
• Load variables from ".env" file
• Add ON_ALL_CHAINS bool to avoid duplication of chains in requested_chains associative array in case of accidental chain names args along with --all flag
• Replace ChainID $chain_id, Version 1.1.0 with ChainID_${chain_id}_Version_1.1.0. Seems like Parser error when passing string as parameter to script foundry-rs/foundry#6265 still exists. In the latest build, Foundry fails to parseempty spaces (" ") and comma (",")
• Added color codes for friendly output

[smol-ninja]

CI failed. It couldn't find "node_modules/forge-std/src/Script.sol" even though build successfully completed with pnpm install. Any idea?

[andreivladbrg]

There is one major issue with deployment:
SablierV2LockupDynamic has a contract size of 25.224 kB
SablierV2NFTDescriptor has a contract size of 25.661 kB

I am going to review this tomorrow, but I want to clarify this now. This issue appeared due to different optimizer_runs config in the foundry.toml file:

v2-periphery/foundry.toml

Line 13 in 6a3ca31

optimizer_runs = 10_000

https://github.com/sablier-labs/v2-core/blob/63204c9ab44e6f5efcb0c38249f229f7d4affe73/foundry.toml#L14

Forgot about this.

This means we can't run a deploy protocol script. We will have to run the deploy core script there and here on periphery we should run deploy periphery.

[smol-ninja]

I see. In that case, I have updated the script to deploy SablierV2Batch and SablierV2MerkleStream: a0e8b39

Also, I think we should include --verifier-url flag as well. The verifier url is not same for all networks: https://docs.etherscan.io/getting-started/endpoint-urls. Without that, the script may not necessarily be able to verify contracts on etherscan.

[andreivladbrg]

Thanks for the PR

Foundry fails to parseempty spaces (" ") and comma (",")

regarding this, a fix would be using "' '": 'ChainID $chain_id, Version 1.1.0'

Added color codes for friendly output

it definitely looks better now, love it.

since we are not going to use the deploy protocol anymore, we can remove:

1. DeployProtocol2 script from script/
2. comptroller variable from shell/deploy-multi-chains.sh
3. admin variable from shell/deploy-multi-chains.sh
4. admin variable from .env.example

CI failed. It couldn't find "node_modules/forge-std/src/Script.sol" even though build successfully completed with pnpm install. Any idea?

this is because the pnpm install step was not added in the ci for all jobs, fixed this: 36a58a8

think we should include --verifier-url flag as well. The verifier url is not same for all networks: https://docs.etherscan.io/getting-started/endpoint-urls.

this flag is used for custom networks. when running the script and you pass a rpc url, it automatically knows its verifier, thus you only need the api key

[andreivladbrg]

LGTM!

great, would mind if you update the core script as well with these:

Load variables from ".env" file
Add ON_ALL_CHAINS bool to avoid duplication of chains in requested_chains associative array in case of accidental chain names args along with --all flag
Replace ChainID chain_id, Version 1.1.0 with ChainID_{chain_id}_Version_1.1.0.
Added color codes for friendly output

[PaulRBerg]

Seems like foundry-rs/foundry#6265 still exists. In the latest build, Foundry fails to parseempty spaces (" ") and comma (",")

In future scenarios like this, @smol-ninja, can you please also update the external GitHub issue?

That issue should have been re-opened - and we should have told the Foundry maintainers about this.

Update: posted now:

foundry-rs/foundry#6265 (comment)

[PaulRBerg]

This means we can't run a deploy protocol script. We will have to run the deploy core script there and here on periphery we should run deploy periphery.

We can run it now since the test/utils are part of the NPM package. References:

• feat: add DeployProtocol script #91 (comment)
• Consider moving the precompiles to the "script" directory v2-core#662

Opened issue: #238

[smol-ninja]

In future scenarios like this, @smol-ninja, can you please also update the external GitHub issue?

Yeah sure. I am sorry I didn't do that but I acknowledge that I should have mentioned it there too.

[PaulRBerg]

No worries, thanks Shub.