Merge branch 'develop' into aj/fpp-fix-build

.circleci/config.yml

@@ -60,7 +60,7 @@ commands:
 jobs:
   yarn-monorepo:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: large
     steps:
       - checkout
@@ -288,7 +288,7 @@ jobs:
 
   contracts-bedrock-tests:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: large
     steps:
       - checkout
@@ -318,7 +318,7 @@ jobs:
 
   contracts-bedrock-checks:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - attach_workspace: { at: "." }
@@ -378,7 +378,7 @@ jobs:
 
   contracts-bedrock-slither:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: large
     steps:
       - checkout
@@ -398,7 +398,7 @@ jobs:
 
   contracts-bedrock-validate-spaces:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - attach_workspace: { at: "." }
@@ -415,7 +415,7 @@ jobs:
 
   bedrock-echidna-build:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - attach_workspace: { at: "." }
@@ -433,7 +433,7 @@ jobs:
 
   bedrock-echidna-run:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     parameters:
       echidna_target:
         description: Which echidna fuzz contract to run
@@ -460,7 +460,7 @@ jobs:
 
   op-bindings-build:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: medium
     steps:
       - checkout
@@ -489,7 +489,7 @@ jobs:
         description: Coverage flag name
         type: string
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: large
     steps:
       - checkout
@@ -535,7 +535,7 @@ jobs:
 
   fuzz-op-node:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - check-changed:
@@ -547,7 +547,7 @@ jobs:
 
   depcheck:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - attach_workspace: { at: "." }
@@ -609,7 +609,7 @@ jobs:
         description: Go Module Name
         type: string
     docker:
-      - image: ethereumoptimism/ci-builder:latest # only used to enable codecov.
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest # only used to enable codecov.
     resource_class: xlarge
     steps:
       - checkout
@@ -637,7 +637,7 @@ jobs:
         description: If the op-e2e package should use HTTP clients
         type: string
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     resource_class: xlarge
     steps:
       - checkout
@@ -676,7 +676,7 @@ jobs:
         type: string
         default: this-package-does-not-exist
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
       - image: cimg/postgres:14.1
     steps:
       - checkout
@@ -705,7 +705,7 @@ jobs:
 
   geth-tests:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - check-changed:
@@ -925,7 +925,7 @@ jobs:
 
   go-mod-tidy:
     docker:
-      - image: ethereumoptimism/ci-builder:latest
+      - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest
     steps:
       - checkout
       - run:

.github/workflows/release.yml

@@ -27,7 +27,6 @@ jobs:
       op-exporter: ${{ steps.packages.outputs.op-exporter }}
       l2geth-exporter: ${{ steps.packages.outputs.l2geth-exporter }}
       batch-submitter-service: ${{ steps.packages.outputs.batch-submitter-service }}
-      ci-builder: ${{ steps.packages.outputs.ci-builder }}
       foundry: ${{ steps.packages.outputs.foundry }}
       endpoint-monitor: ${{ steps.packages.outputs.endpoint-monitor }}
 
@@ -159,32 +158,6 @@ jobs:
           push: true
           tags: ethereumoptimism/hardhat-node:${{ needs.release.outputs.hardhat-node }},ethereumoptimism/hardhat-node:latest
 
-  ci-builder:
-    name: Publish ci-builder ${{ needs.release.outputs.ci-builder }}
-    needs: release
-    if: needs.release.outputs.ci-builder != ''
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-
-      - name: Login to Docker Hub
-        uses: docker/login-action@v1
-        with:
-          username: ${{ secrets.DOCKERHUB_ACCESS_TOKEN_USERNAME }}
-          password: ${{ secrets.DOCKERHUB_ACCESS_TOKEN_SECRET }}
-
-      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v1
-
-      - name: Publish ci-builder
-        uses: docker/build-push-action@v2
-        with:
-          context: ./ops/docker/ci-builder
-          file: ./ops/docker/ci-builder/Dockerfile
-          push: true
-          tags: ethereumoptimism/ci-builder:${{ needs.release.outputs.ci-builder }},ethereumoptimism/ci-builder:latest
-
   foundry:
     name: Publish foundry ${{ needs.release.outputs.foundry }}
     needs: release

op-chain-ops/cmd/withdrawals/main.go

@@ -22,11 +22,14 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/eth"
 	"github.com/ethereum/go-ethereum/eth/tracers"
 	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/rpc"
 )
 
 // abiTrue represents the storage representation of the boolean
