simplify interaction with multisig wallets in deployment scripts #1859

alysiahuggins · 2024-08-13T23:43:21Z

Closes #1838

This PR:

provides scripts to deploy the FeeContract, LightClient Contract and PlonkVerifier library without OpenZeppelin Defender and without a multisig wallet creating the transaction (thus via an EOA)
provides scripts to upgrade the FeeContract and LightClient using an EOA to deploy the new implementation contracts and the Safe SDK to create a transaction on the Safe interface so that a Safe multisig wallet can be used to upgrade the proxy contracts as its the owner of these proxies
consolidates scripts, improve comments across scripts and uses a stricter naming convention for scripts & their containing functions

Key places to review:

updated README
updated scripts
the use of vm.ffi to interact with the Safe wallet (this was used since that interaction flow was already set up for a previous admin modification)

How to test this PR:

Follow the instructions in the README to deploy the the FeeContract and LightClient proxies
Follow the instructions in the README to upgrade the FeeContract and LightClient

Things tested

deploying the contracts mentioned above using the forge script command
upgrading the respective proxies using the forge script command

Things not tested

interacting with functions on the PlonkVerifier library once deployed

…needing to deploy with a multisig wallet - but the multisig wallet is set as the admin

…the readme and example env

… PV without openzeppelin defender and the multisig wallet

…ets-in-deployment-scripts

…contract as we do in dev phases

…ets-in-deployment-scripts

Ayiga · 2024-09-05T14:41:16Z

contracts/script/FeeContract.s.sol

