From 59b0102829517d6eab7d7b7a33d5b3e469e26037 Mon Sep 17 00:00:00 2001 From: clabby Date: Tue, 22 Nov 2022 17:11:09 -0500 Subject: [PATCH 1/7] Update withdrawal specs --- specs/withdrawals.md | 80 +++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/specs/withdrawals.md b/specs/withdrawals.md index dcaa9065e7934..b1ca45b0eea21 100644 --- a/specs/withdrawals.md +++ b/specs/withdrawals.md @@ -1,24 +1,24 @@ # Withdrawals + [g-deposits]: glossary.md#deposits [g-deposited]: glossary.md#deposited-transaction [deposit-tx-type]: glossary.md#deposited-transaction-type - [g-withdrawal]: glossary.md#withdrawal [g-mpt]: glossary.md#merkle-patricia-trie [g-relayer]: glossary.md#withdrawals [g-execution-engine]: glossary.md#execution-engine [Withdrawals][g-withdrawal] are cross domain transactions which are initiated on L2, and finalized by a transaction -executed on L1. Notably, withdrawals may be used by and L2 account to call an L1 contract, or to transfer ETH from +executed on L1. Notably, withdrawals may be used by an L2 account to call an L1 contract, or to transfer ETH from an L2 account to an L1 account. -**Vocabulary note**: *withdrawal* can refer to the transaction at various stages of the process, but we introduce +**Vocabulary note**: _withdrawal_ can refer to the transaction at various stages of the process, but we introduce more specific terms to differentiate: -- A *withdrawal initiating transaction* refers specifically to a transaction on L2 sent to the Withdrawals predeploy. -- A *withdrawal finalizing transaction* refers specifically to an L1 transaction which finalizes and relays the +- A _withdrawal initiating transaction_ refers specifically to a transaction on L2 sent to the Withdrawals predeploy. +- A _withdrawal finalizing transaction_ refers specifically to an L1 transaction which finalizes and relays the withdrawal. Withdrawals are initiated on L2 via a call to the Message Passer predeploy contract, which records the important @@ -31,6 +31,7 @@ finalization. + **Table of Contents** - [Withdrawal Flow](#withdrawal-flow) @@ -55,17 +56,25 @@ We first describe the end to end flow of initiating and finalizing a withdrawal: ### On L2 An L2 account sends a withdrawal message (and possibly also ETH) to the `L2ToL1MessagePasser` predeploy contract. - This is a very simple contract that stores the a hash of the withdrawal data. +This is a very simple contract that stores the a hash of the withdrawal data. ### On L1 1. A [relayer][g-relayer] submits the required inputs to the `OptimismPortal` contract. The relayer need not be the same entity which initiated the withdrawal on L2. - These inputs include the withdrawal transaction data, inclusion proofs, and a timestamp. The timestamp + These inputs include the withdrawal transaction data, inclusion proofs, and a block number. The block number must be one for which an L2 output root exists, which commits to the withdrawal as registered on L2. -2. The `OptimismPortal` contract retrieves the output root for the given timestamp from the `OutputOracle`'s - `l2Outputs()` function, and performs the remainder of the verification process internally. -3. If verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from +2. The `OptimismPortal` contract retrieves the output root for the given block number from the `L2OutputOracle`'s + `getNextProposedOutput()` function, and performs the remainder of the verification process internally. +3. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from + from being re-proven. Note that the withdrawal can be proven more than once if the corresponding output root changes. +4. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants to challenge + its integrity. +5. Once the challenge period has passed, a relayer submits the withdrawal transaction once again to the `OptimismPortal` contract. + Again, the relayer need not be the same entity which initiated the withdrawal on L2. +6. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has both been proven + and passed the challenge period. +7. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from being replayed. ## The L2ToL1MessagePasser Contract @@ -113,7 +122,7 @@ of withdrawals, which do not modify the sender's address. The difference is that - on L2, the deposit sender's address is returned by the `CALLER` opcode, meaning a contract cannot easily tell if the call originated on L1 or L2, whereas -- on L1, the withdrawal sender's address is accessed by calling the `l2Sender`() function on the `OptimismPortal` +- on L1, the withdrawal sender's address is accessed by calling the `l2Sender()` function on the `OptimismPortal` contract. Calling `l2Sender()` removes any ambiguity about which domain the call originated from. Still, developers will need to @@ -132,23 +141,22 @@ interface OptimismPortal { function l2Sender() returns(address) external; + function proveWithdrawalTransaction( + Types.WithdrawalTransaction memory _tx, + uint256 _l2BlockNumber, + Types.OutputRootProof calldata _outputRootProof, + bytes[] calldata _withdrawalProof + ); + function finalizeWithdrawalTransaction( - uint256 _nonce, - address _sender, - address _target, - uint256 _value, - uint256 _gasLimit, - bytes calldata _data, - uint256 _timestamp, - WithdrawalVerifier.OutputRootProof calldata _outputRootProof, - bytes calldata _withdrawalProof + Types.WithdrawalTransaction memory _tx ) } ``` ## Withdrawal Verification and Finalization -The following inputs are required to verify and finalize a withdrawal: +The following inputs are required to prove and finalize a withdrawal: - Withdrawal transaction data: - `nonce`: Nonce for the provided message. @@ -158,15 +166,15 @@ The following inputs are required to verify and finalize a withdrawal: - `data`: Data to send to the target. - `gasLimit`: Gas to be forwarded to the target. - Proof and verification data: - - `timestamp`: The L2 timestamp corresponding with the output root. + - `l2BlockNumber`: The L2 block number corresponding with the output root. - `outputRootProof`: Four `bytes32` values which are used to derive the output root. - `withdrawalProof`: An inclusion proof for the given withdrawal in the L2ToL1MessagePasser contract. These inputs must satisfy the following conditions: -1. The `timestamp` is at least `FINALIZATION_PERIOD` seconds old. -1. `OutputOracle.l2Outputs(timestamp)` returns a non-zero value `l2Output`. -1. The keccak256 hash of the `outputRootProof` values is equal to the `l2Output`. +1. The `l2BlockNumber` must be the block number that corresponds to the `OutputProposal` being proven. +1. `L2OutputOracle.getNextProposedOutput(l2BlockNumber)` returns a non-zero `OutputProposal`. +1. The keccak256 hash of the `outputRootProof` values is equal to the `outputRoot`. 1. The `withdrawalProof` is a valid inclusion proof demonstrating that a hash of the Withdrawal transaction data is contained in the storage of the L2ToL1MessagePasser contract on L2. @@ -175,19 +183,21 @@ These inputs must satisfy the following conditions: ### Key Properties of Withdrawal Verification 1. It should not be possible 'double spend' a withdrawal, ie. to relay a withdrawal on L1 which does not - correspond to a message initiated on L2. For reference, see [this writeup][polygon-dbl-spend] of a vulnerability - of this type found on Polygon. + correspond to a message initiated on L2. For reference, see [this writeup][polygon-dbl-spend] of a vulnerability + of this type found on Polygon. - [polygon-dbl-spend]: https://gerhard-wagner.medium.com/double-spending-bug-in-polygons-plasma-bridge-2e0954ccadf1 + [polygon-dbl-spend]: https://gerhard-wagner.medium.com/double-spending-bug-in-polygons-plasma-bridge-2e0954ccadf1 1. For each withdrawal initiated on L2 (ie. with a unique `nonce`), the following properties must hold: - 1. It should only be possible to finalize the withdrawal once. - 1. It should not be possible to relay the message with any of its fields modified, ie. - 1. Modifying the `sender` field would enable a 'spoofing' attack. - 1. Modifying the `target`, `message`, or `value` fields would enable an attacker to dangerously change the - intended outcome of the withdrawal. - 1. Modifying the `gasLimit` could make the cost of relaying too high, or allow the relayer to cause execution - to fail (out of gas) in the `target`. + 1. It should only be possible to prove the withdrawal once, unless the outputRoot for the withdrawal + has changed. + 1. It should only be possible to finalize the withdrawal once. + 1. It should not be possible to relay the message with any of its fields modified, ie. + 1. Modifying the `sender` field would enable a 'spoofing' attack. + 1. Modifying the `target`, `message`, or `value` fields would enable an attacker to dangerously change the + intended outcome of the withdrawal. + 1. Modifying the `gasLimit` could make the cost of relaying too high, or allow the relayer to cause execution + to fail (out of gas) in the `target`. ### Handling Successfully Verified Messages That Fail When Relayed From 3ce0ddee652fc2ff251614d64d769ea87a2b4c82 Mon Sep 17 00:00:00 2001 From: clabby Date: Tue, 22 Nov 2022 17:19:38 -0500 Subject: [PATCH 2/7] lint toc --- specs/withdrawals.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/specs/withdrawals.md b/specs/withdrawals.md index b1ca45b0eea21..5a2a14e3f5ad4 100644 --- a/specs/withdrawals.md +++ b/specs/withdrawals.md @@ -31,7 +31,6 @@ finalization. - **Table of Contents** - [Withdrawal Flow](#withdrawal-flow) @@ -66,16 +65,17 @@ This is a very simple contract that stores the a hash of the withdrawal data. must be one for which an L2 output root exists, which commits to the withdrawal as registered on L2. 2. The `OptimismPortal` contract retrieves the output root for the given block number from the `L2OutputOracle`'s `getNextProposedOutput()` function, and performs the remainder of the verification process internally. -3. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from - from being re-proven. Note that the withdrawal can be proven more than once if the corresponding output root changes. -4. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants to challenge - its integrity. -5. Once the challenge period has passed, a relayer submits the withdrawal transaction once again to the `OptimismPortal` contract. - Again, the relayer need not be the same entity which initiated the withdrawal on L2. -6. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has both been proven - and passed the challenge period. -7. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it - from being replayed. +3. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to + prevent it from being re-proven. Note that the withdrawal can be proven more than once if the corresponding + output root changes. +4. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants + to challenge its integrity. +5. Once the challenge period has passed, a relayer submits the withdrawal transaction once again to the + `OptimismPortal` contract. Again, the relayer need not be the same entity which initiated the withdrawal on L2. +6. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has + both been proven and passed the challenge period. +7. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to + prevent it from being replayed. ## The L2ToL1MessagePasser Contract @@ -146,11 +146,11 @@ interface OptimismPortal { uint256 _l2BlockNumber, Types.OutputRootProof calldata _outputRootProof, bytes[] calldata _withdrawalProof - ); + ) external; function finalizeWithdrawalTransaction( Types.WithdrawalTransaction memory _tx - ) + ) external; } ``` @@ -166,7 +166,7 @@ The following inputs are required to prove and finalize a withdrawal: - `data`: Data to send to the target. - `gasLimit`: Gas to be forwarded to the target. - Proof and verification data: - - `l2BlockNumber`: The L2 block number corresponding with the output root. + - `l2BlockNumber`: The L2 block number that corresponds to the output root. - `outputRootProof`: Four `bytes32` values which are used to derive the output root. - `withdrawalProof`: An inclusion proof for the given withdrawal in the L2ToL1MessagePasser contract. From b35164df47729114a883e1fb9f4d29f3d93c40da Mon Sep 17 00:00:00 2001 From: clabby Date: Wed, 23 Nov 2022 13:25:32 -0500 Subject: [PATCH 3/7] Change signature of L2OutputOracle function --- specs/withdrawals.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/withdrawals.md b/specs/withdrawals.md index 5a2a14e3f5ad4..038f1a977c4f0 100644 --- a/specs/withdrawals.md +++ b/specs/withdrawals.md @@ -64,7 +64,7 @@ This is a very simple contract that stores the a hash of the withdrawal data. These inputs include the withdrawal transaction data, inclusion proofs, and a block number. The block number must be one for which an L2 output root exists, which commits to the withdrawal as registered on L2. 2. The `OptimismPortal` contract retrieves the output root for the given block number from the `L2OutputOracle`'s - `getNextProposedOutput()` function, and performs the remainder of the verification process internally. + `getL2OutputAfter()` function, and performs the remainder of the verification process internally. 3. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from being re-proven. Note that the withdrawal can be proven more than once if the corresponding output root changes. @@ -173,7 +173,7 @@ The following inputs are required to prove and finalize a withdrawal: These inputs must satisfy the following conditions: 1. The `l2BlockNumber` must be the block number that corresponds to the `OutputProposal` being proven. -1. `L2OutputOracle.getNextProposedOutput(l2BlockNumber)` returns a non-zero `OutputProposal`. +1. `L2OutputOracle.getL2OutputAfter(l2BlockNumber)` returns a non-zero `OutputProposal`. 1. The keccak256 hash of the `outputRootProof` values is equal to the `outputRoot`. 1. The `withdrawalProof` is a valid inclusion proof demonstrating that a hash of the Withdrawal transaction data is contained in the storage of the L2ToL1MessagePasser contract on L2. From 1ed0167b9e79b72653203508c18212c604f519ec Mon Sep 17 00:00:00 2001 From: clabby Date: Thu, 24 Nov 2022 13:25:29 -0500 Subject: [PATCH 4/7] Fix list numbering --- specs/withdrawals.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/withdrawals.md b/specs/withdrawals.md index 038f1a977c4f0..e20280d402a89 100644 --- a/specs/withdrawals.md +++ b/specs/withdrawals.md @@ -63,18 +63,18 @@ This is a very simple contract that stores the a hash of the withdrawal data. not be the same entity which initiated the withdrawal on L2. These inputs include the withdrawal transaction data, inclusion proofs, and a block number. The block number must be one for which an L2 output root exists, which commits to the withdrawal as registered on L2. -2. The `OptimismPortal` contract retrieves the output root for the given block number from the `L2OutputOracle`'s +1. The `OptimismPortal` contract retrieves the output root for the given block number from the `L2OutputOracle`'s `getL2OutputAfter()` function, and performs the remainder of the verification process internally. -3. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to +1. If proof verification fails, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from being re-proven. Note that the withdrawal can be proven more than once if the corresponding output root changes. -4. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants +1. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants to challenge its integrity. -5. Once the challenge period has passed, a relayer submits the withdrawal transaction once again to the +1. Once the challenge period has passed, a relayer submits the withdrawal transaction once again to the `OptimismPortal` contract. Again, the relayer need not be the same entity which initiated the withdrawal on L2. -6. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has +1. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has both been proven and passed the challenge period. -7. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to +1. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to prevent it from being replayed. ## The L2ToL1MessagePasser Contract From 2617c9ae4ad344b09944cbd9f5d6e5bc0e75c7ad Mon Sep 17 00:00:00 2001 From: clabby Date: Wed, 30 Nov 2022 13:12:36 -0500 Subject: [PATCH 5/7] Add link to `Types.WithdrawalTransaction` & `Types.OutputRootProof` --- specs/withdrawals.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs/withdrawals.md b/specs/withdrawals.md index e20280d402a89..5d911f9772153 100644 --- a/specs/withdrawals.md +++ b/specs/withdrawals.md @@ -134,6 +134,9 @@ The Optimism Portal serves as both the entry and exit point to the Optimism L2. the [DepositFeed](./deposits.md#deposit-contract) contract, and in addition provides the following interface for withdrawals: +- [WithdrawalTransaction type](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/contracts/libraries/Types.sol#L46-L56) +- [OutputRootProof type](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/contracts/libraries/Types.sol#L20-L29) + ```js interface OptimismPortal { From c69870c7fec537148436596164b188c78bdca2cb Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Wed, 30 Nov 2022 18:40:56 -0800 Subject: [PATCH 6/7] op-chain-ops: add no check to eth migration --- op-chain-ops/ether/migrate.go | 56 ++++++++++++++++++++-------- op-chain-ops/genesis/db_migration.go | 2 +- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/op-chain-ops/ether/migrate.go b/op-chain-ops/ether/migrate.go index 8785b5320b51c..abbbd66750a72 100644 --- a/op-chain-ops/ether/migrate.go +++ b/op-chain-ops/ether/migrate.go @@ -33,7 +33,7 @@ var ( } ) -func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances []*migration.Allowance, chainID int, commit bool) (common.Hash, error) { +func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances []*migration.Allowance, chainID int, commit, noCheck bool) (common.Hash, error) { // Set of addresses that we will be migrating. addressesToMigrate := make(map[common.Address]bool) // Set of storage slots that we expect to see in the OVM ETH contract. @@ -120,7 +120,11 @@ func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances default: // Check if this slot is a variable. If it isn't, abort. if !ignoredSlots[k] { - log.Crit("missed storage key", "k", k.String(), "v", v.String()) + if noCheck { + log.Error("missed storage key", "k", k.String(), "v", v.String()) + } else { + log.Crit("missed storage key", "k", k.String(), "v", v.String()) + } } } @@ -132,13 +136,23 @@ func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances // had supply bugs. delta := new(big.Int).Sub(totalSupply, totalFound) if delta.Cmp(params.ExpectedSupplyDelta) != 0 { - log.Crit( - "supply mismatch", - "migrated", totalFound.String(), - "supply", totalSupply.String(), - "delta", delta.String(), - "exp_delta", params.ExpectedSupplyDelta.String(), - ) + if noCheck { + log.Error( + "supply mismatch", + "migrated", totalFound.String(), + "supply", totalSupply.String(), + "delta", delta.String(), + "exp_delta", params.ExpectedSupplyDelta.String(), + ) + } else { + log.Crit( + "supply mismatch", + "migrated", totalFound.String(), + "supply", totalSupply.String(), + "delta", delta.String(), + "exp_delta", params.ExpectedSupplyDelta.String(), + ) + } } log.Info( @@ -176,7 +190,11 @@ func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances // No accounts should have a balance in state. If they do, bail. if data.Balance.Sign() > 0 { - log.Crit("account has non-zero balance in state - should never happen", "addr", addr) + if noCheck { + log.Error("account has non-zero balance in state - should never happen", "addr", addr) + } else { + log.Crit("account has non-zero balance in state - should never happen", "addr", addr) + } } // Actually perform the migration by setting the appropriate values in state. @@ -209,11 +227,19 @@ func MigrateLegacyETH(db ethdb.Database, addresses []common.Address, allowances // Make sure that the amount we migrated matches the amount in // our original state. if totalMigrated.Cmp(totalFound) != 0 { - log.Crit( - "total migrated does not equal total OVM eth found", - "migrated", totalMigrated, - "found", totalFound, - ) + if noCheck { + log.Debug( + "total migrated does not equal total OVM eth found", + "migrated", totalMigrated, + "found", totalFound, + ) + } else { + log.Crit( + "total migrated does not equal total OVM eth found", + "migrated", totalMigrated, + "found", totalFound, + ) + } } // Set the total supply to 0 diff --git a/op-chain-ops/genesis/db_migration.go b/op-chain-ops/genesis/db_migration.go index f9453f169172d..b7becebf7aff0 100644 --- a/op-chain-ops/genesis/db_migration.go +++ b/op-chain-ops/genesis/db_migration.go @@ -107,7 +107,7 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m log.Info("Starting to migrate ERC20 ETH") addrs := migrationData.Addresses() - newRoot, err := ether.MigrateLegacyETH(ldb, addrs, migrationData.OvmAllowances, int(config.L1ChainID), commit) + newRoot, err := ether.MigrateLegacyETH(ldb, addrs, migrationData.OvmAllowances, int(config.L1ChainID), commit, noCheck) if err != nil { return nil, fmt.Errorf("cannot migrate legacy eth: %w", err) } From fcb58c1ad924259e33a667c9b9761528942b8998 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 1 Dec 2022 14:53:31 -0800 Subject: [PATCH 7/7] op-chain-ops: migrate db monotonic timestamp Ensure that the timestamps are monotonic when creating the bedrock transition block --- op-chain-ops/genesis/db_migration.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/op-chain-ops/genesis/db_migration.go b/op-chain-ops/genesis/db_migration.go index f31c256e0d05a..24fe5ec8f1955 100644 --- a/op-chain-ops/genesis/db_migration.go +++ b/op-chain-ops/genesis/db_migration.go @@ -58,6 +58,13 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m return nil, errors.New("must configure L2 genesis block base fee per gas") } + // Ensure monotonic timestamps + if uint64(config.L2OutputOracleStartingTimestamp) <= header.Time { + return nil, fmt.Errorf( + "L2 output oracle starting timestamp (%d) is less than the header timestamp (%d)", config.L2OutputOracleStartingTimestamp, header.Time, + ) + } + underlyingDB := state.NewDatabaseWithConfig(ldb, &trie.Config{ Preimages: true, })