@@ -116,6 +119,10 @@ func main() {
 				Value: "bad-withdrawals.json",
 				Usage: "Path to write JSON file of bad withdrawals to manually inspect",
 			},
+			&cli.StringFlag{
+				Name:  "storage-out",
+				Usage: "Path to write text file of L2ToL1MessagePasser storage",
+			},
 		},
 		Action: func(ctx *cli.Context) error {
 			clients, err := util.NewClients(ctx)
@@ -163,10 +170,11 @@ func main() {
 			}
 
 			outfile := ctx.String("bad-withdrawals-out")
-			f, err := os.OpenFile(outfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o755)
+			f, err := os.OpenFile(outfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
 			if err != nil {
 				return err
 			}
+			defer f.Close()
 
 			// create a transactor
 			opts, err := newTransactor(ctx)
@@ -177,6 +185,28 @@ func main() {
 			// Need this to compare in event parsing
 			l1StandardBridgeAddress := common.HexToAddress(ctx.String("l1-standard-bridge-address"))
 
+			if storageOutfile := ctx.String("storage-out"); storageOutfile != "" {
+				ff, err := os.OpenFile(storageOutfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
+				if err != nil {
+					return err
+				}
+				defer ff.Close()
+
+				log.Info("Fetching storage for L2ToL1MessagePasser")
+				if storageRange, err := callStorageRange(clients, predeploys.L2ToL1MessagePasserAddr); err != nil {
+					log.Info("error getting storage range", "err", err)
+				} else {
+					str := ""
+					for key, value := range storageRange {
+						str += fmt.Sprintf("%s: %s\n", key.Hex(), value.Hex())
+					}
+					_, err = ff.WriteString(str)
+					if err != nil {
+						return err
+					}
+				}
+			}
+
 			// iterate over all of the withdrawals and submit them
 			for i, wd := range wds {
 				log.Info("Processing withdrawal", "index", i)
@@ -234,7 +264,7 @@ func main() {
 				// successful messages can be skipped, received messages failed
 				// their execution and should be replayed
 				if isSuccessNew {
-					log.Info("Message already relayed", "index", i, "hash", hash, "slot", slot)
+					log.Info("Message already relayed", "index", i, "hash", hash.Hex(), "slot", slot.Hex())
 					continue
 				}
 
@@ -248,7 +278,7 @@ func main() {
 
 				// the value should be set to a boolean in storage
 				if !bytes.Equal(storageValue, abiTrue.Bytes()) {
-					return fmt.Errorf("storage slot %x not found in state", slot)
+					return fmt.Errorf("storage slot %x not found in state", slot.Hex())
 				}
 
 				legacySlot, err := wd.StorageSlot()
@@ -443,10 +473,48 @@ func callTrace(c *util.Clients, receipt *types.Receipt) (callFrame, error) {
 		Tracer: &tracer,
 	}
 	err := c.L1RpcClient.Call(&finalizationTrace, "debug_traceTransaction", receipt.TxHash, traceConfig)
+	return finalizationTrace, err
+}
+
+func callStorageRangeAt(
+	client *rpc.Client,
+	blockHash common.Hash,
+	txIndex int,
+	addr common.Address,
+	keyStart hexutil.Bytes,
+	maxResult int,
+) (*eth.StorageRangeResult, error) {
+	var storageRange *eth.StorageRangeResult
+	err := client.Call(&storageRange, "debug_storageRangeAt", blockHash, txIndex, addr, keyStart, maxResult)
+	return storageRange, err
+}
+
+func callStorageRange(c *util.Clients, addr common.Address) (state.Storage, error) {
+	header, err := c.L2Client.HeaderByNumber(context.Background(), nil)
 	if err != nil {
-		return finalizationTrace, err
+		return nil, err
 	}
-	return finalizationTrace, err
+	hash := header.Hash()
+	keyStart := hexutil.Bytes(common.Hash{}.Bytes())
+	maxResult := 1000
+
+	ret := make(state.Storage)
+
+	for {
+		result, err := callStorageRangeAt(c.L2RpcClient, hash, 0, addr, keyStart, maxResult)
+		if err != nil {
+			return nil, err
+		}
+		for key, value := range result.Storage {
+			ret[key] = value.Value
+		}
+		if result.NextKey == nil {
+			break
+		} else {
+			keyStart = hexutil.Bytes(result.NextKey.Bytes())
+		}
+	}
+	return ret, nil
 }
 
 // handleFinalizeETHWithdrawal will ensure that the calldata is correct
@@ -709,9 +777,13 @@ func newWithdrawals(ctx *cli.Context, l1ChainID *big.Int) ([]*crossdomain.Legacy
 	witnessFile := ctx.String("witness-file")
 
 	log.Debug("Migration data", "ovm-path", ovmMsgs, "evm-messages", evmMsgs, "witness-file", witnessFile)
-	ovmMessages, err := crossdomain.NewSentMessageFromJSON(ovmMsgs)
-	if err != nil {
-		return nil, err
+	var ovmMessages []*crossdomain.SentMessage
+	var err error
+	if ovmMsgs != "" {
+		ovmMessages, err = crossdomain.NewSentMessageFromJSON(ovmMsgs)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	// use empty ovmMessages if its not mainnet. The mainnet messages are

op-chain-ops/crossdomain/migrate.go

@@ -96,17 +96,12 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 	return w, nil
 }
 
+// MigrateWithdrawalGasLimit computes the gas limit for the migrated withdrawal.
 func MigrateWithdrawalGasLimit(data []byte) uint64 {
-	// Compute the cost of the calldata
-	dataCost := uint64(0)
-	for _, b := range data {
-		if b == 0 {
-			dataCost += params.TxDataZeroGas
-		} else {
-			dataCost += params.TxDataNonZeroGasEIP2028
-		}
-	}
-
+	// Compute the upper bound on the gas limit. This could be more
+	// accurate if individual 0 bytes and non zero bytes were accounted
+	// for.
+	dataCost := uint64(len(data)) * params.TxDataNonZeroGasEIP2028
 	// Set the outer gas limit. This cannot be zero
 	gasLimit := dataCost + 200_000
 	// Cap the gas limit to be 25 million to prevent creating withdrawals

op-chain-ops/crossdomain/migrate_test.go

@@ -71,15 +71,15 @@ func TestMigrateWithdrawalGasLimit(t *testing.T) {
 		},
 		{
 			input:  []byte{0xff, 0x00},
-			output: 200_000 + 16 + 4,
+			output: 200_000 + 16 + 16,
 		},
 		{
 			input:  []byte{0x00},
-			output: 200_000 + 4,
+			output: 200_000 + 16,
 		},
 		{
 			input:  []byte{0x00, 0x00, 0x00},
-			output: 200_000 + 4 + 4 + 4,
+			output: 200_000 + 16 + 16 + 16,
 		},
 	}
 

ops/docker/ci-builder/package.json

-{
-  "name": "@eth-optimism/ci-builder",
-  "version": "0.5.0",
-  "scripts": {},
-  "license": "MIT",
-  "dependencies": {}
-}

package.json

@@ -16,7 +16,6 @@
       "ops/docker/hardhat",
       "ops/docker/go-builder",
       "ops/docker/js-builder",
-      "ops/docker/ci-builder",
       "ops/docker/foundry",
       "endpoint-monitor"
     ],

packages/sdk/src/utils/message-utils.ts

-import { hashWithdrawal, calldataCost } from '@eth-optimism/core-utils'
-import { BigNumber } from 'ethers'
+import { hashWithdrawal } from '@eth-optimism/core-utils'
+import { BigNumber, utils } from 'ethers'
 
 import { LowLevelMessage } from '../interfaces'
 
+const { hexDataLength } = utils
+
 /**
  * Utility for hashing a LowLevelMessage object.
  *
@@ -25,7 +27,7 @@ export const hashLowLevelMessage = (message: LowLevelMessage): string => {
  */
 export const migratedWithdrawalGasLimit = (data: string): BigNumber => {
   // Compute the gas limit and cap at 25 million
-  const dataCost = calldataCost(data)
+  const dataCost = BigNumber.from(hexDataLength(data)).mul(16)
   let minGasLimit = dataCost.add(200_000)
   if (minGasLimit.gt(25_000_000)) {
     minGasLimit = BigNumber.from(25_000_000)

packages/sdk/test/utils/message-utils.spec.ts

@@ -15,9 +15,9 @@ describe('Message Utils', () => {
       const tests = [
         { input: '0x', result: BigNumber.from(200_000) },
         { input: '0xff', result: BigNumber.from(200_000 + 16) },
-        { input: '0xff00', result: BigNumber.from(200_000 + 16 + 4) },
-        { input: '0x00', result: BigNumber.from(200_000 + 4) },
-        { input: '0x000000', result: BigNumber.from(200_000 + 4 + 4 + 4) },
+        { input: '0xff00', result: BigNumber.from(200_000 + 16 + 16) },
+        { input: '0x00', result: BigNumber.from(200_000 + 16) },
+        { input: '0x000000', result: BigNumber.from(200_000 + 16 + 16 + 16) },
       ]
 
       for (const test of tests) {

specs/withdrawals.md

@@ -18,12 +18,17 @@ an L2 account to an L1 account.
 more specific terms to differentiate:
 
 - A _withdrawal initiating transaction_ refers specifically to a transaction on L2 sent to the Withdrawals predeploy.
+- A _withdrawal proving transaction_ refers specifically to an L1 transaction
+  which proves the withdrawal is correct (that it has been included in a merkle
+  tree whose root is available on L1).
 - 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
-properties of the message in its storage. Withdrawals are finalized on L1 via a call to the `OptimismPortal`
-contract, which proves the inclusion of this withdrawal message.
+properties of the message in its storage.
+Withdrawals are proven on L1 via a call to the `OptimismPortal`, which proves the inclusion of this withdrawal message.
+Withdrawals are finalized on L1 via a call to the `OptimismPortal` contract,
+which verifies that the fault challenge period has passed since the withdrawal message has been proved.
 
 In this way, withdrawals are different from [deposits][g-deposits] which make use of a special transaction type in the
 [execution engine][g-execution-engine] client. Rather, withdrawals transaction must use smart contracts on L1 for
@@ -59,18 +64,20 @@ This is a very simple contract that stores the 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.
+1. A [relayer][g-relayer] submits a withdrawal proving transaction with the required inputs
+   to the `OptimismPortal` contract.
+   The relayer is not necessarily 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.
 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.
+   `getL2Output()` function, and performs the remainder of the verification process internally.
 1. If proof verification fails, the call reverts. Otherwise 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.
 1. After the withdrawal is proven, it enters a 7 day challenge period, allowing time for other network participants
    to challenge the integrity of the corresponding output root.
-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.
+1. Once the challenge period has passed, a relayer submits a withdrawal finalizing transaction to the
+   `OptimismPortal` contract.
+   The relayer doesn't need to be the same entity that initiated the withdrawal on L2.
 1. The `OptimismPortal` contract receives the withdrawal transaction data and verifies that the withdrawal has
    both been proven and passed the challenge period.
 1. If the requirements are not met, the call reverts. Otherwise the call is forwarded, and the hash is recorded to
@@ -102,7 +109,7 @@ interface L2ToL1MessagePasser {
 
     function initiateWithdrawal(address _target, uint256 _gasLimit, bytes memory _data) payable external;
 
-    function nonce() view external returns (uint256);
+    function messageNonce() public view returns (uint256);
 
     function sentMessages(bytes32) view external returns (bool);
 }
@@ -139,13 +146,14 @@ withdrawals:
 ```js
 interface OptimismPortal {
 
-    event WithdrawalFinalized(bytes32 indexed);
+    event WithdrawalFinalized(bytes32 indexed withdrawalHash, bool success);
+
 
     function l2Sender() returns(address) external;
 
     function proveWithdrawalTransaction(
         Types.WithdrawalTransaction memory _tx,
-        uint256 _l2BlockNumber,
+        uint256 _l2OutputIndex,
         Types.OutputRootProof calldata _outputRootProof,
         bytes[] calldata _withdrawalProof
     ) external;
@@ -168,14 +176,14 @@ 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 that corresponds to the output root.
+  - `l2OutputIndex`: The index in the L2 outputs where the applicable output root may be found.
   - `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 `l2BlockNumber` must be the block number that corresponds to the `OutputProposal` being proven.
-1. `L2OutputOracle.getL2OutputAfter(l2BlockNumber)` returns a non-zero `OutputProposal`.
+1. The `l2OutputIndex` must be the index in the L2 outputs that contains the applicable output root.
+1. `L2OutputOracle.getL2Output(l2OutputIndex)` 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.
@@ -190,13 +198,13 @@ These inputs must satisfy the following conditions:
 
    [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. For each withdrawal initiated on L2 (i.e. with a unique `messageNonce()`), the following properties must hold:
    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
+      1. Modifying the `target`, `data`, 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`.