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

fix: cleaning, small rephrase #3

Merged
merged 2 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ecosystem/op-deployer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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..."
Expand Down Expand Up @@ -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

Expand Down
11 changes: 8 additions & 3 deletions protocol/additional-fee-scalars.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -92,7 +95,9 @@ In order to ensure a smooth network upgrade process, these scalars are automatic
<!-- What is the resource usage of the proposed solution?
Does it consume a large amount of computational resources or time? -->

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

Expand Down
2 changes: 1 addition & 1 deletion protocol/forge-scripts-deploy-in-go.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
101 changes: 101 additions & 0 deletions protocol/superchainerc20/superc20Redesign.md
Original file line number Diff line number Diff line change
@@ -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.