Merge branch 'develop' into feat/withdrawal-script-fix

op-program/host/kvstore/disk.go

+package kvstore
+
+import (
+	"encoding/hex"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path"
+	"sync"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// read/write mode for user/group/other, not executable.
+const diskPermission = 0666
+
+// DiskKV is a disk-backed key-value store, every key-value pair is a hex-encoded .txt file, with the value as content.
+// DiskKV is safe for concurrent use with a single DiskKV instance.
+// DiskKV is not safe for concurrent use between different DiskKV instances of the same disk directory:
+// a Put needs to be completed before another DiskKV Get retrieves the values.
+type DiskKV struct {
+	sync.RWMutex
+	path string
+}
+
+// NewDiskKV creates a DiskKV that puts/gets pre-images as files in the given directory path.
+// The path must exist, or subsequent Put/Get calls will error when it does not.
+func NewDiskKV(path string) *DiskKV {
+	return &DiskKV{path: path}
+}
+
+func (d *DiskKV) pathKey(k common.Hash) string {
+	return path.Join(d.path, k.String()+".txt")
+}
+
+func (d *DiskKV) Put(k common.Hash, v []byte) error {
+	d.Lock()
+	defer d.Unlock()
+	f, err := os.OpenFile(d.pathKey(k), os.O_WRONLY|os.O_CREATE|os.O_EXCL|os.O_TRUNC, diskPermission)
+	if err != nil {
+		if errors.Is(err, os.ErrExist) {
+			return ErrAlreadyExists
+		}
+		return fmt.Errorf("failed to open new pre-image file %s: %w", k, err)
+	}
+	if _, err := f.Write([]byte(hex.EncodeToString(v))); err != nil {
+		_ = f.Close()
+		return fmt.Errorf("failed to write pre-image %s to disk: %w", k, err)
+	}
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("failed to close pre-image %s file: %w", k, err)
+	}
+	return nil
+}
+
+func (d *DiskKV) Get(k common.Hash) ([]byte, error) {
+	d.RLock()
+	defer d.RUnlock()
+	f, err := os.OpenFile(d.pathKey(k), os.O_RDONLY, diskPermission)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, ErrNotFound
+		}
+		return nil, fmt.Errorf("failed to open pre-image file %s: %w", k, err)
+	}
+	defer f.Close() // fine to ignore closing error here
+	dat, err := io.ReadAll(f)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read pre-image from file %s: %w", k, err)
+	}
+	return hex.DecodeString(string(dat))
+}
+
+var _ KV = (*DiskKV)(nil)

op-program/host/kvstore/disk_test.go

+package kvstore
+
+import "testing"
+
+func TestDiskKV(t *testing.T) {
+	tmp := t.TempDir() // automatically removed by testing cleanup
+	kv := NewDiskKV(tmp)
+	kvTest(t, kv)
+}

op-program/host/kvstore/kv.go

