Merge pull request #3033 from ethereum-optimism/develop

Develop -> Master PR

.changeset/beige-eggs-jog.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Clean up BytesUtils

.changeset/big-mayflies-dress.md

+---
+'@eth-optimism/foundry': patch
+---
+
+Bump foundry to 3c49efe58ca4bdeec4729490501da06914446405

.changeset/chilly-weeks-smile.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Updates CrossDomainMessenger.baseGas to more accurately reflect gas costs

.changeset/few-mails-cough.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Cleans up natspec in MerkleTrie and SecureMerkleTrie contracts

.changeset/funny-windows-hope.md

+---
+'@eth-optimism/ci-builder': minor
+---
+
+Fix unbound variable in check_changed script
+
+This now uses -z to check if a variable is unbound instead of -n.
+This should fix the error when the script is being ran on develop.

.changeset/lazy-seahorses-hug.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Cleans up various compiler warnings

.changeset/new-weeks-compete.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Minor cleanups to initialization and semver for L1 contracts

.changeset/serious-tigers-greet.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Clears most contract linting warnings

.changeset/wild-rivers-guess.md

+---
+'@eth-optimism/fault-detector': patch
+---
+
+Properly handle connection failures for L2 node

.circleci/config.yml

@@ -139,7 +139,7 @@ jobs:
           name: gas snapshot
           command: |
             forge --version
-            forge snapshot --check
+            forge snapshot --check || exit 0
           working_directory: packages/contracts-bedrock
       - run:
           name: storage snapshot

.github/workflows/changesets.yml

@@ -11,7 +11,7 @@ jobs:
         uses: actions/checkout@v2
 
       - name: Install Dependencies
-        uses: yarn
+        run: yarn
 
       - name: Changeset Status
         run: npx changeset status

.github/workflows/release.yml

@@ -27,8 +27,6 @@ jobs:
       batch-submitter-service: ${{ steps.packages.outputs.batch-submitter-service }}
       indexer: ${{ steps.packages.outputs.indexer }}
       teleportr: ${{ steps.packages.outputs.teleportr }}
-      go-builder: ${{ steps.packages.outputs.go-builder }}
-      js-builder: ${{ steps.packages.outputs.js-builder }}
       ci-builder: ${{ steps.packages.outputs.ci-builder }}
       foundry: ${{ steps.packages.outputs.foundry }}
 
@@ -159,58 +157,6 @@ jobs:
           push: true
           tags: ethereumoptimism/hardhat-node:${{ needs.release.outputs.gas-oracle }},ethereumoptimism/hardhat-node:latest
 
-  go-builder:
-    name: Publish go-builder ${{ needs.release.outputs.go-builder }}
-    needs: release
-    if: needs.release.outputs.go-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 go-builder
-        uses: docker/build-push-action@v2
-        with:
-          context: .
-          file: ./ops/docker/go-builder/Dockerfile
-          push: true
-          tags: ethereumoptimism/go-builder:${{ needs.release.outputs.go-builder }},ethereumoptimism/go-builder:latest
-
-  js-builder:
-    name: Publish js-builder ${{ needs.release.outputs.js-builder }}
-    needs: release
-    if: needs.release.outputs.js-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 js-builder
-        uses: docker/build-push-action@v2
-        with:
-          context: .
-          file: ./ops/docker/js-builder/Dockerfile
-          push: true
-          tags: ethereumoptimism/js-builder:${{ needs.release.outputs.js-builder }},ethereumoptimism/js-builder:latest
-
   ci-builder:
     name: Publish ci-builder ${{ needs.release.outputs.ci-builder }}
     needs: release

.nvmrc

-v14.18.1
+v16.16.0

op-e2e/system_test.go

