Add mainnet deployment #119

capossele · 2024-06-17T15:15:25Z

Closes #112

nategraf · 2024-06-20T23:50:50Z

deployment-guide.md

@@ -114,15 +116,16 @@ You can deploy your contracts on a testnet such as `Sepolia` and run an end-to-e
 3. Deploy your contract by running:

    ```bash
-    forge script script/Deploy.s.sol --rpc-url https://eth-sepolia.g.alchemy.com/v2/${ALCHEMY_API_KEY:?} --broadcast
+    forge script script/Deploy.s.sol --rpc-url https://eth-sepolia.g.alchemy.com/v2/${ALCHEMY_API_KEY:?} --broadcast --sig "run(string memory configFile)" -- "script/configs/sepolia/deploy_sepolia_config.json"


Hmmm, this syntax with --sig is kind of hard to understand. Using an environment variable seems like it would be easier to understand.

nategraf · 2024-06-20T23:55:18Z

deployment-guide.md

+    forge script script/MainnetDeploy.s.sol --rpc-url https://eth-mainnet.g.alchemy.com/v2/${ALCHEMY_API_KEY:?} --broadcast --sig "run(string memory configFile)" -- "script/configs/mainnet/deploy_mainnet_config.json"
+    ```
+
+    This command uses the RISC Zero managed RiscZeroVerifierRouter contract (see https://etherscan.io/address/0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319#code.output) and should output something similar to:


Let's link to our docs where the router is explained, rather than linking to Etherscan. If they don't already know what the router is, then this won't help them much. Same comment applied to Sepolia above.

https://dev.risczero.com/api/blockchain-integration/contracts/verifier#verifier-router

Added that reference directly to the config.toml; lemme know if you think adding it also to the guide makes more sense

nategraf · 2024-06-20T23:56:35Z

deployment-guide.md

+    export BONSAI_API_KEY="YOUR_API_KEY" # see form linked in the previous section
+    export BONSAI_API_URL="BONSAI_API_URL" # provided with your api key
+    export ALCHEMY_API_KEY="YOUR_ALCHEMY_API_KEY" # the API_KEY provided with an alchemy account
+    export ETH_WALLET_PRIVATE_KEY="YOUR_WALLET_PRIVATE_KEY" # the private hex-encoded key of your Sepolia testnet wallet


There is a typo here in that this should refer to their Mainnet wallet.

Suggested change

export ETH_WALLET_PRIVATE_KEY="YOUR_WALLET_PRIVATE_KEY" # the private hex-encoded key of your Sepolia testnet wallet

export ETH_WALLET_PRIVATE_KEY="YOUR_WALLET_PRIVATE_KEY" # the private hex-encoded key of your Mainnet wallet

This also raises a bigger issue, in that we don't out-of-the-box support any kind of secure wallet as we would expect them to use on mainnet. To an extent, this is alright since we can't support ever wallet type and we expect our developers to have some idea of how to integrate Forge with their chosen platform. But we should certainly do a bit more work to support this.

Totally agree. Do you think we should address that on this PR as well?

I added some implementation for this in #122

nategraf · 2024-06-20T23:59:55Z

script/Deploy.s.sol

@@ -44,4 +48,26 @@ contract EvenNumberDeploy is Script {

        vm.stopBroadcast();
    }
+
+    function run(string memory deployConfigPath) external {


It's confusing that these two function have the same name. Additionally, taking an argument here causes use to use the weird syntax with --sig for the CLI. It would make more sense to me to have these be one function with an if in the middle that decides whether to deploy a new verifier or use an existing one.

nategraf · 2024-06-21T00:01:58Z

script/Deploy.s.sol

+        address verifier_address = stdJson.readAddress(config_data, ".risc_zero_verifier_address");
+
+        IRiscZeroVerifier verifier = IRiscZeroVerifier(verifier_address);
+        console2.log("Using RiscZeroVerifierRouter contract deployed at", address(verifier));


This log should reference IRiscZeroVerifier since this could actually be implemented by a number of different contracts, depending on the address they use. (Make sure to update the logs copied into the deployment guide)

Suggested change

console2.log("Using RiscZeroVerifierRouter contract deployed at", address(verifier));

console2.log("Using IRiscZeroVerifier contract deployed at", address(verifier));

nategraf · 2024-06-21T00:05:46Z

script/configs/mainnet/deploy_mainnet_config.json

@@ -0,0 +1,3 @@
+{
+    "risc_zero_verifier_address": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"


It seems odd to me to use snake_case in a JSON file. I usually see camelCase or kebab-case. I'd lean towards using camelCase here because Solidity uses it.

Suggested change

"risc_zero_verifier_address": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"

"riscZeroVerifierAddress": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"

* refactor to make defaults based on the chainId and support deployment with ledger * add note about the lack of support for hardware wallets * run forge fmt

nategraf

I've confirmed that the publisher step also works. Here is the contract I've deployed and set an even number on:
https://etherscan.io/address/0x0060518ffad3c2bcee309d308b9f68ec16ba7f85#readContract

capossele added 2 commits June 17, 2024 16:11

add mainnet deployment

db3b3db

fix typos

899731e

capossele requested a review from nategraf June 17, 2024 15:15

capossele added 7 commits June 17, 2024 16:21

add note on RISC Zero managed RiscZeroVerifierRouter

133964c

fmt

35e8d8a

use config files

07726f2

ignore mainnet broadcast

e6858e8

fix typo

5939cf0

fmt

a990951

fix typo

9ff9cdf

capossele self-assigned this Jun 18, 2024

nategraf reviewed Jun 21, 2024

View reviewed changes

capossele added 4 commits June 25, 2024 12:12

use profile config

c11c0be

fmt

e068361

fix typo

7c8df05

use riscZeroVerifierAddress

cd54b93

capossele requested a review from nategraf June 25, 2024 11:27

capossele added 2 commits June 25, 2024 12:33

rename config

1d64413

simplify gitignore

6656f6b

nategraf mentioned this pull request Jun 25, 2024

Suggestions for PR #119 #122

Merged

nategraf added 2 commits June 25, 2024 13:32

Suggestions for PR #119 (#122)

5fd5f3c

* refactor to make defaults based on the chainId and support deployment with ledger * add note about the lack of support for hardware wallets * run forge fmt

remove warning tag

7427324

nategraf approved these changes Jun 25, 2024

View reviewed changes

capossele marked this pull request as ready for review June 26, 2024 09:10

capossele merged commit 0eefc99 into main Jun 26, 2024
3 checks passed

capossele deleted the capossele/mainnet-deployment branch June 26, 2024 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mainnet deployment #119

Add mainnet deployment #119

capossele commented Jun 17, 2024

nategraf Jun 20, 2024

nategraf Jun 20, 2024

capossele Jun 25, 2024

nategraf Jun 20, 2024

nategraf Jun 20, 2024

capossele Jun 25, 2024

nategraf Jun 25, 2024

nategraf Jun 20, 2024

nategraf Jun 21, 2024

nategraf Jun 21, 2024

nategraf left a comment

	export ETH_WALLET_PRIVATE_KEY="YOUR_WALLET_PRIVATE_KEY" # the private hex-encoded key of your Sepolia testnet wallet
	export ETH_WALLET_PRIVATE_KEY="YOUR_WALLET_PRIVATE_KEY" # the private hex-encoded key of your Mainnet wallet

	console2.log("Using RiscZeroVerifierRouter contract deployed at", address(verifier));
	console2.log("Using IRiscZeroVerifier contract deployed at", address(verifier));

		@@ -0,0 +1,3 @@
		{
		"risc_zero_verifier_address": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"

	"risc_zero_verifier_address": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"
	"riscZeroVerifierAddress": "0x8EaB2D97Dfce405A1692a21b3ff3A172d593D319"

Add mainnet deployment #119

Add mainnet deployment #119

Conversation

capossele commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nategraf left a comment

Choose a reason for hiding this comment