+package kvstore
+
+import (
+	"errors"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// ErrNotFound is returned when a pre-image cannot be found in the KV store.
+var ErrNotFound = errors.New("not found")
+
+// ErrAlreadyExists is returned when a pre-image already exists in the KV store.
+var ErrAlreadyExists = errors.New("already exists")
+
+// KV is a Key-Value store interface for pre-image data.
+type KV interface {
+	// Put puts the pre-image value v in the key-value store with key k.
+	// It returns ErrAlreadyExists when the key already exists.
+	// KV store implementations may return additional errors specific to the KV storage.
+	Put(k common.Hash, v []byte) error
+
+	// Get retrieves the pre-image with key k from the key-value store.
+	// It returns ErrNotFound when the pre-image cannot be found.
+	// KV store implementations may return additional errors specific to the KV storage.
+	Get(k common.Hash) ([]byte, error)
+}

op-program/host/kvstore/kv_test.go

+package kvstore
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+)
+
+func kvTest(t *testing.T, kv KV) {
+	t.Run("roundtrip", func(t *testing.T) {
+		t.Parallel()
+		_, err := kv.Get(common.Hash{0xaa})
+		require.Equal(t, err, ErrNotFound, "file (in new tmp dir) does not exist yet")
+
+		require.NoError(t, kv.Put(common.Hash{0xaa}, []byte("hello world")))
+		dat, err := kv.Get(common.Hash{0xaa})
+		require.NoError(t, err, "pre-image must exist now")
+		require.Equal(t, "hello world", string(dat), "pre-image must match")
+	})
+
+	t.Run("empty pre-image", func(t *testing.T) {
+		t.Parallel()
+		require.NoError(t, kv.Put(common.Hash{0xbb}, []byte{}))
+		dat, err := kv.Get(common.Hash{0xbb})
+		require.NoError(t, err, "pre-image must exist now")
+		require.Zero(t, len(dat), "pre-image must be empty")
+	})
+
+	t.Run("zero pre-image key", func(t *testing.T) {
+		t.Parallel()
+		// in case we give a pre-image a special empty key. If it was a hash then we wouldn't know the pre-image.
+		require.NoError(t, kv.Put(common.Hash{}, []byte("hello")))
+		dat, err := kv.Get(common.Hash{})
+		require.NoError(t, err, "pre-image must exist now")
+		require.Equal(t, "hello", string(dat), "pre-image must match")
+	})
+
+	t.Run("non-string value", func(t *testing.T) {
+		t.Parallel()
+		// in case we give a pre-image a special empty key. If it was a hash then we wouldn't know the pre-image.
+		require.NoError(t, kv.Put(common.Hash{0xcc}, []byte{4, 2}))
+		dat, err := kv.Get(common.Hash{0xcc})
+		require.NoError(t, err, "pre-image must exist now")
+		require.Equal(t, []byte{4, 2}, dat, "pre-image must match")
+	})
+
+	t.Run("not overwriting pre-image", func(t *testing.T) {
+		t.Parallel()
+		require.NoError(t, kv.Put(common.Hash{0xdd}, []byte{4, 2}))
+		require.ErrorIs(t, kv.Put(common.Hash{0xdd}, []byte{4, 2}), ErrAlreadyExists)
+	})
+}

op-program/host/kvstore/mem.go

