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: deployment scripts #8

Merged
merged 13 commits into from
Jun 27, 2024
Merged

feat: deployment scripts #8

merged 13 commits into from
Jun 27, 2024

Conversation

mattstam
Copy link
Contributor

@mattstam mattstam commented Jun 20, 2024

fully forge script deployment process, and v1.0.8-testnet deployment

uses template changes found in succinctlabs/sp1#963


/// @notice When used, runs the script on the chains specified in the `CHAIN_IDS` env variable.
/// Must have a `RPC_${CHAIN_ID}` env variable set for each chain.
modifier multichain() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattstam mattstam marked this pull request as ready for review June 20, 2024 20:09
README.md Outdated
@@ -14,6 +15,7 @@ forge install succinctlabs/sp1-contracts
```

To install a specific version:

```bash
forge install succinctlabs/sp1-contracts@<version>
Copy link
Member

Choose a reason for hiding this comment

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

this is no longer correct i tihnk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's still relevant? If someone wants these contracts as a dependency, they can specify a specific tag/commit this way

Copy link
Member

Choose a reason for hiding this comment

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

but shouldn't you only need latest?

Comment on lines 5 to 43
RPC_1=
RPC_11155111=
RPC_17000=
RPC_100=
RPC_137=
RPC_42161=
RPC_421614=
RPC_8453=
RPC_84532=
RPC_534352=
RPC_534351=

# Etherscan API keys for each chain ID
ETHERSCAN_API_KEY_1=
ETHERSCAN_API_KEY_11155111=
ETHERSCAN_API_KEY_17000=
ETHERSCAN_API_KEY_100=
ETHERSCAN_API_KEY_137=
ETHERSCAN_API_KEY_42161=
ETHERSCAN_API_KEY_421614=
ETHERSCAN_API_KEY_8453=
ETHERSCAN_API_KEY_84532=
ETHERSCAN_API_KEY_534352=
ETHERSCAN_API_KEY_534351=

# Etherscan API URLs for each chain ID
ETHERSCAN_API_URL_1=https://api.etherscan.io/api
ETHERSCAN_API_URL_5=https://api-goerli.etherscan.io/api
ETHERSCAN_API_URL_17000=https://api-holesky.etherscan.io/api
ETHERSCAN_API_URL_11155111=https://api-sepolia.etherscan.io/api
ETHERSCAN_API_URL_100=https://api.gnosisscan.io/api
ETHERSCAN_API_URL_137=https://api.polygonscan.com/api
ETHERSCAN_API_URL_420=https://api-optimistic.etherscan.io/api
ETHERSCAN_API_URL_42161=https://api.arbiscan.io/api
ETHERSCAN_API_URL_421614=https://api-sepolia.arbiscan.io/api
ETHERSCAN_API_URL_8453=https://api.basescan.org/api
ETHERSCAN_API_URL_84532=https://api-sepolia.basescan.org/api
ETHERSCAN_API_URL_534352=https://api.scrollscan.com/api
ETHERSCAN_API_URL_534351=https://api-sepolia.scrollscan.com/api
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should just manage this all in foundry and have CHAINS be a list of chain names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice thing about how its setup in this PR is that we can just put in our existing .env from other repo and it works out of the box

ids are easier to standardize around as chain names are conflicting in terms of like exact name ("arb-testnet" vs "arb-sepolia" vs "arb_sepolia") while just leads to work devx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to chain names

Comment on lines 20 to 22
// Add the verifier to the gateway
SP1VerifierGateway gateway = SP1VerifierGateway(SP1_VERIFIER_GATEWAY);
gateway.addRoute(verifier);
Copy link
Member

Choose a reason for hiding this comment

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

in the future we may have multiple routes here right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wdym? Each verifier version corresponds to one route

function run() external multichain(KEY) broadcaster {
// Read config
bytes32 CREATE2_SALT = readBytes32("CREATE2_SALT");
address OWNER = readAddress("OWNER");
Copy link
Member

Choose a reason for hiding this comment

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

these envs are not in example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@mattstam mattstam requested a review from ctian1 June 26, 2024 20:25
# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

[rpc_endpoints]
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should just not need these if CHAINS is user specified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think its nice to have some examples of how it all connects together. Plus having them all here makes it so if anyone else needs to deploy the contracts, they can git clone and go without having to configure stuff

@mattstam mattstam merged commit 608814d into main Jun 27, 2024
1 check passed
@mattstam mattstam deleted the mattstam/deployments branch June 27, 2024 20:55
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