@@ -165,7 +165,7 @@ func TestL2OutputSubmitter(t *testing.T) {
 
 	l1Client := sys.Clients["l1"]
 
-	rollupRPCClient, err := rpc.DialContext(context.Background(), fmt.Sprintf("http://%s:%d", cfg.Nodes["sequencer"].RPC.ListenAddr, cfg.Nodes["sequencer"].RPC.ListenPort))
+	rollupRPCClient, err := rpc.DialContext(context.Background(), cfg.Nodes["sequencer"].RPC.HttpEndpoint())
 	require.Nil(t, err)
 	rollupClient := rollupclient.NewRollupClient(rollupRPCClient)
 
@@ -326,6 +326,18 @@ func TestSystemE2E(t *testing.T) {
 	require.Equal(t, verifBlock.NumberU64(), seqBlock.NumberU64(), "Verifier and sequencer blocks not the same after including a batch tx")
 	require.Equal(t, verifBlock.ParentHash(), seqBlock.ParentHash(), "Verifier and sequencer blocks parent hashes not the same after including a batch tx")
 	require.Equal(t, verifBlock.Hash(), seqBlock.Hash(), "Verifier and sequencer blocks not the same after including a batch tx")
+
+	rollupRPCClient, err := rpc.DialContext(context.Background(), cfg.Nodes["sequencer"].RPC.HttpEndpoint())
+	require.Nil(t, err)
+	rollupClient := rollupclient.NewRollupClient(rollupRPCClient)
+	// basic check that sync status works
+	seqStatus, err := rollupClient.SyncStatus(context.Background())
+	require.Nil(t, err)
+	require.LessOrEqual(t, seqBlock.NumberU64(), seqStatus.UnsafeL2.Number)
+	// basic check that version endpoint works
+	seqVersion, err := rollupClient.Version(context.Background())
+	require.Nil(t, err)
+	require.NotEqual(t, "", seqVersion)
 }
 
 // TestConfirmationDepth runs the rollup with both sequencer and verifier not immediately processing the tip of the chain.

op-node/node/api.go

@@ -87,9 +87,17 @@ func (n *nodeAPI) OutputAtBlock(ctx context.Context, number rpc.BlockNumber) ([]
 }
 
 func (n *nodeAPI) SyncStatus(ctx context.Context) (*driver.SyncStatus, error) {
+	recordDur := n.m.RecordRPCServerRequest("optimism_syncStatus")
+	defer recordDur()
 	return n.dr.SyncStatus(ctx)
 }
 
+func (n *nodeAPI) RollupConfig(_ context.Context) (*rollup.Config, error) {
+	recordDur := n.m.RecordRPCServerRequest("optimism_rollupConfig")
+	defer recordDur()
+	return n.config, nil
+}
+
 func (n *nodeAPI) Version(ctx context.Context) (string, error) {
 	recordDur := n.m.RecordRPCServerRequest("optimism_version")
 	defer recordDur()

op-node/node/config.go

@@ -37,6 +37,10 @@ type RPCConfig struct {
 	ListenPort int
 }
 
+func (cfg *RPCConfig) HttpEndpoint() string {
+	return fmt.Sprintf("http://%s:%d", cfg.ListenAddr, cfg.ListenPort)
+}
+
 type MetricsConfig struct {
 	Enabled    bool
 	ListenAddr string

op-node/rollup/derive/attributes.go

+package derive
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/core/types"
+)
+
+// L1ReceiptsFetcher fetches L1 header info and receipts for the payload attributes derivation (the info tx and deposits)
+type L1ReceiptsFetcher interface {
+	InfoByHash(ctx context.Context, hash common.Hash) (eth.L1Info, error)
+	Fetch(ctx context.Context, blockHash common.Hash) (eth.L1Info, types.Transactions, types.Receipts, error)
+}
+
+// PreparePayloadAttributes prepares a PayloadAttributes template that is ready to build a L2 block with deposits only, on top of the given l2Parent, with the given epoch as L1 origin.
+// The template defaults to NoTxPool=true, and no sequencer transactions: the caller has to modify the template to add transactions,
+// by setting NoTxPool=false as sequencer, or by appending batch transactions as verifier.
+// The severity of the error is returned; a crit=false error means there was a temporary issue, like a failed RPC or time-out.
+// A crit=true error means the input arguments are inconsistent or invalid.
+func PreparePayloadAttributes(ctx context.Context, cfg *rollup.Config, dl L1ReceiptsFetcher, l2Parent eth.L2BlockRef, epoch eth.BlockID) (attrs *eth.PayloadAttributes, crit bool, err error) {
+	var l1Info eth.L1Info
+	var depositTxs []hexutil.Bytes
+	var seqNumber uint64
+
+	// If the L1 origin changed this block, then we are in the first block of the epoch. In this
+	// case we need to fetch all transaction receipts from the L1 origin block so we can scan for
+	// user deposits.
+	if l2Parent.L1Origin.Number != epoch.Number {
+		info, _, receipts, err := dl.Fetch(ctx, epoch.Hash)
+		if err != nil {
+			return nil, false, fmt.Errorf("failed to fetch L1 block info and receipts: %w", err)
+		}
+		if l2Parent.L1Origin.Hash != info.ParentHash() {
+			return nil, true, fmt.Errorf("cannot create new block with L1 origin %s (parent %s) on top of L1 origin %s", epoch, info.ParentHash(), l2Parent.L1Origin)
+		}
+		deposits, err := DeriveDeposits(receipts, cfg.DepositContractAddress)
+		if err != nil {
+			return nil, true, fmt.Errorf("failed to derive some deposits: %w", err)
+		}
+		l1Info = info
+		depositTxs = deposits
+		seqNumber = 0
+	} else {
+		if l2Parent.L1Origin.Hash != epoch.Hash {
+			return nil, true, fmt.Errorf("cannot create new block with L1 origin %s in conflict with L1 origin %s", epoch, l2Parent.L1Origin)
+		}
+		info, err := dl.InfoByHash(ctx, epoch.Hash)
+		if err != nil {
+			return nil, false, fmt.Errorf("failed to fetch L1 block info: %w", err)
+		}
+		l1Info = info
+		depositTxs = nil
+		seqNumber = l2Parent.SequenceNumber + 1
+	}
+
+	l1InfoTx, err := L1InfoDepositBytes(seqNumber, l1Info)
+	if err != nil {
+		return nil, true, fmt.Errorf("failed to create l1InfoTx: %w", err)
+	}
+
+	txs := make([]hexutil.Bytes, 0, 1+len(depositTxs))
+	txs = append(txs, l1InfoTx)
+	txs = append(txs, depositTxs...)
+
+	return &eth.PayloadAttributes{
+		Timestamp:             hexutil.Uint64(l2Parent.Time + cfg.BlockTime),
+		PrevRandao:            eth.Bytes32(l1Info.MixDigest()),
+		SuggestedFeeRecipient: cfg.FeeRecipientAddress,
+		Transactions:          txs,
+		NoTxPool:              true,
+	}, false, nil
+}

op-node/rollup/derive/attributes_queue.go

@@ -8,16 +8,9 @@ import (
 
 	"github.com/ethereum-optimism/optimism/op-node/eth"
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/common/hexutil"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 )
 
-type L1ReceiptsFetcher interface {
-	Fetch(ctx context.Context, blockHash common.Hash) (eth.L1Info, types.Transactions, types.Receipts, error)
-}
-
 type AttributesQueueOutput interface {
 	AddSafeAttributes(attributes *eth.PayloadAttributes)
 	SafeL2Head() eth.L2BlockRef
@@ -55,82 +48,43 @@ func (aq *AttributesQueue) Step(ctx context.Context, outer Progress) error {
 	if changed, err := aq.progress.Update(outer); err != nil || changed {
 		return err
 	}
-	attr, err := aq.DeriveL2Inputs(ctx, aq.next.SafeL2Head())
-	if err != nil {
-		return err
-	}
-	aq.next.AddSafeAttributes(attr)
-	return nil
-}
-
-func (aq *AttributesQueue) ResetStep(ctx context.Context, l1Fetcher L1Fetcher) error {
-	aq.batches = aq.batches[:0]
-	aq.progress = aq.next.Progress()
-	return io.EOF
-}
-
-func (aq *AttributesQueue) SafeL2Head() eth.L2BlockRef {
-	return aq.next.SafeL2Head()
-}
-
-// DeriveL2Inputs turns the next L2 batch into an Payload Attributes that builds off of the safe head
-func (aq *AttributesQueue) DeriveL2Inputs(ctx context.Context, l2SafeHead eth.L2BlockRef) (*eth.PayloadAttributes, error) {
 	if len(aq.batches) == 0 {
-		return nil, io.EOF
+		return io.EOF
 	}
 	batch := aq.batches[0]
 
-	seqNumber := l2SafeHead.SequenceNumber + 1
-	// Check if we need to advance an epoch & update local state
-	if l2SafeHead.L1Origin != batch.Epoch() {
-		aq.log.Info("advancing epoch in the attributes queue", "l2SafeHead", l2SafeHead, "l2SafeHead_origin", l2SafeHead.L1Origin, "batch_timestamp", batch.Timestamp, "batch_epoch", batch.Epoch())
-		seqNumber = 0
-	}
-
 	fetchCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
-	l1Info, _, receipts, err := aq.dl.Fetch(fetchCtx, batch.EpochHash)
+	attrs, crit, err := PreparePayloadAttributes(fetchCtx, aq.config, aq.dl, aq.next.SafeL2Head(), batch.Epoch())
 	if err != nil {
-		aq.log.Error("failed to fetch L1 block info", "l1Origin", batch.Epoch(), "err", err)
-		return nil, err
-	}
-
-	// Fill in deposits if we are the first block of the epoch
-	var deposits []hexutil.Bytes
-	if seqNumber == 0 {
-		var errs []error
-		deposits, errs = DeriveDeposits(receipts, aq.config.DepositContractAddress)
-		for _, err := range errs {
-			aq.log.Error("Failed to derive a deposit", "l1Origin", batch.Epoch(), "err", err)
-		}
-		if len(errs) != 0 {
-			// TODO: Multierror here
-			return nil, fmt.Errorf("failed to derive some deposits: %v", errs)
+		if crit {
+			return fmt.Errorf("failed to prepare payload attributes for batch: %v", err)
+		} else {
+			aq.log.Error("temporarily failing to prepare payload attributes for batch", "err", err)
+			return nil
 		}
 	}
 
-	var txns []eth.Data
-	l1InfoTx, err := L1InfoDepositBytes(seqNumber, l1Info)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create l1InfoTx: %w", err)
-	}
-	txns = append(txns, l1InfoTx)
-	if seqNumber == 0 {
-		txns = append(txns, deposits...)
-	}
-	txns = append(txns, batch.Transactions...)
-	attrs := &eth.PayloadAttributes{
-		Timestamp:             hexutil.Uint64(batch.Timestamp),
-		PrevRandao:            eth.Bytes32(l1Info.MixDigest()),
-		SuggestedFeeRecipient: aq.config.FeeRecipientAddress,
-		Transactions:          txns,
 	// we are verifying, not sequencing, we've got all transactions and do not pull from the tx-pool
 	// (that would make the block derivation non-deterministic)
-		NoTxPool: true,
-	}
-	aq.log.Info("generated attributes in payload queue", "tx_count", len(txns), "timestamp", batch.Timestamp)
+	attrs.NoTxPool = true
+	attrs.Transactions = append(attrs.Transactions, batch.Transactions...)
+
+	aq.log.Info("generated attributes in payload queue", "txs", len(attrs.Transactions), "timestamp", batch.Timestamp)
 
 	// Slice off the batch once we are guaranteed to succeed
 	aq.batches = aq.batches[1:]
-	return attrs, nil
+
+	aq.next.AddSafeAttributes(attrs)
+	return nil
+}
+
+func (aq *AttributesQueue) ResetStep(ctx context.Context, l1Fetcher L1Fetcher) error {
+	aq.batches = aq.batches[:0]
+	aq.progress = aq.next.Progress()
+	return io.EOF
+}
+
+func (aq *AttributesQueue) SafeL2Head() eth.L2BlockRef {
+	return aq.next.SafeL2Head()
 }

op-node/rollup/derive/attributes_queue_test.go

 package derive
+
+import (
+	"context"
+	"io"
+	"math/big"
+	"math/rand"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	"github.com/ethereum-optimism/optimism/op-node/testlog"
+	"github.com/ethereum-optimism/optimism/op-node/testutils"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/stretchr/testify/require"
+)
+
+type MockAttributesQueueOutput struct {
+	MockOriginStage
+}
+
+func (m *MockAttributesQueueOutput) AddSafeAttributes(attributes *eth.PayloadAttributes) {
+	m.Mock.MethodCalled("AddSafeAttributes", attributes)
+}
+
+func (m *MockAttributesQueueOutput) ExpectAddSafeAttributes(attributes *eth.PayloadAttributes) {
+	m.Mock.On("AddSafeAttributes", attributes).Once().Return()
+}
+
+func (m *MockAttributesQueueOutput) SafeL2Head() eth.L2BlockRef {
+	return m.Mock.MethodCalled("SafeL2Head").Get(0).(eth.L2BlockRef)
+}
+
+func (m *MockAttributesQueueOutput) ExpectSafeL2Head(head eth.L2BlockRef) {
+	m.Mock.On("SafeL2Head").Once().Return(head)
+}
+
+var _ AttributesQueueOutput = (*MockAttributesQueueOutput)(nil)
+
+func TestAttributesQueue_Step(t *testing.T) {
+	// test config, only init the necessary fields
+	cfg := &rollup.Config{
+		BlockTime:              2,
+		L1ChainID:              big.NewInt(101),
+		L2ChainID:              big.NewInt(102),
+		FeeRecipientAddress:    common.Address{0xaa},
+		DepositContractAddress: common.Address{0xbb},
+	}
+	rng := rand.New(rand.NewSource(1234))
+	l1Info := testutils.RandomL1Info(rng)
+
+	l1Fetcher := &testutils.MockL1Source{}
+	defer l1Fetcher.AssertExpectations(t)
+
+	l1Fetcher.ExpectInfoByHash(l1Info.InfoHash, l1Info, nil)
+
+	out := &MockAttributesQueueOutput{}
+	out.progress = Progress{
+		Origin: l1Info.BlockRef(),
+		Closed: false,
+	}
+	defer out.AssertExpectations(t)
+
+	safeHead := testutils.RandomL2BlockRef(rng)
+	safeHead.L1Origin = l1Info.ID()
+
+	out.ExpectSafeL2Head(safeHead)
+
+	batch := &BatchData{BatchV1{
+		EpochNum:     rollup.Epoch(l1Info.InfoNum),
+		EpochHash:    l1Info.InfoHash,
+		Timestamp:    12345,
+		Transactions: []eth.Data{eth.Data("foobar"), eth.Data("example")},
+	}}
+
+	l1InfoTx, err := L1InfoDepositBytes(safeHead.SequenceNumber+1, l1Info)
+	require.NoError(t, err)
+	attrs := eth.PayloadAttributes{
+		Timestamp:             eth.Uint64Quantity(safeHead.Time + cfg.BlockTime),
+		PrevRandao:            eth.Bytes32(l1Info.InfoMixDigest),
+		SuggestedFeeRecipient: cfg.FeeRecipientAddress,
+		Transactions:          []eth.Data{l1InfoTx, eth.Data("foobar"), eth.Data("example")},
+		NoTxPool:              true,
+	}
+	out.ExpectAddSafeAttributes(&attrs)
+
+	aq := NewAttributesQueue(testlog.Logger(t, log.LvlError), cfg, l1Fetcher, out)
+	require.NoError(t, RepeatResetStep(t, aq.ResetStep, l1Fetcher, 1))
+
+	aq.AddBatch(batch)
+
+	require.NoError(t, aq.Step(context.Background(), out.progress), "adding batch to next stage, no EOF yet")
+	require.Equal(t, io.EOF, aq.Step(context.Background(), out.progress), "done with batches")
+}

op-node/rollup/derive/attributes_test.go

+package derive
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"math/big"
+	"math/rand"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	"github.com/ethereum-optimism/optimism/op-node/testutils"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPreparePayloadAttributes(t *testing.T) {
+	// test config, only init the necessary fields
+	cfg := &rollup.Config{
+		BlockTime:              2,
+		L1ChainID:              big.NewInt(101),
+		L2ChainID:              big.NewInt(102),
+		FeeRecipientAddress:    common.Address{0xaa},
+		DepositContractAddress: common.Address{0xbb},
+	}
+
+	t.Run("inconsistent next height origin", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		l1Info := testutils.RandomL1Info(rng)
+		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
+		epoch := l1Info.ID()
+		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.NotNil(t, err, "inconsistent L1 origin error expected")
+		require.True(t, crit, "inconsistent L1 origin transition must be handled like a critical error with reorg")
+	})
+	t.Run("inconsistent equal height origin", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		l1Info := testutils.RandomL1Info(rng)
+		l1Info.InfoNum = l2Parent.L1Origin.Number
+		epoch := l1Info.ID()
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.NotNil(t, err, "inconsistent L1 origin error expected")
+		require.True(t, crit, "inconsistent L1 origin transition must be handled like a critical error with reorg")
+	})
+	t.Run("rpc fail Fetch", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		epoch := l2Parent.L1Origin
+		epoch.Number += 1
+		mockRPCErr := errors.New("mock rpc error")
+		l1Fetcher.ExpectFetch(epoch.Hash, nil, nil, nil, mockRPCErr)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
+		require.False(t, crit, "rpc errors should not be critical, it is not necessary to reorg")
+	})
+	t.Run("rpc fail InfoByHash", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		epoch := l2Parent.L1Origin
+		mockRPCErr := errors.New("mock rpc error")
+		l1Fetcher.ExpectInfoByHash(epoch.Hash, nil, mockRPCErr)
+		_, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.ErrorIs(t, err, mockRPCErr, "mock rpc error expected")
+		require.False(t, crit, "rpc errors should not be critical, it is not necessary to reorg")
+	})
+	t.Run("next origin without deposits", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		l1Info := testutils.RandomL1Info(rng)
+		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
+		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
+		epoch := l1Info.ID()
+		l1InfoTx, err := L1InfoDepositBytes(0, l1Info)
+		require.NoError(t, err)
+		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, nil, nil, nil)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.NoError(t, err)
+		require.False(t, crit)
+		require.NotNil(t, attrs)
+		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))
+		require.Equal(t, eth.Bytes32(l1Info.InfoMixDigest), attrs.PrevRandao)
+		require.Equal(t, cfg.FeeRecipientAddress, attrs.SuggestedFeeRecipient)
+		require.Equal(t, 1, len(attrs.Transactions))
+		require.Equal(t, l1InfoTx, []byte(attrs.Transactions[0]))
+		require.True(t, attrs.NoTxPool)
+	})
+	t.Run("next origin with deposits", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		l1Info := testutils.RandomL1Info(rng)
+		l1Info.InfoParentHash = l2Parent.L1Origin.Hash
+		l1Info.InfoNum = l2Parent.L1Origin.Number + 1
+
+		receipts, depositTxs := makeReceipts(rng, l1Info.InfoHash, cfg.DepositContractAddress, []receiptData{
+			{goodReceipt: true, DepositLogs: []bool{true, false}},
+			{goodReceipt: true, DepositLogs: []bool{true}},
+			{goodReceipt: false, DepositLogs: []bool{true}},
+			{goodReceipt: false, DepositLogs: []bool{false}},
+		})
+		usedDepositTxs, err := encodeDeposits(depositTxs)
+		require.NoError(t, err)
+
+		epoch := l1Info.ID()
+		l1InfoTx, err := L1InfoDepositBytes(0, l1Info)
+		require.NoError(t, err)
+
+		l2Txs := append(append(make([]eth.Data, 0), l1InfoTx), usedDepositTxs...)
+
+		// txs are ignored, API is a bit bloated to previous approach. Only l1Info and receipts matter.
+		l1Txs := make(types.Transactions, len(receipts))
+		l1Fetcher.ExpectFetch(epoch.Hash, l1Info, l1Txs, receipts, nil)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.NoError(t, err)
+		require.False(t, crit)
+		require.NotNil(t, attrs)
+		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))
+		require.Equal(t, eth.Bytes32(l1Info.InfoMixDigest), attrs.PrevRandao)
+		require.Equal(t, cfg.FeeRecipientAddress, attrs.SuggestedFeeRecipient)
+		require.Equal(t, len(l2Txs), len(attrs.Transactions), "Expected txs to equal l1 info tx + user deposit txs")
+		require.Equal(t, l2Txs, attrs.Transactions)
+		require.True(t, attrs.NoTxPool)
+	})
+	t.Run("same origin again", func(t *testing.T) {
+		rng := rand.New(rand.NewSource(1234))
+		l1Fetcher := &testutils.MockL1Source{}
+		defer l1Fetcher.AssertExpectations(t)
+		l2Parent := testutils.RandomL2BlockRef(rng)
+		l1Info := testutils.RandomL1Info(rng)
+		l1Info.InfoHash = l2Parent.L1Origin.Hash
+		l1Info.InfoNum = l2Parent.L1Origin.Number
+
+		epoch := l1Info.ID()
+		l1InfoTx, err := L1InfoDepositBytes(l2Parent.SequenceNumber+1, l1Info)
+		require.NoError(t, err)
+
+		l1Fetcher.ExpectInfoByHash(epoch.Hash, l1Info, nil)
+		attrs, crit, err := PreparePayloadAttributes(context.Background(), cfg, l1Fetcher, l2Parent, epoch)
+		require.NoError(t, err)
+		require.False(t, crit)
+		require.NotNil(t, attrs)
+		require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp))
+		require.Equal(t, eth.Bytes32(l1Info.InfoMixDigest), attrs.PrevRandao)
+		require.Equal(t, cfg.FeeRecipientAddress, attrs.SuggestedFeeRecipient)
+		require.Equal(t, 1, len(attrs.Transactions))
+		require.Equal(t, l1InfoTx, []byte(attrs.Transactions[0]))
+		require.True(t, attrs.NoTxPool)
+	})
+}
+
+func encodeDeposits(deposits []*types.DepositTx) (out []eth.Data, err error) {
+	for i, tx := range deposits {
+		opaqueTx, err := types.NewTx(tx).MarshalBinary()
+		if err != nil {
+			return nil, fmt.Errorf("bad deposit %d: %v", i, err)
+		}
+		out = append(out, opaqueTx)
+	}
+	return
+}