+package kvstore
+
+import (
+	"sync"
+
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// MemKV implements the KV store interface in memory, backed by a regular Go map.
+// This should only be used in testing, as large programs may require more pre-image data than available memory.
+// MemKV is safe for concurrent use.
+type MemKV struct {
+	sync.RWMutex
+	m map[common.Hash][]byte
+}
+
+var _ KV = (*MemKV)(nil)
+
+func NewMemKV() *MemKV {
+	return &MemKV{m: make(map[common.Hash][]byte)}
+}
+
+func (m *MemKV) Put(k common.Hash, v []byte) error {
+	m.Lock()
+	defer m.Unlock()
+	if _, ok := m.m[k]; ok {
+		return ErrAlreadyExists
+	}
+	m.m[k] = v
+	return nil
+}
+
+func (m *MemKV) Get(k common.Hash) ([]byte, error) {
+	m.RLock()
+	defer m.RUnlock()
+	v, ok := m.m[k]
+	if !ok {
+		return nil, ErrNotFound
+	}
+	return v, nil
+}

op-program/host/kvstore/mem_test.go

+package kvstore
+
+import "testing"
+
+func TestMemKV(t *testing.T) {
+	kv := NewMemKV()
+	kvTest(t, kv)
+}

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

@@ -80,7 +80,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
     event ConfigUpdate(uint256 indexed version, UpdateType indexed updateType, bytes data);
 
     /**
-     * @custom:semver 1.2.0
+     * @custom:semver 1.3.0
      *
      * @param _owner             Initial owner of the contract.
      * @param _overhead          Initial overhead value.
@@ -98,7 +98,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
         uint64 _gasLimit,
         address _unsafeBlockSigner,
         ResourceMetering.ResourceConfig memory _config
-    ) Semver(1, 2, 0) {
+    ) Semver(1, 3, 0) {
         initialize({
             _owner: _owner,
             _overhead: _overhead,
@@ -270,7 +270,7 @@ contract SystemConfig is OwnableUpgradeable, Semver {
             "SystemConfig: min base fee must be less than max base"
         );
         // Base fee change denominator must be greater than 0.
-        require(_config.baseFeeMaxChangeDenominator > 0, "SystemConfig: denominator cannot be 0");
+        require(_config.baseFeeMaxChangeDenominator > 1, "SystemConfig: denominator must be larger than 1");
         // Max resource limit plus system tx gas must be less than or equal to the L2 gas limit.
         // The gas limit must be increased before these values can be increased.
         require(

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

@@ -1085,3 +1085,88 @@ contract OptimismPortalUpgradeable_Test is Portal_Initializer {
         assertEq(slot21Expected, slot21After);
     }
 }
+
+/**
+ * @title OptimismPortalResourceFuzz_Test
+ * @dev Test various values of the resource metering config to ensure that deposits cannot be
+ *         broken by changing the config.
+ */
+contract OptimismPortalResourceFuzz_Test is Portal_Initializer {
+
+    /**
+     * @dev The max gas limit observed throughout this test. Setting this too high can cause
+     *      the test to take too long to run.
+     */
+    uint256 constant MAX_GAS_LIMIT = 30_000_000;
+
+    /**
+     * @dev Test that various values of the resource metering config will not break deposits.
+     */
+    function testFuzz_systemConfigDeposit_succeeds(
+        uint32 _maxResourceLimit,
+        uint8 _elasticityMultiplier,
+        uint8 _baseFeeMaxChangeDenominator,
+        uint32 _minimumBaseFee,
+        uint32 _systemTxMaxGas,
+        uint128 _maximumBaseFee,
+        uint64 _gasLimit,
+        uint64 _prevBoughtGas,
+        uint128 _prevBaseFee,
+        uint8 _blockDiff
+    ) external {
+        // Get the set system gas limit
+        uint64 gasLimit = systemConfig.gasLimit();
+        // Bound resource config
+        _maxResourceLimit = uint32(bound(_maxResourceLimit, 21000, MAX_GAS_LIMIT / 8));
+        _gasLimit = uint64(bound( _gasLimit,  21000, _maxResourceLimit));
+        _prevBaseFee = uint128(bound(_prevBaseFee, 0, 5 gwei));
+        // Prevent values that would cause reverts
+        vm.assume(gasLimit >= _gasLimit);
+        vm.assume(_minimumBaseFee < _maximumBaseFee);
+        vm.assume(_baseFeeMaxChangeDenominator > 1);
+        vm.assume(uint256(_maxResourceLimit) + uint256(_systemTxMaxGas) <= gasLimit);
+        vm.assume(_elasticityMultiplier > 0);
+        vm.assume(
+            ((_maxResourceLimit / _elasticityMultiplier) * _elasticityMultiplier) == _maxResourceLimit
+        );
+        _prevBoughtGas = uint64(bound(_prevBoughtGas, 0, _maxResourceLimit - _gasLimit));
+        _blockDiff = uint8(bound(_blockDiff, 0, 3));
+
+        // Create a resource config to mock the call to the system config with
+        ResourceMetering.ResourceConfig memory rcfg = ResourceMetering.ResourceConfig({
+            maxResourceLimit: _maxResourceLimit,
+            elasticityMultiplier: _elasticityMultiplier,
+            baseFeeMaxChangeDenominator: _baseFeeMaxChangeDenominator,
+            minimumBaseFee: _minimumBaseFee,
+            systemTxMaxGas: _systemTxMaxGas,
+            maximumBaseFee: _maximumBaseFee
+        });
+        vm.mockCall(
+            address(systemConfig),
+            abi.encodeWithSelector(systemConfig.resourceConfig.selector),
+            abi.encode(rcfg)
+        );
+
+        // Set the resource params
+        uint256 _prevBlockNum = block.number - _blockDiff;
+        vm.store(
+            address(op),
+            bytes32(uint256(1)),
+            bytes32((_prevBlockNum << 192) | (uint256(_prevBoughtGas) << 128) | _prevBaseFee)
+        );
+        // Ensure that the storage setting is correct
+        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = op.params();
+        assertEq(prevBaseFee, _prevBaseFee);
+        assertEq(prevBoughtGas, _prevBoughtGas);
+        assertEq(prevBlockNum, _prevBlockNum);
+
+        // Do a deposit, should not revert
+        op.depositTransaction{ gas: MAX_GAS_LIMIT }({
+            _to: address(0x20),
+            _value: 0x40,
+            _gasLimit: _gasLimit,
+            _isCreation: false,
+            _data: hex""
+        });
+    }
+}

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

@@ -110,7 +110,7 @@ contract SystemConfig_Setters_TestFail is SystemConfig_Init {
             maximumBaseFee: 2 gwei
         });
         vm.prank(sysConf.owner());
-        vm.expectRevert("SystemConfig: denominator cannot be 0");
+        vm.expectRevert("SystemConfig: denominator must be larger than 1");
         sysConf.setResourceConfig(config);
     }