+/// @notice Upgrades the fee contract first by deploying the new implementation
+/// and then executing the upgrade via the Safe Multisig wallet using the SAFE SDK.
+contract UpgradeFeeContractScript is Script {
+    string internal originalContractName = "FeeContract.sol";


If we want to use this upgrade more than once, should we also make it so that the originalContractName can be derived from an environment variable?

thanks @Ayiga , i did that in this update 4ac6ca7 however, the actual contract still needs to be imported into the script as we see here

espresso-sequencer/contracts/script/FeeContract.s.sol

Line 67 in 4ac6ca7

FC implementationContract = new FC();

so there will still need to be changes to the script when the time comes

…ets-in-deployment-scripts

…ariable for the original contract

zacshowa

one comment / nit for my own understanding 😄

zacshowa · 2024-09-12T02:03:31Z

contracts/broadcast/FeeContract.s.sol/11155111/run-1723219248.json

@@ -0,0 +1,59 @@
+{


For my own understanding, Are these run files used by something in the repo like CI or tests?

If we do need to be committing them to the repo, what are we using them for? If not should we add the /contracts/broadcast directory to the .gitignore and remove ones that currently reside in the repo?

The idea is to only commit the files for public deployments to the repo. It's to make life easy for anyone who wants to interact with the deployed contracts.

…ets-in-deployment-scripts

Sneh1999 · 2024-09-13T19:58:42Z

contracts/script/multisigTransactionProposals/safeSDK/upgradeProxy.ts

+     * change from SEPOLIA_RPC_URL to production URL when deploying to production
+     */
+    // Initialize web3 provider using the RPC URL from environment variables
+    const web3Provider = new ethers.JsonRpcProvider(getEnvVar("SEPOLIA_RPC_URL"));


nit: could just name it RPC_URL then we can just change the RPC_URL in .env and no code changes needed

i like, will do

Nice, added a few extra comments related to this

…ets-in-deployment-scripts

.env

justfile

Sneh1999

LGTM!

.env

alysiahuggins added 6 commits August 8, 2024 17:37

instructions for deploying Fee contract without defender and without …

a82aa8b

…needing to deploy with a multisig wallet - but the multisig wallet is set as the admin

added script to deploy the LC contract without defender and upgraded …

eeb5487

…the readme and example env

ugprading the fee contract without opzd but with the safe sdk

eb3b549

include initData as part of safe sdk proxy upgrades

286c14d

upgrade LC contract without OPZD and support for initData

5edd532

consolidated LC scripts and created script contract for deploying the…

1a77382

… PV without openzeppelin defender and the multisig wallet

alysiahuggins requested review from nomaxg, philippecamacho and ImJeremyHe as code owners August 13, 2024 23:43

alysiahuggins linked an issue Aug 13, 2024 that may be closed by this pull request

simplify interaction with multisig wallets in deployment scripts #1838

Closed

alysiahuggins requested review from sveitser, jbearer, tbro and imabdulbasit as code owners August 13, 2024 23:43

alysiahuggins added 12 commits August 16, 2024 13:25

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

443f373

…ets-in-deployment-scripts

move scripts related to tests in the test folder

a084678

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

5e6d32b

…ets-in-deployment-scripts

remove unused script and added the ability to upgrade to the same LC …

35293e0

…contract as we do in dev phases

switched all scripts using private keys to mnenomics

54b7aa1

consistent naming convention for deploy & upgrade scripts

80db3c0

consolidated scripts and added more comprehensive comments to scripts

439b82d

restructuring and updating the deployment & upgrades readme

77c5525

do not commit sepolia deployments

9c7606a

do not commit sepolia deployments

c1b7b6a

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

75842cd

…ets-in-deployment-scripts

move all test scripts into the test/script folder

10874e9

alysiahuggins changed the title ~~DRAFT PR | simplify interaction with multisig wallets in deployment scripts~~ simplify interaction with multisig wallets in deployment scripts Aug 22, 2024

consolidating test scripts

d705835

Ayiga reviewed Sep 5, 2024

View reviewed changes

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

3085a65

…ets-in-deployment-scripts

updating scripts and readme after merge from main and adding an env v…

4ac6ca7

…ariable for the original contract

zacshowa reviewed Sep 12, 2024

View reviewed changes

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

017f487

…ets-in-deployment-scripts

Sneh1999 approved these changes Sep 13, 2024

View reviewed changes

alysiahuggins added 2 commits September 15, 2024 20:11

add rpc url and use instead of sepolia rpc url

391ce1c

Merge branch 'main' into 1838-simplify-interaction-with-multisig-wall…

d5d8c3c

…ets-in-deployment-scripts

Sneh1999 reviewed Sep 16, 2024

View reviewed changes

.env Show resolved Hide resolved

justfile Show resolved Hide resolved

update safe sdk proposal tests and switch env variable name

c61a520

Sneh1999 self-requested a review September 16, 2024 19:51

Sneh1999 approved these changes Sep 16, 2024

View reviewed changes

.env Show resolved Hide resolved

alysiahuggins merged commit f3ce319 into main Sep 16, 2024
16 checks passed

alysiahuggins deleted the 1838-simplify-interaction-with-multisig-wallets-in-deployment-scripts branch September 16, 2024 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simplify interaction with multisig wallets in deployment scripts #1859

simplify interaction with multisig wallets in deployment scripts #1859

alysiahuggins commented Aug 13, 2024 •

edited

Loading

Ayiga Sep 5, 2024

alysiahuggins Sep 9, 2024

zacshowa left a comment

zacshowa Sep 12, 2024

sveitser Sep 12, 2024

Sneh1999 Sep 13, 2024

alysiahuggins Sep 13, 2024

alysiahuggins Sep 16, 2024

Sneh1999 Sep 16, 2024

Sneh1999 left a comment

simplify interaction with multisig wallets in deployment scripts #1859

simplify interaction with multisig wallets in deployment scripts #1859

Conversation

alysiahuggins commented Aug 13, 2024 • edited Loading

This PR:

Key places to review:

How to test this PR:

Things tested

Things not tested

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zacshowa left a 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

Sneh1999 left a comment

Choose a reason for hiding this comment

alysiahuggins commented Aug 13, 2024 •

edited

Loading