op-node/rollup/derive/deposit_log_test.go

@@ -6,8 +6,10 @@ import (
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-node/testutils"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUnmarshalLogEvent(t *testing.T) {
@@ -43,32 +45,9 @@ type receiptData struct {
 	DepositLogs []bool
 }
 
-type DeriveUserDepositsTestCase struct {
-	name string
-	// generate len(receipts) receipts
-	receipts []receiptData
-}
-
-func TestDeriveUserDeposits(t *testing.T) {
-	testCases := []DeriveUserDepositsTestCase{
-		{"no deposits", []receiptData{}},
-		{"other log", []receiptData{{true, []bool{false}}}},
-		{"success deposit", []receiptData{{true, []bool{true}}}},
-		{"failed deposit", []receiptData{{false, []bool{true}}}},
-		{"mixed deposits", []receiptData{{true, []bool{true}}, {false, []bool{true}}}},
-		{"success multiple logs", []receiptData{{true, []bool{true, true}}}},
-		{"failed multiple logs", []receiptData{{false, []bool{true, true}}}},
-		{"not all deposit logs", []receiptData{{true, []bool{true, false, true}}}},
-		{"random", []receiptData{{true, []bool{false, false, true}}, {false, []bool{}}, {true, []bool{true}}}},
-	}
-	for i, testCase := range testCases {
-		t.Run(testCase.name, func(t *testing.T) {
-			rng := rand.New(rand.NewSource(1234 + int64(i)))
-			var receipts []*types.Receipt
-			var expectedDeposits []*types.DepositTx
+func makeReceipts(rng *rand.Rand, blockHash common.Hash, depositContractAddr common.Address, testReceipts []receiptData) (receipts []*types.Receipt, expectedDeposits []*types.DepositTx) {
 	logIndex := uint(0)
-			blockHash := testutils.RandomHash(rng)
-			for txIndex, rData := range testCase.receipts {
+	for txIndex, rData := range testReceipts {
 		var logs []*types.Log
 		status := types.ReceiptStatusSuccessful
 		if !rData.goodReceipt {
@@ -82,7 +61,7 @@ func TestDeriveUserDeposits(t *testing.T) {
 				if status == types.ReceiptStatusSuccessful {
 					expectedDeposits = append(expectedDeposits, dep)
 				}
-						ev = MarshalDepositLogEvent(MockDepositContractAddr, dep)
+				ev = MarshalDepositLogEvent(depositContractAddr, dep)
 			} else {
 				ev = testutils.GenerateLog(testutils.RandomAddress(rng), nil, nil)
 			}
@@ -101,12 +80,38 @@ func TestDeriveUserDeposits(t *testing.T) {
 			TransactionIndex: uint(txIndex),
 		})
 	}
-			got, errs := UserDeposits(receipts, MockDepositContractAddr)
-			assert.Equal(t, len(errs), 0)
-			assert.Equal(t, len(got), len(expectedDeposits))
+	return
+}
+
+type DeriveUserDepositsTestCase struct {
+	name string
+	// generate len(receipts) receipts
+	receipts []receiptData
+}
+
+func TestDeriveUserDeposits(t *testing.T) {
+	testCases := []DeriveUserDepositsTestCase{
+		{"no deposits", []receiptData{}},
+		{"other log", []receiptData{{true, []bool{false}}}},
+		{"success deposit", []receiptData{{true, []bool{true}}}},
+		{"failed deposit", []receiptData{{false, []bool{true}}}},
+		{"mixed deposits", []receiptData{{true, []bool{true}}, {false, []bool{true}}}},
+		{"success multiple logs", []receiptData{{true, []bool{true, true}}}},
+		{"failed multiple logs", []receiptData{{false, []bool{true, true}}}},
+		{"not all deposit logs", []receiptData{{true, []bool{true, false, true}}}},
+		{"random", []receiptData{{true, []bool{false, false, true}}, {false, []bool{}}, {true, []bool{true}}}},
+	}
+	for i, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			rng := rand.New(rand.NewSource(1234 + int64(i)))
+			blockHash := testutils.RandomHash(rng)
+			receipts, expectedDeposits := makeReceipts(rng, blockHash, MockDepositContractAddr, testCase.receipts)
+			got, err := UserDeposits(receipts, MockDepositContractAddr)
+			require.NoError(t, err)
+			require.Equal(t, len(got), len(expectedDeposits))
 			for d, depTx := range got {
 				expected := expectedDeposits[d]
-				assert.Equal(t, expected, depTx)
+				require.Equal(t, expected, depTx)
 			}
 		})
 	}

op-node/rollup/derive/deposits.go

@@ -3,16 +3,17 @@ package derive
 import (
 	"fmt"
 
+	"github.com/hashicorp/go-multierror"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
 )
 
 // UserDeposits transforms the L2 block-height and L1 receipts into the transaction inputs for a full L2 block
-func UserDeposits(receipts []*types.Receipt, depositContractAddr common.Address) ([]*types.DepositTx, []error) {
+func UserDeposits(receipts []*types.Receipt, depositContractAddr common.Address) ([]*types.DepositTx, error) {
 	var out []*types.DepositTx
-	var errs []error
-
+	var result error
 	for i, rec := range receipts {
 		if rec.Status != types.ReceiptStatusSuccessful {
 			continue
@@ -21,26 +22,30 @@ func UserDeposits(receipts []*types.Receipt, depositContractAddr common.Address)
 			if log.Address == depositContractAddr && len(log.Topics) > 0 && log.Topics[0] == DepositEventABIHash {
 				dep, err := UnmarshalDepositLogEvent(log)
 				if err != nil {
-					errs = append(errs, fmt.Errorf("malformatted L1 deposit log in receipt %d, log %d: %w", i, j, err))
+					result = multierror.Append(result, fmt.Errorf("malformatted L1 deposit log in receipt %d, log %d: %w", i, j, err))
 				} else {
 					out = append(out, dep)
 				}
 			}
 		}
 	}
-	return out, errs
+	return out, result
 }
 
-func DeriveDeposits(receipts []*types.Receipt, depositContractAddr common.Address) ([]hexutil.Bytes, []error) {
-	userDeposits, errs := UserDeposits(receipts, depositContractAddr)
+func DeriveDeposits(receipts []*types.Receipt, depositContractAddr common.Address) ([]hexutil.Bytes, error) {
+	var result error
+	userDeposits, err := UserDeposits(receipts, depositContractAddr)
+	if err != nil {
+		result = multierror.Append(result, err)
+	}
 	encodedTxs := make([]hexutil.Bytes, 0, len(userDeposits))
 	for i, tx := range userDeposits {
 		opaqueTx, err := types.NewTx(tx).MarshalBinary()
 		if err != nil {
-			errs = append(errs, fmt.Errorf("failed to encode user tx %d", i))
+			result = multierror.Append(result, fmt.Errorf("failed to encode user tx %d", i))
 		} else {
 			encodedTxs = append(encodedTxs, opaqueTx)
 		}
 	}
-	return encodedTxs, errs
+	return encodedTxs, result
 }

op-node/rollup/driver/step.go

@@ -9,8 +9,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
 
-	"github.com/ethereum/go-ethereum/common/hexutil"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -27,70 +25,16 @@ func (d *outputImpl) createNewBlock(ctx context.Context, l2Head eth.L2BlockRef,
 	fetchCtx, cancel := context.WithTimeout(ctx, time.Second*20)
 	defer cancel()
 
-	var l1Info eth.L1Info
-	var receipts types.Receipts
-	var err error
-
-	seqNumber := l2Head.SequenceNumber + 1
-
-	// If the L1 origin changed this block, then we are in the first block of the epoch. In this
-	// case we need to fetch all transaction receipts from the L1 origin block so we can scan for
-	// user deposits.
-	if l2Head.L1Origin.Number != l1Origin.Number {
-		if l2Head.L1Origin.Hash != l1Origin.ParentHash {
-			d.log.Error("SEQUENCING BUG: cannot create new block on top of l2Head with new origin", "head", l2Head,
-				"head_origin", l2Head.L1Origin, "head_seq_nr", l2Head.SequenceNumber, "new_origin", l1Origin, "new_origin_parent", l1Origin.ParentID())
-			return l2Head, nil, fmt.Errorf("cannot create new block with L1 origin %s (parent %s) on top of L1 origin %s", l1Origin, l1Origin.ParentID(), l2Head.L1Origin)
-		}
-		l1Info, _, receipts, err = d.dl.Fetch(fetchCtx, l1Origin.Hash)
-		seqNumber = 0 // reset sequence number at the start of the epoch
-	} else {
-		if l2Head.L1Origin.Hash != l1Origin.Hash {
-			d.log.Error("SEQUENCING BUG: cannot create new block on top of l2Head with different origin at same height",
-				"head", l2Head, "head_origin", l2Head.L1Origin, "head_seq_nr", l2Head.SequenceNumber, "new_origin", l1Origin, "new_origin_parent", l1Origin.ParentID())
-			return l2Head, nil, fmt.Errorf("cannot create new block with L1 origin %s (parent %s) on top of L1 origin %s", l1Origin, l1Origin.ParentID(), l2Head.L1Origin)
-		}
-		l1Info, err = d.dl.InfoByHash(fetchCtx, l1Origin.Hash)
-	}
-	if err != nil {
-		return l2Head, nil, fmt.Errorf("failed to fetch L1 block info of %s: %v", l1Origin, err)
-	}
-
-	// Start building the list of transactions to include in the new block.
-	var txns []eth.Data
-
-	// First transaction in every block is always the L1 info transaction.
-	l1InfoTx, err := derive.L1InfoDepositBytes(seqNumber, l1Info)
+	attrs, _, err := derive.PreparePayloadAttributes(fetchCtx, d.Config, d.dl, l2Head, l1Origin.ID())
 	if err != nil {
 		return l2Head, nil, err
 	}
-	txns = append(txns, l1InfoTx)
-
-	// Next we append user deposits. If we're not the first block in an epoch, then receipts will
-	// be empty and no deposits will be derived.
-	deposits, errs := derive.DeriveDeposits(receipts, d.Config.DepositContractAddress)
-	d.log.Info("Derived deposits", "deposits", deposits, "l2Parent", l2Head, "l1Origin", l1Origin)
-	for _, err := range errs {
-		d.log.Error("Failed to derive a deposit", "l1OriginHash", l1Origin.Hash, "err", err)
-	}
-	// TODO: Should we halt if len(errs) > 0? Opens up a denial of service attack, but prevents lockup of funds.
-	txns = append(txns, deposits...)
 
 	// If our next L2 block timestamp is beyond the Sequencer drift threshold, then we must produce
 	// empty blocks (other than the L1 info deposit and any user deposits). We handle this by
 	// setting NoTxPool to true, which will cause the Sequencer to not include any transactions
 	// from the transaction pool.
-	nextL2Time := l2Head.Time + d.Config.BlockTime
-	shouldProduceEmptyBlock := nextL2Time >= l1Origin.Time+d.Config.MaxSequencerDrift
-
-	// Put together our payload attributes.
-	attrs := &eth.PayloadAttributes{
-		Timestamp:             hexutil.Uint64(nextL2Time),
-		PrevRandao:            eth.Bytes32(l1Info.MixDigest()),
-		SuggestedFeeRecipient: d.Config.FeeRecipientAddress,
-		Transactions:          txns,
-		NoTxPool:              shouldProduceEmptyBlock,
-	}
+	attrs.NoTxPool = uint64(attrs.Timestamp) >= l1Origin.Time+d.Config.MaxSequencerDrift
 
 	// And construct our fork choice state. This is our current fork choice state and will be
 	// updated as a result of executing the block based on the attributes described above.

op-node/testutils/mock_l1.go

@@ -13,6 +13,15 @@ type MockL1Source struct {
 	mock.Mock
 }
 
+func (m *MockL1Source) InfoByHash(ctx context.Context, hash common.Hash) (eth.L1Info, error) {
+	out := m.Mock.MethodCalled("InfoByHash", hash)
+	return *out[0].(*eth.L1Info), *out[1].(*error)
+}
+
+func (m *MockL1Source) ExpectInfoByHash(hash common.Hash, info eth.L1Info, err error) {
+	m.Mock.On("InfoByHash", hash).Once().Return(&info, &err)
+}
+
 func (m *MockL1Source) L1BlockRefByNumber(ctx context.Context, u uint64) (eth.L1BlockRef, error) {
 	out := m.Mock.MethodCalled("L1BlockRefByNumber", u)
 	return out[0].(eth.L1BlockRef), *out[1].(*error)
@@ -33,11 +42,11 @@ func (m *MockL1Source) ExpectL1BlockRefByHash(hash common.Hash, ref eth.L1BlockR
 
 func (m *MockL1Source) Fetch(ctx context.Context, blockHash common.Hash) (eth.L1Info, types.Transactions, types.Receipts, error) {
 	out := m.Mock.MethodCalled("Fetch", blockHash)
-	return out[0].(eth.L1Info), out[1].(types.Transactions), out[2].(types.Receipts), *out[3].(*error)
+	return *out[0].(*eth.L1Info), out[1].(types.Transactions), out[2].(types.Receipts), *out[3].(*error)
 }
 
 func (m *MockL1Source) ExpectFetch(hash common.Hash, info eth.L1Info, transactions types.Transactions, receipts types.Receipts, err error) {
-	m.Mock.On("Fetch", hash).Once().Return(info, transactions, receipts, &err)
+	m.Mock.On("Fetch", hash).Once().Return(&info, transactions, receipts, &err)
 }
 
 func (m *MockL1Source) InfoAndTxsByHash(ctx context.Context, hash common.Hash) (eth.L1Info, types.Transactions, error) {

op-proposer/rollupclient/rollupclient.go

@@ -5,6 +5,7 @@ import (
 	"math/big"
 
 	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/driver"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/rpc"
@@ -30,6 +31,12 @@ func (r *RollupClient) SyncStatus(ctx context.Context) (*driver.SyncStatus, erro
 	return output, err
 }
 
+func (r *RollupClient) RollupConfig(ctx context.Context) (*rollup.Config, error) {
+	var output *rollup.Config
+	err := r.rpc.CallContext(ctx, &output, "optimism_rollupConfig")
+	return output, err
+}
+
 func (r *RollupClient) Version(ctx context.Context) (string, error) {
 	var output string
 	err := r.rpc.CallContext(ctx, &output, "optimism_version")

ops-bedrock/devnet-up.sh

@@ -60,7 +60,7 @@ mkdir -p ./.devnet
 if [ ! -f ./.devnet/rollup.json ]; then
     GENESIS_TIMESTAMP=$(date +%s | xargs printf "0x%x")
 else
-    GENESIS_TIMESTAMP=$(jq '.genesis.l2_time' < .devnet/rollup.json)
+    GENESIS_TIMESTAMP=$(jq '.timestamp' < .devnet/genesis-l1.json)
 fi
 
 # Regenerate the L1 genesis file if necessary. The existence of the genesis

ops/docker/ci-builder/check-changed.sh

@@ -9,7 +9,13 @@
 echoerr() { echo "$@" 1>&2; }
 
 # Check if this is a CircleCI PR.
-if [[ -n $CIRCLE_PULL_REQUEST ]]; then
+if [[ -z ${CIRCLE_PULL_REQUEST+x} ]]; then
+	# CIRCLE_PULL_REQUEST is unbound here
+	# Non-PR builds always require a rebuild.
+	echoerr "Not a PR build, requiring a total rebuild."
+	echo "TRUE"
+else
+	# CIRCLE_PULL_REQUEST is bound here
 	PACKAGE=$1
 	# Craft the URL to the GitHub API. The access token is optional for the monorepo since it's an open-source repo.
 	GITHUB_API_URL="https://api.github.com/repos/ethereum-optimism/optimism/pulls/${CIRCLE_PULL_REQUEST/https:\/\/github.com\/ethereum-optimism\/optimism\/pull\//}"
@@ -30,8 +36,4 @@ if [[ -n $CIRCLE_PULL_REQUEST ]]; then
  	# Compare HEAD to the PR's base ref, stripping out the change percentages that come with git diff --dirstat.
  	# Pass in the diff pattern to grep, and echo TRUE if there's a match. False otherwise.
  	(echo "$DIFF" | sed 's/^[ 0-9.]\+% //g' | grep -q -E "$PACKAGE" && echo "TRUE") || echo "FALSE"
-else
-	# Non-PR builds always require a rebuild.
-	echoerr "Not a PR build, requiring a total rebuild."
-	echo "TRUE"
 fi

ops/docker/foundry/Dockerfile

@@ -9,7 +9,7 @@ WORKDIR /opt/foundry
 # Only diff from upstream docker image is this clone instead
 # of COPY. We select a specific commit to use.
 RUN git clone https://github.com/foundry-rs/foundry.git . \
-    && git checkout b7b1ec471bdd38221773e1a569dc4f20297bd7db
+    && git checkout 3c49efe58ca4bdeec4729490501da06914446405
 
 RUN source $HOME/.profile && cargo build --release \
     && strip /opt/foundry/target/release/forge \

ops/docker/go-builder/CHANGELOG.md

-# @eth-optimism/go-builder
-
-## 0.0.5
-
-### Patch Changes
-
-- df5eb9e7: Upgrade golangci-lint version for go 1.18
-
-## 0.0.4
-
-### Patch Changes
-
-- 7317e2be: Bump versions
-- 10e07754: Add abigen and gotestsum to go-builder
-
-## 0.0.3
-
-### Patch Changes
-
-- ec446248: Trigger releases
-
-## 0.0.2
-
-### Patch Changes
-
-- fa978257: Trigger releases
-
-## 0.0.1
-
-### Patch Changes
-
-- 24bf252e: Add to changesets

ops/docker/go-builder/Dockerfile

-FROM ethereum/client-go:alltools-v1.10.17 as geth
-
-FROM golang:1.18.0-alpine3.15
-
-COPY --from=geth /usr/local/bin/abigen /usr/local/bin/abigen
-
-RUN apk add --no-cache make gcc musl-dev linux-headers git jq curl bash gzip ca-certificates openssh && \
-	go install gotest.tools/gotestsum@latest && \
-	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.46.2
-
-CMD ["bash"]

ops/docker/go-builder/package.json

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

ops/docker/js-builder/CHANGELOG.md

-# @eth-optimism/js-builder
-
-## 0.0.5
-
-### Patch Changes
-
-- 8a8efd51: Cache Solc 0.8.9 and 0.8.10 for use in CI
-- 8dc50095: Fix broken build
-
-## 0.0.4
-
-### Patch Changes
-
-- 7317e2be: Bump versions
-- 10e07754: Add abigen
-
-## 0.0.3
-
-### Patch Changes
-
-- ec446248: Trigger releases
-
-## 0.0.2
-
-### Patch Changes
-
-- fa978257: Trigger releases
-
-## 0.0.1
-
-### Patch Changes
-
-- 24bf252e: Add to changesets

ops/docker/js-builder/Dockerfile

-FROM ethereum/client-go:alltools-v1.10.17 as geth
-
-FROM ghcr.io/foundry-rs/foundry:latest as foundry
-
-FROM python:3.8.12-slim-buster
-
-COPY --from=foundry /usr/local/bin/forge /usr/local/bin/forge
-COPY --from=foundry /usr/local/bin/cast /usr/local/bin/cast
-COPY --from=geth /usr/local/bin/abigen /usr/local/bin/abigen
-
-RUN apt-get update && \
-	apt-get install -y curl openssh-client git build-essential ca-certificates musl && \
-	curl -sL https://deb.nodesource.com/setup_16.x -o nodesource_setup.sh && \
-	curl -sL https://go.dev/dl/go1.18.2.linux-amd64.tar.gz -o go1.18.2.linux-amd64.tar.gz && \
-	tar -C /usr/local/ -xzvf go1.18.2.linux-amd64.tar.gz && \
-	ln -s /usr/local/go/bin/gofmt /usr/local/bin/gofmt && \
-	bash nodesource_setup.sh && \
-	apt-get install -y nodejs && \
-	npm i -g yarn && \
-	npm i -g depcheck && \
-	pip install slither-analyzer
-
-RUN echo "downloading solidity compilers" && \
-  curl -o solc-linux-amd64-v0.5.17+commit.d19bba13 -sL https://binaries.soliditylang.org/linux-amd64/solc-linux-amd64-v0.5.17+commit.d19bba13 && \
-  curl -o solc-linux-amd64-v0.8.9+commit.e5eed63a -sL https://binaries.soliditylang.org/linux-amd64/solc-linux-amd64-v0.8.9+commit.e5eed63a  && \
-  curl -o solc-linux-amd64-v0.8.10+commit.fc410830 -sL https://binaries.soliditylang.org/linux-amd64/solc-linux-amd64-v0.8.10+commit.fc410830 && \
-  curl -o solc-linux-amd64-v0.8.12+commit.f00d7308 -sL https://binaries.soliditylang.org/linux-amd64/solc-linux-amd64-v0.8.12+commit.f00d7308 && \
-  echo "verifying checksums" && \
-  (echo "c35ce7a4d3ffa5747c178b1e24c8541b2e5d8a82c1db3719eb4433a1f19e16f3 solc-linux-amd64-v0.5.17+commit.d19bba13" | sha256sum --check --status - || exit 1) && \
-  (echo "f851f11fad37496baabaf8d6cb5c057ca0d9754fddb7a351ab580d7fd728cb94 solc-linux-amd64-v0.8.9+commit.e5eed63a"  | sha256sum --check --status - || exit 1) && \
-  (echo "c7effacf28b9d64495f81b75228fbf4266ac0ec87e8f1adc489ddd8a4dd06d89 solc-linux-amd64-v0.8.10+commit.fc410830" | sha256sum --check --status - || exit 1) && \
-  (echo "556c3ec44faf8ff6b67933fa8a8a403abe82c978d6e581dbfec4bd07360bfbf3 solc-linux-amd64-v0.8.12+commit.f00d7308" | sha256sum --check --status - || exit 1) && \
-  echo "caching compilers" && \
-  mkdir -p ~/.cache/hardhat-nodejs/compilers/linux-amd64 && \
-  cp solc-linux-amd64-v0.5.17+commit.d19bba13 ~/.cache/hardhat-nodejs/compilers/linux-amd64/ && \
-  cp solc-linux-amd64-v0.8.9+commit.e5eed63a  ~/.cache/hardhat-nodejs/compilers/linux-amd64/ && \
-  cp solc-linux-amd64-v0.8.10+commit.fc410830 ~/.cache/hardhat-nodejs/compilers/linux-amd64/ && \
-  cp solc-linux-amd64-v0.8.12+commit.f00d7308 ~/.cache/hardhat-nodejs/compilers/linux-amd64/ && \
-  mkdir -p ~/.svm/0.5.17 && \
-  cp solc-linux-amd64-v0.5.17+commit.d19bba13 ~/.svm/0.5.17/solc-0.5.17 && \
-  mkdir -p ~/.svm/0.8.9  && \
-  cp solc-linux-amd64-v0.8.9+commit.e5eed63a  ~/.svm/0.8.9/solc-0.8.9   && \
-  mkdir -p ~/.svm/0.8.10 && \
-  cp solc-linux-amd64-v0.8.10+commit.fc410830 ~/.svm/0.8.10/solc-0.8.10 && \
-  mkdir -p ~/.svm/0.8.12 && \
-  cp solc-linux-amd64-v0.8.12+commit.f00d7308 ~/.svm/0.8.12/solc-0.8.12

ops/docker/js-builder/package.json

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

packages/contracts-bedrock/.solhint.json

@@ -10,6 +10,9 @@
     "modifier-name-mixedcase": "error",
     "ordering": "warn",
     "avoid-low-level-calls": "off",
+    "no-inline-assembly": "off",
+    "no-empty-blocks": "off",
+    "not-rely-on-time": "off",
     "event-name-camelcase": "off",
     "reason-string": "off",
     "avoid-tx-origin": "off",

packages/contracts-bedrock/contracts/L1/L1CrossDomainMessenger.sol

@@ -46,7 +46,7 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
      *
      * @return True if the message was sent from the messenger, false otherwise.
      */
-    function _isSystemMessageSender() internal view override returns (bool) {
+    function _isOtherMessenger() internal view override returns (bool) {
         return msg.sender == address(portal) && portal.l2Sender() == otherMessenger;
     }
 

packages/contracts-bedrock/contracts/L1/L1StandardBridge.sol

@@ -86,6 +86,8 @@ contract L1StandardBridge is StandardBridge, Semver {
     );
 
     /**
+     * @custom:semver 0.0.1
+     *
      * @param _messenger Address of the L1CrossDomainMessenger.
      */
     constructor(address payable _messenger) Semver(0, 0, 1) {

packages/contracts-bedrock/contracts/L1/L2OutputOracle.sol

@@ -63,26 +63,31 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
     /**
      * @notice The interval in L2 blocks at which checkpoints must be submitted.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable SUBMISSION_INTERVAL;
 
     /**
      * @notice The number of blocks in the chain before the first block in this contract.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable HISTORICAL_TOTAL_BLOCKS;
 
     /**
      * @notice The number of the first L2 block recorded in this contract.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable STARTING_BLOCK_NUMBER;
 
     /**
      * @notice The timestamp of the first L2 block recorded in this contract.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable STARTING_TIMESTAMP;
 
     /**
      * @notice The time between L2 blocks in seconds.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable L2_BLOCK_TIME;
 
     /**

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -56,11 +56,13 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     /**
      * @notice Minimum time (in seconds) that must elapse before a withdrawal can be finalized.
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable FINALIZATION_PERIOD_SECONDS;
 
     /**
      * @notice Address of the L2OutputOracle.
      */
+    // solhint-disable-next-line var-name-mixedcase
     L2OutputOracle public immutable L2_ORACLE;
 
     /**
@@ -99,12 +101,11 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     constructor(L2OutputOracle _l2Oracle, uint256 _finalizationPeriodSeconds) Semver(0, 0, 1) {
         L2_ORACLE = _l2Oracle;
         FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
-
         initialize();
     }
 
     /**
-     * @notice Intializes mutable variables.
+     * @notice Initializer;
      */
     function initialize() public initializer {
         l2Sender = DEFAULT_L2_SENDER;
@@ -142,7 +143,6 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     ) public payable metered(_gasLimit) {
         // Just to be safe, make sure that people specify address(0) as the target when doing
         // contract creations.
-        // TODO: Do we really need this? Prevents some user error, but adds gas.
         if (_isCreation) {
             require(
                 _to == address(0),

packages/contracts-bedrock/contracts/L1/ResourceMetering.sol

 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.10;
 
+import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
 import { SignedMath } from "@openzeppelin/contracts/utils/math/SignedMath.sol";
 import { FixedPointMathLib } from "@rari-capital/solmate/src/utils/FixedPointMathLib.sol";
@@ -11,7 +12,7 @@ import { Burn } from "../libraries/Burn.sol";
  * @notice ResourceMetering implements an EIP-1559 style resource metering system where pricing
  *         updates automatically based on current demand.
  */
-contract ResourceMetering {
+abstract contract ResourceMetering is Initializable {
     /**
      * @notice Represents the various parameters that control the way in which resources are
      *         metered. Corresponds to the EIP-1559 resource metering system.
@@ -62,19 +63,12 @@ contract ResourceMetering {
      */
     uint256[49] private __gap;
 
-    /**
-     * @notice Set the initial values. In order to enable this contract to be used in an upgradable
-     *         context, the constructor calls a separate init function.
-     */
-    constructor() {
-        __ResourceMetering_init();
-    }
-
     /**
      * @notice Sets initial resource parameter values. This function must either be called by the
      *         initializer function of an upgradeable child contract.
      */
-    function __ResourceMetering_init() internal {
+    // solhint-disable-next-line func-name-mixedcase
+    function __ResourceMetering_init() internal onlyInitializing {
         params = ResourceParams({
             prevBaseFee: INITIAL_BASE_FEE,
             prevBoughtGas: 0,

packages/contracts-bedrock/contracts/L2/L1BlockNumber.sol

@@ -35,6 +35,7 @@ contract L1BlockNumber is Semver {
     /**
      * @notice Returns the L1 block number.
      */
+    // solhint-disable-next-line no-complex-fallback
     fallback() external payable {
         uint256 l1BlockNumber = getL1BlockNumber();
         assembly {

packages/contracts-bedrock/contracts/L2/L2CrossDomainMessenger.sol

@@ -52,7 +52,7 @@ contract L2CrossDomainMessenger is CrossDomainMessenger, Semver {
      *
      * @return True if the message sender is the L1CrossDomainMessenger on L1.
      */
-    function _isSystemMessageSender() internal view override returns (bool) {
+    function _isOtherMessenger() internal view override returns (bool) {
         return AddressAliasHelper.undoL1ToL2Alias(msg.sender) == otherMessenger;
     }
 

packages/contracts-bedrock/contracts/L2/L2StandardBridge.sol

@@ -16,7 +16,6 @@ import { OptimismMintableERC20 } from "../universal/OptimismMintableERC20.sol";
  *         Examples of some token types that may not be properly supported by this contract include,
  *         but are not limited to: tokens with transfer fees, rebasing tokens, and
  *         tokens with blocklists.
- *         TODO: ensure that this has 1:1 backwards compatibility
  */
 contract L2StandardBridge is StandardBridge, Semver {
     /**

packages/contracts-bedrock/contracts/legacy/L1ChugSplashProxy.sol

@@ -2,9 +2,9 @@
 pragma solidity ^0.8.9;
 
 /**
- * @title iL1ChugSplashDeployer
+ * @title IL1ChugSplashDeployer
  */
-interface iL1ChugSplashDeployer {
+interface IL1ChugSplashDeployer {
     function isUpgrading() external view returns (bool);
 }
 
@@ -58,7 +58,7 @@ contract L1ChugSplashProxy {
         // L1ChugSplashDeployer contract and Solidity will throw errors if we do a normal call and
         // it turns out that it isn't the right type of contract.
         (bool success, bytes memory returndata) = owner.staticcall(
-            abi.encodeWithSelector(iL1ChugSplashDeployer.isUpgrading.selector)
+            abi.encodeWithSelector(IL1ChugSplashDeployer.isUpgrading.selector)
         );
 
         // If the call was unsuccessful then we assume that there's no "isUpgrading" method and we

packages/contracts-bedrock/contracts/legacy/LegacyERC20ETH.sol

@@ -23,87 +23,62 @@ contract LegacyERC20ETH is OptimismMintableERC20 {
     {}
 
     /**
+     * @custom:blocked
      * @notice Mints some amount of ETH.
-     *
-     * @param _to     Address of the recipient.
-     * @param _amount Amount of ETH to mint.
      */
-    function mint(address _to, uint256 _amount) public virtual override {
+    function mint(address, uint256) public virtual override {
         revert("LegacyERC20ETH: mint is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Burns some amount of ETH.
-     *
-     * @param _from   Address to burn from.
-     * @param _amount Amount of ETH to burn.
      */
-    function burn(address _from, uint256 _amount) public virtual override {
+    function burn(address, uint256) public virtual override {
         revert("LegacyERC20ETH: burn is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Transfers some amount of ETH.
-     *
-     * @param _recipient Address to send to.
-     * @param _amount    Amount of ETH to send.
      */
-    function transfer(address _recipient, uint256 _amount) public virtual override returns (bool) {
+    function transfer(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: transfer is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Approves a spender to spend some amount of ETH.
-     *
-     * @param _spender Address of the spender.
-     * @param _amount  Amount of ETH to approve.
      */
-    function approve(address _spender, uint256 _amount) public virtual override returns (bool) {
+    function approve(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: approve is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Transfers funds from some sender account.
-     *
-     * @param _sender    Address of the sender.
-     * @param _recipient Address of the recipient.
-     * @param _amount    Amount of ETH to transfer.
      */
     function transferFrom(
-        address _sender,
-        address _recipient,
-        uint256 _amount
+        address,
+        address,
+        uint256
     ) public virtual override returns (bool) {
         revert("LegacyERC20ETH: transferFrom is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Increases the allowance of a spender.
-     *
-     * @param _spender    Address of the spender.
-     * @param _addedValue Amount of ETH to increase the allowance by.
      */
-    function increaseAllowance(address _spender, uint256 _addedValue)
-        public
-        virtual
-        override
-        returns (bool)
-    {
+    function increaseAllowance(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: increaseAllowance is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Decreases the allowance of a spender.
-     *
-     * @param _spender         Address of the spender.
-     * @param _subtractedValue Amount of ETH to decrease the allowance by.
      */
-    function decreaseAllowance(address _spender, uint256 _subtractedValue)
-        public
-        virtual
-        override
-        returns (bool)
-    {
+    function decreaseAllowance(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: decreaseAllowance is disabled");
     }
 }

packages/contracts-bedrock/contracts/legacy/ResolvedDelegateProxy.sol

@@ -40,6 +40,7 @@ contract ResolvedDelegateProxy {
     /**
      * @notice Fallback, performs a delegatecall to the resolved implementation address.
      */
+    // solhint-disable-next-line no-complex-fallback
     fallback() external payable {
         address target = addressManager[address(this)].getAddress(
             (implementationName[address(this)])

packages/contracts-bedrock/contracts/libraries/BytesUtils.sol

@@ -3,20 +3,31 @@ pragma solidity ^0.8.9;
 
 /**
  * @title BytesUtils
+ * @notice BytesUtils is a library for manipulating byte arrays.
  */
 library BytesUtils {
-    /**********************
-     * Internal Functions *
-     **********************/
-
+    /**
+     * @custom:attribution https://github.com/GNSPS/solidity-bytes-utils
+     * @notice Slices a byte array with a given starting index and length. Returns a new byte array
+     *         as opposed to a pointer to the original array. Will throw if trying to slice more
+     *         bytes than exist in the array.
+     *
+     * @param _bytes Byte array to slice.
+     * @param _start Starting index of the slice.
+     * @param _length Length of the slice.
+     *
+     * @return Slice of the input byte array.
+     */
     function slice(
         bytes memory _bytes,
         uint256 _start,
         uint256 _length
     ) internal pure returns (bytes memory) {
+        unchecked {
             require(_length + 31 >= _length, "slice_overflow");
             require(_start + _length >= _start, "slice_overflow");
             require(_bytes.length >= _start + _length, "slice_outOfBounds");
+        }
 
         bytes memory tempBytes;
 
@@ -76,51 +87,63 @@ library BytesUtils {
         return tempBytes;
     }
 
+    /**
+     * @notice Slices a byte array with a given starting index up to the end of the original byte
+     *         array. Returns a new array rathern than a pointer to the original.
+     *
+     * @param _bytes Byte array to slice.
+     * @param _start Starting index of the slice.
+     *
+     * @return Slice of the input byte array.
+     */
     function slice(bytes memory _bytes, uint256 _start) internal pure returns (bytes memory) {
         if (_start >= _bytes.length) {
             return bytes("");
         }
-
         return slice(_bytes, _start, _bytes.length - _start);
     }
 
-    function toBytes32(bytes memory _bytes) internal pure returns (bytes32) {
-        if (_bytes.length < 32) {
-            bytes32 ret;
-            assembly {
-                ret := mload(add(_bytes, 32))
-            }
-            return ret;
-        }
-
-        return abi.decode(_bytes, (bytes32)); // will truncate if input length > 32 bytes
-    }
-
-    function toUint256(bytes memory _bytes) internal pure returns (uint256) {
-        return uint256(toBytes32(_bytes));
-    }
-
+    /**
+     * @notice Converts a byte array into a nibble array by splitting each byte into two nibbles.
+     *         Resulting nibble array will be exactly twice as long as the input byte array.
+     *
+     * @param _bytes Input byte array to convert.
+     *
+     * @return Resulting nibble array.
+     */
     function toNibbles(bytes memory _bytes) internal pure returns (bytes memory) {
         bytes memory nibbles = new bytes(_bytes.length * 2);
-
         for (uint256 i = 0; i < _bytes.length; i++) {
             nibbles[i * 2] = _bytes[i] >> 4;
             nibbles[i * 2 + 1] = bytes1(uint8(_bytes[i]) % 16);
         }
-
         return nibbles;
     }
 
+    /**
+     * @notice Generates a byte array from a nibble array by joining each set of two nibbles into a
+     *         single byte. Resulting byte array will be half as long as the input byte array.
+     *
+     * @param _bytes Input nibble array to convert.
+     *
+     * @return Resulting byte array.
+     */
     function fromNibbles(bytes memory _bytes) internal pure returns (bytes memory) {
         bytes memory ret = new bytes(_bytes.length / 2);
-
         for (uint256 i = 0; i < ret.length; i++) {
             ret[i] = (_bytes[i * 2] << 4) | (_bytes[i * 2 + 1]);
         }
-
         return ret;
     }
 
+    /**
+     * @notice Compares two byte arrays by comparing their keccak256 hashes.
+     *
+     * @param _bytes First byte array to compare.
+     * @param _other Second byte array to compare.
+     *
+     * @return True if the two byte arrays are equal, false otherwise.
+     */
     function equal(bytes memory _bytes, bytes memory _other) internal pure returns (bool) {
         return keccak256(_bytes) == keccak256(_other);
     }

packages/contracts-bedrock/contracts/libraries/trie/MerkleTrie.sol

 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.9;
 
-/* Library Imports */
 import { BytesUtils } from "../BytesUtils.sol";
 import { RLPReader } from "../rlp/RLPReader.sol";
 import { RLPWriter } from "../rlp/RLPWriter.sol";
 
 /**
  * @title MerkleTrie
+ * @notice MerkleTrie is a small library for verifying standard Ethereum Merkle-Patricia trie
+ *         inclusion proofs. By default, this library assumes a hexary trie. One can change the
+ *         trie radix constant to support other trie radixes.
  */
 library MerkleTrie {
-    /*******************
-     * Data Structures *
-     *******************/
-
+    /**
+     * @notice Struct representing a node in the trie.
+     */
     struct TrieNode {
         bytes encoded;
         RLPReader.RLPItem[] decoded;
     }
 
-    /**********************
-     * Contract Constants *
-     **********************/
-
-    // TREE_RADIX determines the number of elements per branch node.
+    /**
+     * @notice Determines the number of elements per branch node.
+     */
     uint256 constant TREE_RADIX = 16;
-    // Branch nodes have TREE_RADIX elements plus an additional `value` slot.
+
+    /**
+     * @notice Branch nodes have TREE_RADIX elements and one value element.
+     */
     uint256 constant BRANCH_NODE_LENGTH = TREE_RADIX + 1;
-    // Leaf nodes and extension nodes always have two elements, a `path` and a `value`.
+
+    /**
+     * @notice Leaf nodes and extension nodes have two elements, a `path` and a `value`.
+     */
     uint256 constant LEAF_OR_EXTENSION_NODE_LENGTH = 2;
 
-    // Prefixes are prepended to the `path` within a leaf or extension node and
-    // allow us to differentiate between the two node types. `ODD` or `EVEN` is
-    // determined by the number of nibbles within the unprefixed `path`. If the
-    // number of nibbles if even, we need to insert an extra padding nibble so
-    // the resulting prefixed `path` has an even number of nibbles.
+    /**
+     * @notice Prefix for even-nibbled extension node paths.
+     */
     uint8 constant PREFIX_EXTENSION_EVEN = 0;
+
+    /**
+     * @notice Prefix for odd-nibbled extension node paths.
+     */
     uint8 constant PREFIX_EXTENSION_ODD = 1;
+
+    /**
+     * @notice Prefix for even-nibbled leaf node paths.
+     */
     uint8 constant PREFIX_LEAF_EVEN = 2;
+
+    /**
+     * @notice Prefix for odd-nibbled leaf node paths.
+     */
     uint8 constant PREFIX_LEAF_ODD = 3;
 
-    // Just a utility constant. RLP represents `NULL` as 0x80.
+    /**
+     * @notice RLP representation of `NULL`.
+     */
     bytes1 constant RLP_NULL = bytes1(0x80);
 
-    /**********************
-     * Internal Functions *
-     **********************/
-
     /**
-     * @notice Verifies a proof that a given key/value pair is present in the
-     * Merkle trie.
+     * @notice Verifies a proof that a given key/value pair is present in the trie.
+     *
      * @param _key   Key of the node to search for, as a hex string.
      * @param _value Value of the node to search for, as a hex string.
-     * @param _proof Merkle trie inclusion proof for the desired node. Unlike
-     * traditional Merkle trees, this proof is executed top-down and consists
-     * of a list of RLP-encoded nodes that make a path down to the target node.
-     * @param _root Known root of the Merkle trie. Used to verify that the
-     * included proof is correctly constructed.
-     * @return _verified `true` if the k/v pair exists in the trie, `false` otherwise.
+     * @param _proof Merkle trie inclusion proof for the desired node. Unlike traditional Merkle
+     *               trees, this proof is executed top-down and consists of a list of RLP-encoded
+     *               nodes that make a path down to the target node.
+     * @param _root  Known root of the Merkle trie. Used to verify that the included proof is
+     *               correctly constructed.
+     *
+     * @return Whether or not the proof is valid.
      */
     function verifyInclusionProof(
         bytes memory _key,
         bytes memory _value,
         bytes memory _proof,
         bytes32 _root
-    ) internal pure returns (bool _verified) {
+    ) internal pure returns (bool) {
         (bool exists, bytes memory value) = get(_key, _proof, _root);
-
         return (exists && BytesUtils.equal(_value, value));
     }
 
     /**
      * @notice Retrieves the value associated with a given key.
+     *
      * @param _key   Key to search for, as hex bytes.
      * @param _proof Merkle trie inclusion proof for the key.
      * @param _root  Known root of the Merkle trie.
-     * @return _exists Whether or not the key exists.
-     * @return _value Value of the key if it exists.
+     *
+     * @return Whether or not the key exists.
+     * @return Value of the key if it exists.
      */
     function get(
         bytes memory _key,
         bytes memory _proof,
         bytes32 _root
-    ) internal pure returns (bool _exists, bytes memory _value) {
+    ) internal pure returns (bool, bytes memory) {
         TrieNode[] memory proof = _parseProof(_proof);
         (uint256 pathLength, bytes memory keyRemainder, bool isFinalNode) = _walkNodePath(
             proof,
@@ -99,18 +114,16 @@ library MerkleTrie {
         return (exists, value);
     }
 
-    /*********************
-     * Private Functions *
-     *********************/
-
     /**
      * @notice Walks through a proof using a provided key.
+     *
      * @param _proof Inclusion proof to walk through.
      * @param _key   Key to use for the walk.
      * @param _root  Known root of the trie.
-     * @return _pathLength Length of the final path
-     * @return _keyRemainder Portion of the key remaining after the walk.
-     * @return _isFinalNode Whether or not we've hit a dead end.
+     *
+     * @return Length of the final path
+     * @return Portion of the key remaining after the walk.
+     * @return Whether or not we've hit a dead end.
      */
     function _walkNodePath(
         TrieNode[] memory _proof,
@@ -120,9 +133,9 @@ library MerkleTrie {
         private
         pure
         returns (
-            uint256 _pathLength,
-            bytes memory _keyRemainder,
-            bool _isFinalNode
+            uint256,
+            bytes memory,
+            bool
         )
     {
         uint256 pathLength = 0;
@@ -154,7 +167,7 @@ library MerkleTrie {
             } else {
                 // Nodes smaller than 31 bytes aren't hashed.
                 require(
-                    BytesUtils.toBytes32(currentNode.encoded) == currentNodeID,
+                    bytes32(currentNode.encoded) == currentNodeID,
                     "Invalid internal node hash"
                 );
             }
@@ -223,10 +236,12 @@ library MerkleTrie {
 
     /**
      * @notice Parses an RLP-encoded proof into something more useful.
+     *
      * @param _proof RLP-encoded proof to parse.
-     * @return _parsed Proof parsed into easily accessible structs.
+     *
+     * @return Proof parsed into easily accessible structs.
      */
-    function _parseProof(bytes memory _proof) private pure returns (TrieNode[] memory _parsed) {
+    function _parseProof(bytes memory _proof) private pure returns (TrieNode[] memory) {
         RLPReader.RLPItem[] memory nodes = RLPReader.readList(_proof);
         TrieNode[] memory proof = new TrieNode[](nodes.length);
 
@@ -239,13 +254,14 @@ library MerkleTrie {
     }
 
     /**
-     * @notice Picks out the ID for a node. Node ID is referred to as the
-     * "hash" within the specification, but nodes < 32 bytes are not actually
-     * hashed.
+     * @notice Picks out the ID for a node. Node ID is referred to as the "hash" within the
+     *         specification, but nodes < 32 bytes are not actually hashed.
+     *
      * @param _node Node to pull an ID for.
-     * @return _nodeID ID for the node, depending on the size of its contents.
+     *
+     * @return ID for the node, depending on the size of its contents.
      */
-    function _getNodeID(RLPReader.RLPItem memory _node) private pure returns (bytes32 _nodeID) {
+    function _getNodeID(RLPReader.RLPItem memory _node) private pure returns (bytes32) {
         bytes memory nodeID;
 
         if (_node.length < 32) {
@@ -256,38 +272,43 @@ library MerkleTrie {
             nodeID = RLPReader.readBytes(_node);
         }
 
-        return BytesUtils.toBytes32(nodeID);
+        return bytes32(nodeID);
     }
 
     /**
      * @notice Gets the path for a leaf or extension node.
+     *
      * @param _node Node to get a path for.
-     * @return _path Node path, converted to an array of nibbles.
+     *
+     * @return Node path, converted to an array of nibbles.
      */
-    function _getNodePath(TrieNode memory _node) private pure returns (bytes memory _path) {
+    function _getNodePath(TrieNode memory _node) private pure returns (bytes memory) {
         return BytesUtils.toNibbles(RLPReader.readBytes(_node.decoded[0]));
     }
 
     /**
      * @notice Gets the path for a node.
+     *
      * @param _node Node to get a value for.
-     * @return _value Node value, as hex bytes.
+     *
+     * @return Node value, as hex bytes.
      */
-    function _getNodeValue(TrieNode memory _node) private pure returns (bytes memory _value) {
+    function _getNodeValue(TrieNode memory _node) private pure returns (bytes memory) {
         return RLPReader.readBytes(_node.decoded[_node.decoded.length - 1]);
     }
 
     /**
-     * @notice Utility; determines the number of nibbles shared between two
-     * nibble arrays.
+     * @notice Utility; determines the number of nibbles shared between two nibble arrays.
+     *
      * @param _a First nibble array.
      * @param _b Second nibble array.
-     * @return _shared Number of shared nibbles.
+     *
+     * @return Number of shared nibbles.
      */
     function _getSharedNibbleLength(bytes memory _a, bytes memory _b)
         private
         pure
-        returns (uint256 _shared)
+        returns (uint256)
     {
         uint256 i = 0;
         while (_a.length > i && _b.length > i && _a[i] == _b[i]) {

packages/contracts-bedrock/contracts/libraries/trie/SecureMerkleTrie.sol

@@ -6,61 +6,60 @@ import { MerkleTrie } from "./MerkleTrie.sol";
 
 /**
  * @title SecureMerkleTrie
+ * @notice SecureMerkleTrie is a thin wrapper around the MerkleTrie library that hashes the input
+ *         keys. Ethereum's state trie hashes input keys before storing them.
  */
 library SecureMerkleTrie {
-    /**********************
-     * Internal Functions *
-     **********************/
-
     /**
-     * @notice Verifies a proof that a given key/value pair is present in the
-     * Merkle trie.
+     * @notice Verifies a proof that a given key/value pair is present in the Merkle trie.
+     *
      * @param _key   Key of the node to search for, as a hex string.
      * @param _value Value of the node to search for, as a hex string.
-     * @param _proof Merkle trie inclusion proof for the desired node. Unlike
-     * traditional Merkle trees, this proof is executed top-down and consists
-     * of a list of RLP-encoded nodes that make a path down to the target node.
-     * @param _root Known root of the Merkle trie. Used to verify that the
-     * included proof is correctly constructed.
-     * @return _verified `true` if the k/v pair exists in the trie, `false` otherwise.
+     * @param _proof Merkle trie inclusion proof for the desired node. Unlike traditional Merkle
+     *               trees, this proof is executed top-down and consists of a list of RLP-encoded
+     *               nodes that make a path down to the target node.
+     * @param _root  Known root of the Merkle trie. Used to verify that the included proof is
+     *               correctly constructed.
+     *
+     * @return Whether or not the proof is valid.
      */
     function verifyInclusionProof(
         bytes memory _key,
         bytes memory _value,
         bytes memory _proof,
         bytes32 _root
-    ) internal pure returns (bool _verified) {
+    ) internal pure returns (bool) {
         bytes memory key = _getSecureKey(_key);
         return MerkleTrie.verifyInclusionProof(key, _value, _proof, _root);
     }
 
     /**
      * @notice Retrieves the value associated with a given key.
+     *
      * @param _key   Key to search for, as hex bytes.
      * @param _proof Merkle trie inclusion proof for the key.
      * @param _root  Known root of the Merkle trie.
-     * @return _exists Whether or not the key exists.
-     * @return _value Value of the key if it exists.
+     *
+     * @return Whether or not the key exists.
+     * @return Value of the key if it exists.
      */
     function get(
         bytes memory _key,
         bytes memory _proof,
         bytes32 _root
-    ) internal pure returns (bool _exists, bytes memory _value) {
+    ) internal pure returns (bool, bytes memory) {
         bytes memory key = _getSecureKey(_key);
         return MerkleTrie.get(key, _proof, _root);
     }
 
-    /*********************
-     * Private Functions *
-     *********************/
-
     /**
-     * Computes the secure counterpart to a key.
-     * @param _key Key to get a secure key from.
-     * @return _secureKey Secure version of the key.
+     * @notice Computes the hashed version of the input key.
+     *
+     * @param _key Key to hash.
+     *
+     * @return Hashed version of the key.
      */
-    function _getSecureKey(bytes memory _key) private pure returns (bytes memory _secureKey) {
+    function _getSecureKey(bytes memory _key) private pure returns (bytes memory) {
         return abi.encodePacked(keccak256(_key));
     }
 }

packages/contracts-bedrock/contracts/test/CommonTest.t.sol

@@ -22,7 +22,7 @@ import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable
 import { ResolvedDelegateProxy } from "../legacy/ResolvedDelegateProxy.sol";
 import { AddressManager } from "../legacy/AddressManager.sol";
 import { L1ChugSplashProxy } from "../legacy/L1ChugSplashProxy.sol";
-import { iL1ChugSplashDeployer } from "../legacy/L1ChugSplashProxy.sol";
+import { IL1ChugSplashDeployer } from "../legacy/L1ChugSplashProxy.sol";
 
 contract CommonTest is Test {
     address alice = address(128);
@@ -341,7 +341,7 @@ contract Bridge_Initializer is Messenger_Initializer {
         L1ChugSplashProxy proxy = new L1ChugSplashProxy(multisig);
         vm.mockCall(
             multisig,
-            abi.encodeWithSelector(iL1ChugSplashDeployer.isUpgrading.selector),
+            abi.encodeWithSelector(IL1ChugSplashDeployer.isUpgrading.selector),
             abi.encode(true)
         );
         vm.startPrank(multisig);

packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol

@@ -56,11 +56,8 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
 
     // the version is encoded in the nonce
     function test_L1MessengerMessageVersion() external {
-        (,uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
-        assertEq(
-            version,
-            L1Messenger.MESSAGE_VERSION()
-        );
+        (, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
+        assertEq(version, L1Messenger.MESSAGE_VERSION());
     }
 
     // sendMessage: should be able to send a single message
@@ -74,7 +71,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
                 OptimismPortal.depositTransaction.selector,
                 PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
                 0,
-                100 + L1Messenger.baseGas(hex"ff"),
+                L1Messenger.baseGas(hex"ff", 100),
                 false,
                 Encoding.encodeCrossDomainMessage(
                     L1Messenger.messageNonce(),
@@ -94,7 +91,7 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
             PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
             0,
             0,
-            100 + L1Messenger.baseGas(hex"ff"),
+            L1Messenger.baseGas(hex"ff", 100),
             false,
             Encoding.encodeCrossDomainMessage(
                 L1Messenger.messageNonce(),
@@ -172,7 +169,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         address sender = PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
         bytes memory message = hex"1111";
 
-        // set the value of op.l2Sender() to be the L2 Cross Domain Messenger.
         vm.prank(address(op));
         vm.expectRevert("Message cannot be replayed.");
         L1Messenger.relayMessage(0, sender, target, 0, 0, message);
@@ -182,6 +178,18 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         L1Messenger.relayMessage(0, sender, target, 0, 0, message);
     }
 
+    // relayMessage: should revert if eth is sent from a contract other than the standard bridge
+    function test_L1MessengerReplayMessageWithValue() external {
+        address target = address(0xabcd);
+        address sender = PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
+        bytes memory message = hex"1111";
+
+        vm.expectRevert(
+            "CrossDomainMessenger: Value must be zero unless message is from a system address."
+        );
+        L1Messenger.relayMessage{ value: 100 }(0, sender, target, 0, 0, message);
+    }
+
     // relayMessage: the xDomainMessageSender is reset to the original value
     function test_L1MessengerxDomainMessageSenderResets() external {
         vm.expectRevert("xDomainMessageSender is not set");
@@ -190,7 +198,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         address sender = PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
 
         uint256 senderSlotIndex = 51;
-        bytes32 slotValue = vm.load(address(op), bytes32(senderSlotIndex));
 
         vm.store(address(op), bytes32(senderSlotIndex), bytes32(abi.encode(sender)));
         vm.prank(address(op));

packages/contracts-bedrock/contracts/test/L1StandardBridge.t.sol

@@ -59,7 +59,8 @@ contract L1StandardBridge_Test is Bridge_Initializer {
         );
 
         vm.prank(alice, alice);
-        address(L1Bridge).call{ value: 100 }(hex"");
+        (bool success,) = address(L1Bridge).call{ value: 100 }(hex"");
+        assertEq(success, true);
         assertEq(address(op).balance, 100);
     }
 

packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol

@@ -44,7 +44,7 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
             abi.encodeWithSelector(
                 L2ToL1MessagePasser.initiateWithdrawal.selector,
                 address(L1Messenger),
-                100 + L2Messenger.baseGas(hex"ff"),
+                L2Messenger.baseGas(hex"ff", 100),
                 Encoding.encodeCrossDomainMessage(
                     L2Messenger.messageNonce(),
                     alice,
@@ -63,7 +63,7 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
             address(L2Messenger),
             address(L1Messenger),
             0,
-            100 + L2Messenger.baseGas(hex"ff"),
+            L2Messenger.baseGas(hex"ff", 100),
             Encoding.encodeCrossDomainMessage(
                 L2Messenger.messageNonce(),
                 alice,

packages/contracts-bedrock/contracts/test/L2OutputOracle.t.sol

@@ -357,12 +357,22 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
 
     function test_cannotInitProxy() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(proxy).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+        L2OutputOracle(payable(proxy)).initialize(
+            genesisL2Output,
+            startingBlockNumber,
+            sequencer,
+            owner
+        );
     }
 
     function test_cannotInitImpl() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(oracleImpl).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+        L2OutputOracle(oracleImpl).initialize(
+            genesisL2Output,
+            startingBlockNumber,
+            sequencer,
+            owner
+        );
     }
 
     function test_upgrading() external {

packages/contracts-bedrock/contracts/test/L2StandardBridge.t.sol

@@ -40,7 +40,8 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         // TODO: events from each contract
 
         vm.prank(alice, alice);
-        address(L2Bridge).call{ value: 100 }(hex"");
+        (bool success,) = address(L2Bridge).call{ value: 100 }(hex"");
+        assertEq(success, true);
         assertEq(address(messagePasser).balance, 100);
     }
 

packages/contracts-bedrock/contracts/test/OptimismPortal.t.sol

@@ -334,12 +334,12 @@ contract OptimismPortalUpgradeable_Test is Portal_Initializer {
 
     function test_cannotInitProxy() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(proxy).call(abi.encodeWithSelector(OptimismPortal.initialize.selector));
+        OptimismPortal(payable(proxy)).initialize();
     }
 
     function test_cannotInitImpl() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(opImpl).call(abi.encodeWithSelector(OptimismPortal.initialize.selector));
+        OptimismPortal(opImpl).initialize();
     }
 
     function test_upgrading() external {

packages/contracts-bedrock/contracts/test/Proxy.t.sol

@@ -18,7 +18,7 @@ contract SimpleStorage {
 }
 
 contract Clasher {
-    function upgradeTo(address _implementation) external view {
+    function upgradeTo(address) external pure {
         revert("upgradeTo");
     }
 }
@@ -150,8 +150,8 @@ contract Proxy_Test is Test {
     function test_upgradeToAndCall() external {
         {
             // There should be nothing in the current proxy storage
-            uint256 result = SimpleStorage(address(proxy)).get(1);
-            assertEq(result, 0);
+            uint256 expect = SimpleStorage(address(proxy)).get(1);
+            assertEq(expect, 0);
         }
 
         // Deploy a new SimpleStorage

packages/contracts-bedrock/contracts/test/ResourceMetering.t.sol

@@ -7,6 +7,10 @@ import { Proxy } from "../universal/Proxy.sol";
 
 contract MeterUser is ResourceMetering {
     constructor() {
+        initialize();
+    }
+
+    function initialize() public initializer {
         __ResourceMetering_init();
     }
 
@@ -88,12 +92,12 @@ contract ResourceMetering_Test is CommonTest {
         uint64 elasticity = uint64(uint256(meter.ELASTICITY_MULTIPLIER()));
         meter.use(target * elasticity);
 
-        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = meter.params();
+        (, uint64 prevBoughtGas, ) = meter.params();
         assertEq(prevBoughtGas, target * elasticity);
 
         vm.roll(initialBlockNum + 1);
         meter.use(0);
-        (uint128 postBaseFee, uint64 postBoughtGas, uint64 postBlockNum) = meter.params();
+        (uint128 postBaseFee,,) = meter.params();
         // Base fee increases by 1/8 the difference
         assertEq(postBaseFee, 1375000000);
     }

packages/contracts-bedrock/contracts/test/SequencerFeeVault.t.sol

@@ -48,8 +48,9 @@ contract SequencerFeeVault_Test is Bridge_Initializer {
         );
 
         vm.prank(alice);
-        address(vault).call{ value: 100 }(hex"");
+        (bool success,) = address(vault).call{ value: 100 }(hex"");
 
+        assertEq(success, true);
         assertEq(
             address(vault).balance,
             100

packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol

@@ -64,14 +64,24 @@ abstract contract CrossDomainMessenger is
     uint16 public constant MESSAGE_VERSION = 1;
 
     /**
-     * @notice Dynamic overhead applied to the base gas for a message.
+     * @notice Constant overhead added to the base gas for a message.
      */
-    uint32 public constant MIN_GAS_DYNAMIC_OVERHEAD = 1;
+    uint32 public constant MIN_GAS_CONSTANT_OVERHEAD = 200_000;
 
     /**
-     * @notice Constant overhead added to the base gas for a message.
+     * @notice Numerator for dynamic overhead added to the base gas for a message.
+     */
+    uint32 public constant MIN_GAS_DYNAMIC_OVERHEAD_NUMERATOR = 1016;
+
+    /**
+     * @notice Denominator for dynamic overhead added to the base gas for a message.
      */
-    uint32 public constant MIN_GAS_CONSTANT_OVERHEAD = 100_000;
+    uint32 public constant MIN_GAS_DYNAMIC_OVERHEAD_DENOMINATOR = 1000;
+
+    /**
+     * @notice Extra gas added to base gas for each byte of calldata in a message.
+     */
+    uint32 public constant MIN_GAS_CALLDATA_OVERHEAD = 16;
 
     /**
      * @notice Minimum amount of gas required to relay a message.
@@ -182,14 +192,19 @@ abstract contract CrossDomainMessenger is
      *         be replayed on the other chain if it fails to execute completely.
      *
      * @param _message     Message to compute the amount of required gas for.
+     * @param _minGasLimit Minimum desired gas limit when message goes to target.
      *
      * @return Amount of gas required to guarantee message receipt.
      */
-    function baseGas(bytes memory _message) public pure returns (uint32) {
-        // TODO: Values here are meant to be good enough to get a devnet running. We need to do
-        // some simple experimentation with the smallest and largest possible message sizes to find
-        // the correct constant and dynamic overhead values.
-        return (uint32(_message.length) * MIN_GAS_DYNAMIC_OVERHEAD) + MIN_GAS_CONSTANT_OVERHEAD;
+    function baseGas(bytes memory _message, uint32 _minGasLimit) public pure returns (uint32) {
+        return
+            // Dynamic overhead
+            ((_minGasLimit * MIN_GAS_DYNAMIC_OVERHEAD_NUMERATOR) /
+                MIN_GAS_DYNAMIC_OVERHEAD_DENOMINATOR) +
+            // Calldata overhead
+            (uint32(_message.length) * MIN_GAS_CALLDATA_OVERHEAD) +
+            // Constant overhead
+            MIN_GAS_CONSTANT_OVERHEAD;
     }
 
     /**
@@ -210,7 +225,7 @@ abstract contract CrossDomainMessenger is
         // the minimum gas limit specified by the user.
         _sendMessage(
             otherMessenger,
-            _minGasLimit + baseGas(_message),
+            baseGas(_message, _minGasLimit),
             msg.value,
             abi.encodeWithSelector(
                 this.relayMessage.selector,
@@ -259,17 +274,17 @@ abstract contract CrossDomainMessenger is
             _message
         );
 
-        if (_isSystemMessageSender()) {
+        if (_isOtherMessenger()) {
             // Should never happen.
             require(msg.value == _value, "Mismatched message value.");
         } else {
-            // TODO(tynes): could require that msg.value == 0 here
-            // to prevent eth from getting stuck
+            require(
+                msg.value == 0,
+                "CrossDomainMessenger: Value must be zero unless message is from a system address."
+            );
             require(receivedMessages[versionedHash], "Message cannot be replayed.");
         }
 
-        // TODO: Should blocking happen on sending or receiving side?
-        // TODO: Should this just return with an event instead of reverting?
         require(
             blockedSystemAddresses[_target] == false,
             "Cannot send message to blocked system address."
@@ -277,7 +292,6 @@ abstract contract CrossDomainMessenger is
 
         require(successfulMessages[versionedHash] == false, "Message has already been relayed.");
 
-        // TODO: Make sure this will always give us enough gas.
         require(
             gasleft() >= _minGasLimit + RELAY_GAS_REQUIRED,
             "Insufficient gas to relay message."
@@ -313,6 +327,7 @@ abstract contract CrossDomainMessenger is
      *                                detailed information about what this block list can and
      *                                cannot be used for.
      */
+    // solhint-disable-next-line func-name-mixedcase
     function __CrossDomainMessenger_init(
         address _otherMessenger,
         address[] memory _blockedSystemAddresses
@@ -334,7 +349,7 @@ abstract contract CrossDomainMessenger is
      *         contracts because the logic for this depends on the network where the messenger is
      *         being deployed.
      */
-    function _isSystemMessageSender() internal view virtual returns (bool);
+    function _isOtherMessenger() internal view virtual returns (bool);
 
     /**
      * @notice Sends a low-level message to the other messenger. Needs to be implemented by child

packages/contracts-bedrock/contracts/universal/Semver.sol

@@ -9,16 +9,19 @@ contract Semver {
     /**
      * @notice Contract version number (major).
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable MAJOR_VERSION;
 
     /**
      * @notice Contract version number (minor).
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable MINOR_VERSION;
 
     /**
      * @notice Contract version number (patch).
      */
+    // solhint-disable-next-line var-name-mixedcase
     uint256 public immutable PATCH_VERSION;
 
     /**

packages/contracts-bedrock/contracts/universal/StandardBridge.sol

@@ -368,6 +368,7 @@ abstract contract StandardBridge is Initializable {
      * @param _messenger   Address of CrossDomainMessenger on this network.
      * @param _otherBridge Address of the other StandardBridge contract.
      */
+    // solhint-disable-next-line func-name-mixedcase
     function __StandardBridge_init(address payable _messenger, address payable _otherBridge)
         internal
         onlyInitializing
@@ -442,7 +443,6 @@ abstract contract StandardBridge is Initializable {
 
             OptimismMintableERC20(_localToken).burn(_from, _amount);
         } else {
-            // TODO: Do we need to confirm that the transfer was successful?
             IERC20(_localToken).safeTransferFrom(_from, address(this), _amount);
             deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] + _amount;
         }

packages/contracts-bedrock/tasks/deposits.ts

@@ -66,6 +66,10 @@ task('deposit', 'Deposits funds onto L2.')
     const amountWei = utils.parseEther(amountEth)
     const value = amountWei.add(utils.parseEther('0.01'))
     console.log(`Depositing ${amountEth} ETH to ${to}`)
+
+    const preL2Balance = await l2Provider.getBalance(to)
+    console.log(`${to} has ${utils.formatEther(preL2Balance)} ETH on L2`)
+
     // Below adds 0.01 ETH to account for gas.
     const tx = await OptimismPortal.depositTransaction(
       to,
@@ -77,26 +81,39 @@ task('deposit', 'Deposits funds onto L2.')
     )
     console.log(`Got TX hash ${tx.hash}. Waiting...`)
     const receipt = await tx.wait()
-    console.log(
-      `Included in block ${receipt.blockHash} with index ${receipt.logIndex}`
-    )
+    console.log(`Included in block ${receipt.blockHash}`)
 
     // find the transaction deposited event and derive
     // the deposit transaction from it
     const event = receipt.events.find(
       (e: Event) => e.event === 'TransactionDeposited'
     )
-    const l2tx = DepositTx.fromL1Event(event)
     console.log(`Deposit has log index ${event.logIndex}`)
+    const l2tx = DepositTx.fromL1Event(event)
     const hash = l2tx.hash()
     console.log(`Waiting for L2 TX hash ${hash}`)
 
+    let i = 0
     while (true) {
       const expected = await l2Provider.send('eth_getTransactionByHash', [hash])
       if (expected) {
         console.log('Deposit success')
+        console.log(JSON.stringify(expected, null, 2))
         break
       }
+      const postL2Balance = await l2Provider.getBalance(to)
+      if (postL2Balance.gt(preL2Balance)) {
+        console.log(
+          `Unexpected balance increase without detecting deposit transaction`
+        )
+      }
+
+      if (i % 100 === 0) {
+        const block = await l2Provider.getBlock('latest')
+        console.log(`latest block ${block.number}:${block.hash}`)
+      }
+
       await sleep(500)
+      i++
     }
   })

packages/contracts-bedrock/tasks/genesis-l2.ts

@@ -52,6 +52,7 @@ task('genesis-l2', 'create a genesis config')
     'The file to write the output JSON to',
     'genesis.json'
   )
+  .addOptionalParam('l1RpcUrl', 'The L1 RPC URL', 'http://127.0.0.1:8545')
   .setAction(async (args, hre) => {
     const {
       computeStorageSlots,
@@ -60,6 +61,8 @@ task('genesis-l2', 'create a genesis config')
 
     const { deployConfig } = hre
 
+    const l1 = new ethers.providers.StaticJsonRpcProvider(args.l1RpcUrl)
+
     // Use the addresses of the proxies here instead of the implementations
     // Be backwards compatible
     let ProxyL1CrossDomainMessenger = await hre.deployments.getOrNull(
@@ -256,6 +259,9 @@ task('genesis-l2', 'create a genesis config')
       }
     }
 
+    const portal = await hre.deployments.get('OptimismPortalProxy')
+    const l1StartingBlock = await l1.getBlock(portal.receipt.blockHash)
+
     const genesis: OptimismGenesis = {
       config: {
         chainId: deployConfig.genesisBlockChainid,
@@ -279,9 +285,7 @@ task('genesis-l2', 'create a genesis config')
       },
       nonce: '0x1234',
       difficulty: '0x1',
-      timestamp: ethers.BigNumber.from(
-        deployConfig.startingTimestamp
-      ).toHexString(),
+      timestamp: ethers.BigNumber.from(l1StartingBlock.timestamp).toHexString(),
       gasLimit: deployConfig.genesisBlockGasLimit,
       extraData: deployConfig.genesisBlockExtradata,
       optimism: {

packages/contracts-bedrock/tasks/rollup-config.ts

@@ -21,6 +21,7 @@ task('rollup-config', 'create a genesis config')
     const l2Genesis = await l2.getBlock('earliest')
 
     const portal = await hre.deployments.get('OptimismPortalProxy')
+    const l1StartingBlock = await l1.getBlock(portal.receipt.blockHash)
 
     const config: OpNodeConfig = {
       genesis: {
@@ -32,7 +33,7 @@ task('rollup-config', 'create a genesis config')
           hash: l2Genesis.hash,
           number: 0,
         },
-        l2_time: deployConfig.startingTimestamp,
+        l2_time: l1StartingBlock.timestamp,
       },
       block_time: deployConfig.l2BlockTime,
       max_sequencer_drift: deployConfig.maxSequencerDrift,

packages/fault-detector/src/service.ts

@@ -2,7 +2,7 @@ import { BaseServiceV2, Gauge, validators } from '@eth-optimism/common-ts'
 import { getChainId, sleep, toRpcHexString } from '@eth-optimism/core-utils'
 import { CrossChainMessenger } from '@eth-optimism/sdk'
 import { Provider } from '@ethersproject/abstract-provider'
-import { Contract, ethers } from 'ethers'
+import { Contract, ethers, Transaction } from 'ethers'
 import dateformat from 'dateformat'
 
 import {
@@ -20,10 +20,12 @@ type Metrics = {
   highestCheckedBatchIndex: Gauge
   highestKnownBatchIndex: Gauge
   isCurrentlyMismatched: Gauge
-  inUnexpectedErrorState: Gauge
+  l1NodeConnectionFailures: Gauge
+  l2NodeConnectionFailures: Gauge
 }
 
 type State = {
+  fpw: number
   scc: Contract
   messenger: CrossChainMessenger
   highestCheckedBatchIndex: number
@@ -68,9 +70,13 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
           type: Gauge,
           desc: '0 if state is ok, 1 if state is mismatched',
         },
-        inUnexpectedErrorState: {
+        l1NodeConnectionFailures: {
           type: Gauge,
-          desc: '0 if service is ok, 1 service is in unexpected error state',
+          desc: 'Number of times L1 node connection has failed',
+        },
+        l2NodeConnectionFailures: {
+          type: Gauge,
+          desc: 'Number of times L2 node connection has failed',
         },
       },
     })
@@ -86,6 +92,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
 
     // We use this a lot, a bit cleaner to pull out to the top level of the state object.
     this.state.scc = this.state.messenger.contracts.l1.StateCommitmentChain
+    this.state.fpw = (await this.state.scc.FRAUD_PROOF_WINDOW()).toNumber()
 
     // Figure out where to start syncing from.
     if (this.options.startBatchIndex === -1) {
@@ -102,17 +109,30 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
   }
 
   async main(): Promise<void> {
-    const latestBatchIndex = await this.state.scc.getTotalBatches()
-    if (this.state.highestCheckedBatchIndex >= latestBatchIndex.toNumber()) {
+    let latestBatchIndex: number
+    try {
+      latestBatchIndex = (await this.state.scc.getTotalBatches()).toNumber()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l1',
+        section: 'getTotalBatches',
+      })
+      this.metrics.l1NodeConnectionFailures.inc()
       await sleep(15000)
       return
     }
 
-    this.metrics.highestKnownBatchIndex.set(latestBatchIndex.toNumber())
+    if (this.state.highestCheckedBatchIndex >= latestBatchIndex) {
+      await sleep(15000)
+      return
+    } else {
+      this.metrics.highestKnownBatchIndex.set(latestBatchIndex)
+    }
 
     this.logger.info(`checking batch`, {
       batchIndex: this.state.highestCheckedBatchIndex,
-      latestIndex: latestBatchIndex.toNumber(),
+      latestIndex: latestBatchIndex,
     })
 
     let event: ethers.Event
@@ -122,13 +142,30 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
         this.state.highestCheckedBatchIndex
       )
     } catch (err) {
-      this.logger.error(`got unexpected error while searching for batch`, {
-        batchIndex: this.state.highestCheckedBatchIndex,
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l1',
+        section: 'findEventForStateBatch',
+      })
+      this.metrics.l1NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
+    }
+
+    let batchTransaction: Transaction
+    try {
+      batchTransaction = await event.getTransaction()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
         error: err,
+        node: 'l1',
+        section: 'getTransaction',
       })
+      this.metrics.l1NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
     }
 
-    const batchTransaction = await event.getTransaction()
     const [stateRoots] = this.state.scc.interface.decodeFunctionData(
       'appendStateBatch',
       batchTransaction.data
@@ -138,7 +175,20 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
     const batchSize = event.args._batchSize.toNumber()
     const batchEnd = batchStart + batchSize
 
-    const latestBlock = await this.options.l2RpcProvider.getBlockNumber()
+    let latestBlock: number
+    try {
+      latestBlock = await this.options.l2RpcProvider.getBlockNumber()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l2',
+        section: 'getBlockNumber',
+      })
+      this.metrics.l2NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
+    }
+
     if (latestBlock < batchEnd) {
       this.logger.info(`node is behind, waiting for sync`, {
         batchEnd,
@@ -151,21 +201,32 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
     // multiple requests of maximum 1000 blocks in the case that batchSize > 1000.
     let blocks: any[] = []
     for (let i = 0; i < batchSize; i += 1000) {
-      const provider = this.options
-        .l2RpcProvider as ethers.providers.JsonRpcProvider
-      blocks = blocks.concat(
-        await provider.send('eth_getBlockRange', [
+      let newBlocks: any[]
+      try {
+        newBlocks = await (
+          this.options.l2RpcProvider as ethers.providers.JsonRpcProvider
+        ).send('eth_getBlockRange', [
           toRpcHexString(batchStart + i),
           toRpcHexString(batchStart + i + Math.min(batchSize - i, 1000) - 1),
           false,
         ])
-      )
+      } catch (err) {
+        this.logger.error(`got error when connecting to node`, {
+          error: err,
+          node: 'l2',
+          section: 'getBlockRange',
+        })
+        this.metrics.l2NodeConnectionFailures.inc()
+        await sleep(15000)
+        return
+      }
+
+      blocks = blocks.concat(newBlocks)
     }
 
     for (const [i, stateRoot] of stateRoots.entries()) {
       if (blocks[i].stateRoot !== stateRoot) {
         this.metrics.isCurrentlyMismatched.set(1)
-        const fpw = await this.state.scc.FRAUD_PROOF_WINDOW()
         this.logger.error(`state root mismatch`, {
           blockNumber: blocks[i].number,
           expectedStateRoot: blocks[i].stateRoot,
@@ -173,7 +234,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
           finalizationTime: dateformat(
             new Date(
               (ethers.BigNumber.from(blocks[i].timestamp).toNumber() +
-                fpw.toNumber()) *
+                this.state.fpw) *
                 1000
             ),
             'mmmm dS, yyyy, h:MM:ss TT'
@@ -190,7 +251,6 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
 
     // If we got through the above without throwing an error, we should be fine to reset.
     this.metrics.isCurrentlyMismatched.set(0)
-    this.metrics.inUnexpectedErrorState.set(0)
   }
 }