From 6ed60ecc77d3863e6a73e6e1bc35e3b7f372fa4b Mon Sep 17 00:00:00 2001 From: ethnical Date: Mon, 23 Sep 2024 22:58:55 +0200 Subject: [PATCH 1/4] docs: add the `dgm` proxy upgrade design-doc --- security/DeputyGuardianModule-Proxy.md | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 security/DeputyGuardianModule-Proxy.md diff --git a/security/DeputyGuardianModule-Proxy.md b/security/DeputyGuardianModule-Proxy.md new file mode 100644 index 00000000..b08d9fe7 --- /dev/null +++ b/security/DeputyGuardianModule-Proxy.md @@ -0,0 +1,49 @@ +# Proxify the `DeputyGuardianModule` + +## Context + +The `DeputyGuardianModule` is an important component for the Superchain security model. +The `DeputyGuardianModule`, also known as _DGM_, is a smart contract deployed on L1 (currently [here](https://etherscan.io/address/0xc6901f65369fc59fc1b4d6d6be7a2318ff38db5b)) that has, for example, the ability to widely pause the Superchain. +The _DGM_ is owned by the _Foundation Operation Safe_ so only operation run from the _Foundation Operation Safe_ can be executed into the _DGM_. +To make sure we can widely pause the Superchain rapidly enough in case of an emergency, we need to presign the pause transaction from the signers of the _FoS_. These transactions are also known as `PSP` for PreSigned Pause transactions. +This process requires a ceremony and this is slow and tedious to put in place. + +The list of available actions that the `DeputyGuardianModule` can execute are: +| Function Name | Description | +| ---------------------- | ---------------------------------------------------------- | +| setRespectedGameType() | Change the current `gameType` used by the `OptimismPortal` | +| blacklistDisputeGame() | Blacklist a disputeGame into the `OptimismPortal` | +| unpause() | Unpause the withdrawal of the Superchain | +| pause() | Pause the withdrawal of the Superchain | +| setAnchorState()| Set a state into `AnchorStateRegistry` | + +## Problem Statement + +The existing `DeputyGuardianModule` is not _proxified_. This is inconvenient each time we want to upgrade the `DeputyGuardianModule` to add a new feature or to fix a potential bug. +As this is not a proxy this require to redeploy the `DeputyGuardianModule` on L1. And also, to update the `Guardian` contract to add the new `DeputyGuardianModule` address as authorized module. +Moreover, changing the `DeputyGuardianModule` will break the current PSPs setup as these pause transactions are presigned with the **previous** `DeputyGuardianModule`. +Thus, we need to generate new PSPs through the tedious process of a ceremony to add the new `DeputyGuardianModule` for making the PSPs valid again. +We are clearly seeing that this is not a sustainable solution for the long term. + +Additionally, we have to simulate the new PSPs and share them with other member of the Superchain. + +## Proposed Solution + +Here we propose some modification of the architecture with a proxy contract on top of the `DeputyGuardianModule`, this will allow us to upgrade the `DeputyGuardianModule` and keeping the same address. +Thus, this will not invalidate the PSPs and we will not need to regenerate new PSPs each time we upgrade the `DeputyGuardianModule`. + +The owner of the proxy contract `DeputyGuardianModuleProxy` would be the `Security Council` as the `DeputyGuardianModule` is doing action into the `Guardian` contract that is owned by the `Security Council`. +This will means that only the security council would be able to upgrade the implementation of the `DeputyGuardianModuleProxy`. + +## Alternatives Considered + +No other alternatives were considered for now. + +## Risks & Uncertainties + +### Security Model + +We don't see any particular risk with this current design for the long term as this is a common pattern to use a proxy contract for upgrading a contract. + +A potential issue can occur with the PSPs coverage. +During the upgrade the PSPs are going to be invalid so we need to make sure to presign new PSPs with the new `DeputyGuardianModuleProxy` address and simulate the PSPs accordingly. From e81e861e73113895200ae9adaebb221aa2b6c5b1 Mon Sep 17 00:00:00 2001 From: Ethnical Date: Mon, 23 Sep 2024 23:13:24 +0200 Subject: [PATCH 2/4] Update DeputyGuardianModule-Proxy.md --- security/DeputyGuardianModule-Proxy.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/security/DeputyGuardianModule-Proxy.md b/security/DeputyGuardianModule-Proxy.md index b08d9fe7..1fb45d2f 100644 --- a/security/DeputyGuardianModule-Proxy.md +++ b/security/DeputyGuardianModule-Proxy.md @@ -2,10 +2,11 @@ ## Context -The `DeputyGuardianModule` is an important component for the Superchain security model. -The `DeputyGuardianModule`, also known as _DGM_, is a smart contract deployed on L1 (currently [here](https://etherscan.io/address/0xc6901f65369fc59fc1b4d6d6be7a2318ff38db5b)) that has, for example, the ability to widely pause the Superchain. -The _DGM_ is owned by the _Foundation Operation Safe_ so only operation run from the _Foundation Operation Safe_ can be executed into the _DGM_. -To make sure we can widely pause the Superchain rapidly enough in case of an emergency, we need to presign the pause transaction from the signers of the _FoS_. These transactions are also known as `PSP` for PreSigned Pause transactions. +The `DeputyGuardianModule` is an important component for the Superchain security model. \ +The `DeputyGuardianModule`, also known as _DGM_, is a smart contract deployed on L1 (currently [here](https://etherscan.io/address/0xc6901f65369fc59fc1b4d6d6be7a2318ff38db5b)) that has, for example, the ability to widely pause the Superchain. \ +The `DGM` is owned by the _Foundation Operation Safe_ so only operation run from the _Foundation Operation Safe_ can be executed into the `DGM`. \ +To make sure we can widely pause the Superchain rapidly enough in case of an emergency, we need to presign the pause transaction from the signers of the _FoS_. \ +These transactions are also known as `PSP` for PreSigned Pause transactions. \ This process requires a ceremony and this is slow and tedious to put in place. The list of available actions that the `DeputyGuardianModule` can execute are: @@ -19,11 +20,12 @@ The list of available actions that the `DeputyGuardianModule` can execute are: ## Problem Statement -The existing `DeputyGuardianModule` is not _proxified_. This is inconvenient each time we want to upgrade the `DeputyGuardianModule` to add a new feature or to fix a potential bug. -As this is not a proxy this require to redeploy the `DeputyGuardianModule` on L1. And also, to update the `Guardian` contract to add the new `DeputyGuardianModule` address as authorized module. -Moreover, changing the `DeputyGuardianModule` will break the current PSPs setup as these pause transactions are presigned with the **previous** `DeputyGuardianModule`. -Thus, we need to generate new PSPs through the tedious process of a ceremony to add the new `DeputyGuardianModule` for making the PSPs valid again. -We are clearly seeing that this is not a sustainable solution for the long term. +The existing `DeputyGuardianModule` is not _proxified_. \ +This is inconvenient each time we want to upgrade the `DeputyGuardianModule` to add a new feature or to fix a potential bug. \ +As this is not a proxy this require to redeploy the `DeputyGuardianModule` on L1. And also, to update the `Guardian` contract to add the new `DeputyGuardianModule` address as authorized module. \ +Moreover, changing the `DeputyGuardianModule` will break the current PSPs setup as these pause transactions are presigned with the **previous** `DeputyGuardianModule`. \ +Thus, we need to generate new PSPs through the tedious process of a ceremony to add the new `DeputyGuardianModule` for making the PSPs valid again. \ +We are clearly seeing that this is not a sustainable solution for the long term. Additionally, we have to simulate the new PSPs and share them with other member of the Superchain. From 6385ef8207c2e1770824b8a3c4353be9236522eb Mon Sep 17 00:00:00 2001 From: Ethnical Date: Tue, 24 Sep 2024 09:28:11 +0200 Subject: [PATCH 3/4] Update DeputyGuardianModule-Proxy.md --- security/DeputyGuardianModule-Proxy.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/security/DeputyGuardianModule-Proxy.md b/security/DeputyGuardianModule-Proxy.md index 1fb45d2f..8fe08d5d 100644 --- a/security/DeputyGuardianModule-Proxy.md +++ b/security/DeputyGuardianModule-Proxy.md @@ -37,6 +37,8 @@ Thus, this will not invalidate the PSPs and we will not need to regenerate new P The owner of the proxy contract `DeputyGuardianModuleProxy` would be the `Security Council` as the `DeputyGuardianModule` is doing action into the `Guardian` contract that is owned by the `Security Council`. This will means that only the security council would be able to upgrade the implementation of the `DeputyGuardianModuleProxy`. +In this solution, we propose to follow the current proxy standard used by the rest of the contracts. + ## Alternatives Considered No other alternatives were considered for now. @@ -49,3 +51,5 @@ We don't see any particular risk with this current design for the long term as t A potential issue can occur with the PSPs coverage. During the upgrade the PSPs are going to be invalid so we need to make sure to presign new PSPs with the new `DeputyGuardianModuleProxy` address and simulate the PSPs accordingly. + +Now, we are using a proxy executing the same actions than before will cost a more gas. We should ensure there no previous call that can run out of gas (really unlikely). From 8a65e70ffefbd2a3026ed6a4300897d981db05ff Mon Sep 17 00:00:00 2001 From: Ethnical Date: Tue, 24 Sep 2024 12:25:04 +0200 Subject: [PATCH 4/4] Update DeputyGuardianModule-Proxy.md --- security/DeputyGuardianModule-Proxy.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/security/DeputyGuardianModule-Proxy.md b/security/DeputyGuardianModule-Proxy.md index 8fe08d5d..cfdb1264 100644 --- a/security/DeputyGuardianModule-Proxy.md +++ b/security/DeputyGuardianModule-Proxy.md @@ -2,6 +2,11 @@ ## Context +![image](https://github.com/user-attachments/assets/466c7d28-f4d1-425d-82ec-eb1326287689) + + + + The `DeputyGuardianModule` is an important component for the Superchain security model. \ The `DeputyGuardianModule`, also known as _DGM_, is a smart contract deployed on L1 (currently [here](https://etherscan.io/address/0xc6901f65369fc59fc1b4d6d6be7a2318ff38db5b)) that has, for example, the ability to widely pause the Superchain. \ The `DGM` is owned by the _Foundation Operation Safe_ so only operation run from the _Foundation Operation Safe_ can be executed into the `DGM`. \ @@ -18,6 +23,7 @@ The list of available actions that the `DeputyGuardianModule` can execute are: | pause() | Pause the withdrawal of the Superchain | | setAnchorState()| Set a state into `AnchorStateRegistry` | + ## Problem Statement The existing `DeputyGuardianModule` is not _proxified_. \ @@ -31,6 +37,8 @@ Additionally, we have to simulate the new PSPs and share them with other member ## Proposed Solution +![image](https://github.com/user-attachments/assets/099258f4-19fb-44b8-84e9-670b78a2f868) + Here we propose some modification of the architecture with a proxy contract on top of the `DeputyGuardianModule`, this will allow us to upgrade the `DeputyGuardianModule` and keeping the same address. Thus, this will not invalidate the PSPs and we will not need to regenerate new PSPs each time we upgrade the `DeputyGuardianModule`.