From 0334658268a85cc62b053daeffaf916b1ad1abc9 Mon Sep 17 00:00:00 2001 From: Yuwen Zhang Date: Tue, 1 Oct 2024 22:44:15 -0700 Subject: [PATCH 1/5] new update type and role --- protocol/additional-fee-scalars.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/protocol/additional-fee-scalars.md b/protocol/additional-fee-scalars.md index da6b5acd..4d86d3df 100644 --- a/protocol/additional-fee-scalars.md +++ b/protocol/additional-fee-scalars.md @@ -76,9 +76,12 @@ These 2 new config values can be added to the [`L1 attributes`](https://github.c The chain governor (aka [System Config Owner](https://specs.optimism.io/protocol/configurability.html#system-config-owner)) will be responsible for updating these values. We expose a new function in the `SystemConfig` contract for updating the `operatorFeeScalar` and `operatorFeeConstant`. ```solidity -function setOperatorFeeScalars(uint32 operatorFeeScalar, uint64 operatorFeeConstant) external onlyOwner; +function setOperatorFeeScalars(uint32 operatorFeeScalar, uint64 operatorFeeConstant) external onlyOperatorFeeManager; ``` +This function will emit a `ConfigUpdate` log-event, with a new `UpdateType`: `UpdateType.OPERATOR_FEE_SCALARS`. +It is also only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. + In order to ensure a smooth network upgrade process, these scalars are automatically set to zero at the start of the upgrade. ## Analysis @@ -92,7 +95,9 @@ In order to ensure a smooth network upgrade process, these scalars are automatic -There are basically no additional resources used by this proposal. +The L1Attributes deposited transactions includes one extra slot of calldata -- the two new scalars, +storage packed together. +The SystemConfig contract stores the two scalars, requiring two new storage slots. # Alternatives Considered From f7bb114f4ad26da8fe1caba6c7882919990bf8b8 Mon Sep 17 00:00:00 2001 From: Yuwen Zhang Date: Wed, 2 Oct 2024 16:41:53 -0700 Subject: [PATCH 2/5] clean diff --- ecosystem/op-deployer.md | 8 +- protocol/additional-fee-scalars.md | 2 +- protocol/forge-scripts-deploy-in-go.md | 2 +- ...r-arch.md => op-contracts-manager-arch.md} | 16 +-- protocol/superchainerc20/superc20Redesign.md | 101 ++++++++++++++++++ 5 files changed, 115 insertions(+), 14 deletions(-) rename protocol/{op-stack-manager-arch.md => op-contracts-manager-arch.md} (94%) create mode 100644 protocol/superchainerc20/superc20Redesign.md diff --git a/ecosystem/op-deployer.md b/ecosystem/op-deployer.md index ee3bf0ec..251a6303 100644 --- a/ecosystem/op-deployer.md +++ b/ecosystem/op-deployer.md @@ -62,7 +62,7 @@ directly. Instead, they'll define a _deployment intent_ which will describe the deploy. `op-deployer` will then diff the intent against the current state of the deployment, and run the pipeline to ensure that the real deployment matches the intent. -An example deployment intent is below. Deployment intents are encoded as TOML to match what the OP Stack Manager +An example deployment intent is below. Deployment intents are encoded as TOML to match what the OP Contracts Manager expects: ```toml @@ -101,8 +101,8 @@ baseFeeVaultRecipient = "0xbc4a9110dad00b4d9fb61743598848ddda6eeb03" l1FeeVaultRecipient = "0x6b0c2542fa2cadced5c7f64ef6fb9ebbce7630ff" sequencerFeeVaultRecipient = "0xb4e5b724bbc54c95d292613d956871281120ead6" -# Address of OPSM so that other addresses can be retrieved from on-chain data -opStackManagerAddress = "0x79c6c6b1844e3db7c30107f189cfb095bd2c4b5d" +# Address of OPCM so that other addresses can be retrieved from on-chain data +opContractsManagerAddress = "0x79c6c6b1844e3db7c30107f189cfb095bd2c4b5d" # Genesis data genesis = "base64://abcd..." @@ -142,7 +142,7 @@ To fix this, we will: - Remove booleans that are used to enable/disable features. This data belongs inside of the deployment intent, not the config - the config describes the chain, not the deployment process. -Some of this is already started by the OP Stack Manager work. The `op-deployer` project will complete it. +Some of this is already started by the OP Contracts Manager work. The `op-deployer` project will complete it. # Alternatives Considered diff --git a/protocol/additional-fee-scalars.md b/protocol/additional-fee-scalars.md index 4d86d3df..e2de72a5 100644 --- a/protocol/additional-fee-scalars.md +++ b/protocol/additional-fee-scalars.md @@ -55,7 +55,7 @@ of why one solution was picked over other solutions. As a rule of thumb, including code snippets (except for defining an external API) is likely too low level. --> -We propose the addition of two new rollup operator-configured scalars, collectively referred to as the **operator fee scalars**. These scalars, `operatorFeeScalar` and `operatorFeeConstant`, play a role in calculating fees as follows. +We propose the addition of two new rollup operator-configured scalars, collectively referred to as the **operator fee parameters**. These scalars, the `operatorFeeScalar` and `operatorFeeConstant`, play a role in calculating fees as follows. ## Operator Fee Formula diff --git a/protocol/forge-scripts-deploy-in-go.md b/protocol/forge-scripts-deploy-in-go.md index 1101c23f..36d79658 100644 --- a/protocol/forge-scripts-deploy-in-go.md +++ b/protocol/forge-scripts-deploy-in-go.md @@ -41,7 +41,7 @@ in a way that is configurable but not error-prone. # Proposed Solution In [deployment-chains design doc 52](https://github.com/ethereum-optimism/design-docs/pull/52) and -[OPStackManager design doc 60](https://github.com/ethereum-optimism/design-docs/pull/60) the importance +[OPContractsManager design doc 60](https://github.com/ethereum-optimism/design-docs/pull/60) the importance of Modular configs and incremental deploy steps is outlined. To utilize those modular configs and deploy steps, we need a way to either diff --git a/protocol/op-stack-manager-arch.md b/protocol/op-contracts-manager-arch.md similarity index 94% rename from protocol/op-stack-manager-arch.md rename to protocol/op-contracts-manager-arch.md index c109fbc3..5b47bf5f 100644 --- a/protocol/op-stack-manager-arch.md +++ b/protocol/op-contracts-manager-arch.md @@ -1,22 +1,22 @@ # Purpose -OP Stack Manager is a contract that will deploy the L1 contracts for standard OP Stack chains in a single transaction. +OP Contracts Manager is a contract that will deploy the L1 contracts for standard OP Stack chains in a single transaction. However, we also have use cases for deploying non-standard chains, such as deploying devnet and testnet contracts while developing interop. This document will outline an architecture that: -- Enables the OP Stack Manager to deploy these non-standard chains as an initial milestone. +- Enables the OP Contracts Manager to deploy these non-standard chains as an initial milestone. - Gets us most of the way towards supporting standard chains deployments for the subsequent milestone. # Summary -The original OP Stack Manager (OPSM) milestones were: +The original OP Contracts Manager (OPCM) milestones were: - Milestone 1: Eliminating key handover complexity. - Milestone 2: Simplifying Superchain upgrades. - Milestone 3: Supporting interop upgrades. -This design doc proposes the architecture for a Milestone 0.5 of "OPSM is used for all interop devnet & testnet L1 deployments". -Adding this milestone will allow the interop team to build testing infrastructure that wraps OPSM, unifying our development, testnet, and production testing and deployment processes. +This design doc proposes the architecture for a Milestone 0.5 of "OPCM is used for all interop devnet & testnet L1 deployments". +Adding this milestone will allow the interop team to build testing infrastructure that wraps OPCM, unifying our development, testnet, and production testing and deployment processes. # Problem Statement + Context @@ -34,10 +34,10 @@ This design doc focuses on the L1 contract configuration and deployment part of # Alternatives Considered -The primary alternative is for the interop team to not use OPSM for devnet and testnet deployments. +The primary alternative is for the interop team to not use OPCM for devnet and testnet deployments. This is not ideal for various reasons: -- Duplication of effort: The interop team would need to develop and maintain their own deployment scripts, even though new ones will already be written for OPSM. +- Duplication of effort: The interop team would need to develop and maintain their own deployment scripts, even though new ones will already be written for OPCM. - Divergence: With duplication, it's likely the interop team's deployment scripts diverge from the production deployment scripts, leading to inconsistencies and bugs. # Proposed Solution @@ -290,7 +290,7 @@ blobBaseFeeScalar = 1 The initial version of these input files will be minimal and only include the required inputs to deploy standard chains. Future versions will include more inputs to support more complex deployments, such as custom gas tokens or alt-DA (it is currently TBD whether this will be part of Milestone 0.5). -The deploy scripts and OP Stack Manager code will be written in such a way to default to standard chains and only require additional inputs for non-standard chains. +The deploy scripts and OP Contracts Manager code will be written in such a way to default to standard chains and only require additional inputs for non-standard chains. # Risks & Uncertainties diff --git a/protocol/superchainerc20/superc20Redesign.md b/protocol/superchainerc20/superc20Redesign.md new file mode 100644 index 00000000..a132a894 --- /dev/null +++ b/protocol/superchainerc20/superc20Redesign.md @@ -0,0 +1,101 @@ +## Summary + +This design doc evaluates the pros and cons of moving the interop specific functions from the [`SuperchainERC20` standard](https://github.com/ethereum-optimism/specs/blob/main/specs/interop/token-bridging.md) to a new `SuperchainERC20Bridge` predeploy. The goal of this document is to open up the discussion and ensure that the best design path is taken. + +## The current design + +The current version of the `SuperchainERC20` standard, as presented in the [specs](https://github.com/ethereum-optimism/specs/blob/main/specs/interop/token-bridging.md), extends the ERC20 to include interop specific functions. In particular, it introduces two functions: + +- `sendERC20`: burns tokens and initializes a message to the `L2ToL2CrossDomainMessenger`. +- `relayERC20`: process incoming messages from the `L2ToL2CrossDomainMessenger` and mints the corresponding amount. + +The current flow can be seen below: + +```mermaid +sequenceDiagram + participant from + participant SuperERC20_A as SuperchainERC20 (Chain A) + participant Messenger_A as L2ToL2CrossDomainMessenger (Chain A) + participant Inbox as CrossL2Inbox + participant Messenger_B as L2ToL2CrossDomainMessenger (Chain B) + participant SuperERC20_B as SuperchainERC20 (Chain B) + participant Recipient as to + + from->>SuperERC20_A: sendERC20To(to, amount, chainID) + SuperERC20_A->>SuperERC20_A: burn(from, amount) + SuperERC20_A->>Messenger_A: sendMessage(chainId, message) + SuperERC20_A-->SuperERC20_A: emit SentERC20(from, to, amount, destination) + Inbox->>Messenger_B: relayMessage() + Messenger_B->>SuperERC20_B: relayERC20(from, to, amount) + SuperERC20_B->>SuperERC20_B: mint(to, amount) + SuperERC20_B-->SuperERC20_B: emit RelayedERC20(from, to, amount, source) + +``` + +## The alternative design + +Even though the current design works well, it's not obvious that it is the best approach. An alternative design could move the interop functions from the token to a `SuperchainERC20Bridge` contract. +This would look like follows: + +- The `SuperchainERC20Bridge` will include both `sendERC20` and `relayERC20` with the same logic, but adding the token address as an input. +- The `SuperchainERC20Bridge` will require mint and burn permissions on the `SuperchainERC20`. +- The `SuperchainERC20` tokens will still be required to have the same address across chain. This is a very nice way of trusting cross-chain messages without the need for a complex registry (a source of truth to know which token to mint given a burnt token address in a source chain). Notice this is an opinionated design, as it is possible to implement the same interop functionalities with the registry. +- The `OptimismSuperchainERC20` will remain a contract separate from the `OptimismMintableERC20` and require liquidity migration using the `convert` function in the `L2StandardBridge`. + - This implies that the `OptimismSuperchainERC20` will need to provide mint and burn rights to both the `L2StandardBridge` and `SuperchainERC20Bridge`. +- The `OptimismSuperchainERC20` will be a Beacon Proxy. The `OptimismSuperchainERC20` implementation would now be similar to the `OptimismMintableERC20` implementation (give mint and burn rights to the `L2StandardBridge` and include a `remoteToken`), but with an initialize function to replace the constructor. +- It is possible to reuse the `OptimismMintableERC20Factory` and avoid introducing a new predeploy factory. + +**Code modifications:** + +- The `OptimismSuperchainERC20Factory` will of course be modified to deploy the new simpler implementations, but will still record deployments in a mapping. +- We will develop the `SuperchainERC20Bridge` logic. + +The updated flow would look like follows: + +```mermaid +sequenceDiagram + participant from + participant L2SBA as SuperchainERC20Bridge (Chain A) + participant SuperERC20_A as SuperchainERC20 (Chain A) + participant Messenger_A as L2ToL2CrossDomainMessenger (Chain A) + participant Inbox as CrossL2Inbox + participant Messenger_B as L2ToL2CrossDomainMessenger (Chain B) + participant L2SBB as SuperchainERC20Bridge (Chain B) + participant SuperERC20_B as SuperchainERC20 (Chain B) + participant Recipient as to + + from->>L2SBA: sendERC20To(tokenAddr, to, amount, chainID) + L2SBA->>SuperERC20_A: burn(from, amount) + L2SBA->>Messenger_A: sendMessage(chainId, message) + L2SBA-->L2SBA: emit SentERC20(tokenAddr, from, to, amount, destination) + Inbox->>Messenger_B: relayMessage() + Messenger_B->>L2SBB: relayERC20(tokenAddr, from, to, amount) + L2SBB->>SuperERC20_B: mint(to, amount) + L2SBB-->L2SBB: emit RelayedERC20(from, to, amount, source) + +``` + +It’s important to notice that, even though this functions get implemented in the bridge, tokens will still have agency to reimplement the bridging functions on their contract if they want so. The alternative design gives the optionality. + +## Comparison + +### Pros of using the alternative design + +- **Compatible with the current design:** adding the `sendERC20` and `relayERC20` functions to the bridge does not imply that these should no longer exist in the token. It only gives the option to token developers not to include them. +- **Separation of functionalities:** bridging lives in the bridge, token functions live in the token. This fosters clarity but also comes with improved upgrade flows. + - If bridging functions are implemented on the ERC20, updates to the bridge might require updates to every ERC20. This might be easy to do for implementations that use a BeaconProxy structure, but not in other cases. +- **Easier for integrators:** integrators must deal with a single entrypoint instead of many. This would improve their devX considerably. +- **Improved indexing and monitoring:** only monitoring events and traces coming from a single source would be necessary instead of many. Improved monitoring correlates with improved security. +- **Improved backwards compatibility:** to implement interop for an existing token it would not be necessary to upgrade the whole implementation, but only give mint and burn permissions to the `SuperchainERC20Bridge` and somehow be able to deploy to the same address on the other chains. +- **Homogeneous implementation:** most tokens on other chains do not implement bridging functionalities (except for OFTs and some others), which leads to a fragmented developer experience. + +### Cons of using the alternative design + +- **Timing:** + - **Changing code:** even though not many contracts and tests would need to be changed, it is still a considerable factor. The code has not only been developed but also extensively reviewed. + - **Communication:** a lot of teams are already aware of the current standard. Changes would need to be communicated. There is place for miscommunication or errors at this point. + +## Takeaways + +All things considered, moving the `sendERC20` and `relayERC20` seems like the best long-term solution. It's a more general approach that considers the current approach to a particular case. +There is some concern about the timing, especially regarding communication, but it is considered worth it. \ No newline at end of file From fdb728148dcfe0f91a5e69c0913f4e1a9e9a1a9c Mon Sep 17 00:00:00 2001 From: Yuwen Zhang Date: Wed, 2 Oct 2024 20:03:44 -0700 Subject: [PATCH 3/5] role typo --- protocol/additional-fee-scalars.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocol/additional-fee-scalars.md b/protocol/additional-fee-scalars.md index e2de72a5..14bb7498 100644 --- a/protocol/additional-fee-scalars.md +++ b/protocol/additional-fee-scalars.md @@ -73,14 +73,14 @@ The `operatorFee` will be directed to a new `FeeVault`, the `OperatorFeeVault`, These 2 new config values can be added to the [`L1 attributes`](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/ecotone/l1-attributes.md) transaction, with a very small diff to the proof. -The chain governor (aka [System Config Owner](https://specs.optimism.io/protocol/configurability.html#system-config-owner)) will be responsible for updating these values. We expose a new function in the `SystemConfig` contract for updating the `operatorFeeScalar` and `operatorFeeConstant`. +We expose a new function in the `SystemConfig` contract for updating the `operatorFeeScalar` and `operatorFeeConstant`. This function is only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. The `OperatorFeeManager` is adminstrated by +the chain governor. ```solidity function setOperatorFeeScalars(uint32 operatorFeeScalar, uint64 operatorFeeConstant) external onlyOperatorFeeManager; ``` This function will emit a `ConfigUpdate` log-event, with a new `UpdateType`: `UpdateType.OPERATOR_FEE_SCALARS`. -It is also only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. In order to ensure a smooth network upgrade process, these scalars are automatically set to zero at the start of the upgrade. From be2650cd5c0c594dcb9cdb8648fecca69c26ea5e Mon Sep 17 00:00:00 2001 From: Yuwen Zhang Date: Wed, 2 Oct 2024 20:08:38 -0700 Subject: [PATCH 4/5] typo --- protocol/additional-fee-scalars.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/protocol/additional-fee-scalars.md b/protocol/additional-fee-scalars.md index db4cf7f0..0368d3b7 100644 --- a/protocol/additional-fee-scalars.md +++ b/protocol/additional-fee-scalars.md @@ -73,7 +73,7 @@ The `operatorFee` will be directed to a new `FeeVault`, the `OperatorFeeVault`, These 2 new config values can be added to the [`L1 attributes`](https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/ecotone/l1-attributes.md) transaction, with a very small diff to the proof. -We expose a new function in the `SystemConfig` contract for updating the `operatorFeeScalar` and `operatorFeeConstant`. This function is only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. The `OperatorFeeManager` is adminstrated by +We expose a new function in the `SystemConfig` contract for updating the `operatorFeeScalar` and `operatorFeeConstant`. This function is only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. The `OperatorFeeManager` is administrated by the chain governor. ```solidity @@ -82,8 +82,6 @@ function setOperatorFeeScalars(uint32 operatorFeeScalar, uint64 operatorFeeConst This function will emit a `ConfigUpdate` log-event, with a new `UpdateType`: `UpdateType.OPERATOR_FEE_SCALARS`. -It is also only callable by the `OperatorFeeManager`, a new role responsible for tuning the operator fee scalars. - In order to ensure a smooth network upgrade process, these scalars are automatically set to zero at the start of the upgrade. ## Analysis From 9be11cb31e04b54a8f1ea7d836fe01c692897d1c Mon Sep 17 00:00:00 2001 From: Yuwen Zhang Date: Fri, 4 Oct 2024 11:21:50 -0700 Subject: [PATCH 5/5] reword a bit --- protocol/additional-fee-scalars.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/protocol/additional-fee-scalars.md b/protocol/additional-fee-scalars.md index 0368d3b7..779fb59d 100644 --- a/protocol/additional-fee-scalars.md +++ b/protocol/additional-fee-scalars.md @@ -2,7 +2,7 @@ #### Metadata -Authors: @puma314. +Authors: @puma314, @yuwen01. Created: September 15, 2024. # Purpose @@ -28,11 +28,19 @@ The purpose of this design doc is to propose a simple addition to the existing f While the length will likely be proportional to the length of the full document, the summary should be as succinct as possible. --> -We propose the addition of 2 new rollup operator configured scalars (that can be updated via the `SystemConfig`) named `operatorFeeScalar` and `operatorFeeConstant` that factor in the fee calculation as follows: +We propose the addition of 2 new rollup operator configured scalars, collectively named the Operator Fee Parameters. These scalars are named +`operatorFeeScalar` and `operatorFeeConstant`, and they factor in the fee calculation as follows: ``` -totalFee = operatorFeeConstant + operatorFeeScalar * gasUsed / 1e6 + gasUsed * (baseFee + priorityFee) + l1Fee +operatorFee = operatorFeeConstant + operatorFeeScalar * gasUsed / 1e6 + +totalFee = operatorFee + gasUsed * (baseFee + priorityFee) + l1Fee ``` +These scalars will be updated via the `SystemConfig` L1 contract. A new role, the `OperatorFeeManager`, is added +to administrate the `operatorFeeScalar` and `operatorFeeConstant`. + +A new fee vault, the `OperatorFeeVault`, is added to store the operator fee. + # Problem Statement + Context The L1Attributes deposited transactions includes one extra slot of calldata -- the two new scalars, storage packed together. -The SystemConfig contract stores the two scalars, requiring two new storage slots. # Alternatives Considered