l2geth: allow 0 gasprice txs for `OVM_GasPriceOracle.owner`

This PR allows the owner of the `OVM_GasPriceOracle` to send
transactions with 0 gas price when the enforce fees config option
is turned on.

The L2 gas price is currently updated by sending transactions to the
chain to a special contract. In the future it should be updated as a
side effect of transaction execution. Having the gas price on chain is
important so that it can be replicated accross the network to ensure
that users can send transactions with a high enough fee.

Having the `OVM_GasPriceOracle.owner` key not need to maintain
ETH on L2 is an operational simplification as well prevents a
terrible scenario where a bug causes the L2 gas price to go so high
that it is impossible for the owner to update it.

.changeset/forty-pigs-study.md

+---
+'@eth-optimism/l2geth': patch
+---
+
+Allow zero gas price transactions from the `OVM_GasPriceOracle.owner` when enforce fees is set to true. This is to prevent the need to manage an additional hot wallet as well as prevent any situation where a bug causes the fees to go too high that it is not possible to lower the fee by sending a transaction

l2geth/rollup/sync_service.go

@@ -24,47 +24,62 @@ import (
 	"github.com/ethereum/go-ethereum/rollup/fees"
 )
 
-// errShortRemoteTip is an error for when the remote tip is shorter than the
-// local tip
 var (
+	// errBadConfig is the error when the SyncService is started with invalid
+	// configuration options
+	errBadConfig = errors.New("bad config")
+	// errShortRemoteTip is an error for when the remote tip is shorter than the
+	// local tip
 	errShortRemoteTip = errors.New("unexpected remote less than tip")
-	errBadConfig      = errors.New("bad config")
+	// errZeroGasPriceTx is the error for when a user submits a transaction
+	// with gas price zero and fees are currently enforced
+	errZeroGasPriceTx = errors.New("cannot accept 0 gas price transaction")
 	float1            = big.NewFloat(1)
 )
 
-// L2GasPrice slot refers to the storage slot that the execution price is stored
-// in the L2 predeploy contract, the GasPriceOracle
-var l2GasPriceSlot = common.BigToHash(big.NewInt(1))
-var l2GasPriceOracleAddress = common.HexToAddress("0x420000000000000000000000000000000000000F")
+var (
+	// l2GasPriceSlot refers to the storage slot that the L2 gas price is stored
+	// in in the OVM_GasPriceOracle predeploy
+	l2GasPriceSlot = common.BigToHash(big.NewInt(1))
+	// l2GasPriceOracleOwnerSlot refers to the storage slot that the owner of
+	// the OVM_GasPriceOracle is stored in
+	l2GasPriceOracleOwnerSlot = common.BigToHash(big.NewInt(0))
+	// l2GasPriceOracleAddress is the address of the OVM_GasPriceOracle
+	// predeploy
+	l2GasPriceOracleAddress = common.HexToAddress("0x420000000000000000000000000000000000000F")
+)
 
 // SyncService implements the main functionality around pulling in transactions
 // and executing them. It can be configured to run in both sequencer mode and in
 // verifier mode.
 type SyncService struct {
-	ctx                       context.Context
-	cancel                    context.CancelFunc
-	verifier                  bool
-	db                        ethdb.Database
-	scope                     event.SubscriptionScope
-	txFeed                    event.Feed
-	txLock                    sync.Mutex
-	loopLock                  sync.Mutex
-	enable                    bool
-	eth1ChainId               uint64
-	bc                        *core.BlockChain
-	txpool                    *core.TxPool
-	RollupGpo                 *gasprice.RollupOracle
-	client                    RollupClient
-	syncing                   atomic.Value
-	chainHeadSub              event.Subscription
-	OVMContext                OVMContext
-	pollInterval              time.Duration
-	timestampRefreshThreshold time.Duration
-	chainHeadCh               chan core.ChainHeadEvent
-	backend                   Backend
-	enforceFees               bool
-	feeThresholdUp            *big.Float
-	feeThresholdDown          *big.Float
+	ctx                            context.Context
+	cancel                         context.CancelFunc
+	verifier                       bool
+	db                             ethdb.Database
+	scope                          event.SubscriptionScope
+	txFeed                         event.Feed
+	txLock                         sync.Mutex
+	loopLock                       sync.Mutex
+	enable                         bool
+	eth1ChainId                    uint64
+	bc                             *core.BlockChain
+	txpool                         *core.TxPool
+	RollupGpo                      *gasprice.RollupOracle
+	client                         RollupClient
+	syncing                        atomic.Value
+	chainHeadSub                   event.Subscription
+	OVMContext                     OVMContext
+	pollInterval                   time.Duration
+	timestampRefreshThreshold      time.Duration
+	chainHeadCh                    chan core.ChainHeadEvent
+	backend                        Backend
+	gasPriceOracleOwnerAddress     common.Address
+	gasPriceOracleOwnerAddressLock *sync.RWMutex
+	enforceFees                    bool
+	signer                         types.Signer
+	feeThresholdUp                 *big.Float
+	feeThresholdDown               *big.Float
 }
 
 // NewSyncService returns an initialized sync service
@@ -122,23 +137,26 @@ func NewSyncService(ctx context.Context, cfg Config, txpool *core.TxPool, bc *co
 	}
 
 	service := SyncService{
-		ctx:                       ctx,
-		cancel:                    cancel,
-		verifier:                  cfg.IsVerifier,
-		enable:                    cfg.Eth1SyncServiceEnable,
-		syncing:                   atomic.Value{},
-		bc:                        bc,
-		txpool:                    txpool,
-		chainHeadCh:               make(chan core.ChainHeadEvent, 1),
-		eth1ChainId:               cfg.Eth1ChainId,
-		client:                    client,
-		db:                        db,
-		pollInterval:              pollInterval,
-		timestampRefreshThreshold: timestampRefreshThreshold,
-		backend:                   cfg.Backend,
-		enforceFees:               cfg.EnforceFees,
-		feeThresholdDown:          cfg.FeeThresholdDown,
-		feeThresholdUp:            cfg.FeeThresholdUp,
+		ctx:                            ctx,
+		cancel:                         cancel,
+		verifier:                       cfg.IsVerifier,
+		enable:                         cfg.Eth1SyncServiceEnable,
+		syncing:                        atomic.Value{},
+		bc:                             bc,
+		txpool:                         txpool,
+		chainHeadCh:                    make(chan core.ChainHeadEvent, 1),
+		eth1ChainId:                    cfg.Eth1ChainId,
+		client:                         client,
+		db:                             db,
+		pollInterval:                   pollInterval,
+		timestampRefreshThreshold:      timestampRefreshThreshold,
+		backend:                        cfg.Backend,
+		gasPriceOracleOwnerAddress:     cfg.GasPriceOracleOwnerAddress,
+		gasPriceOracleOwnerAddressLock: new(sync.RWMutex),
+		enforceFees:                    cfg.EnforceFees,
+		signer:                         types.NewEIP155Signer(chainID),
+		feeThresholdDown:               cfg.FeeThresholdDown,
+		feeThresholdUp:                 cfg.FeeThresholdUp,
 	}
 
 	// The chainHeadSub is used to synchronize the SyncService with the chain.
@@ -234,8 +252,12 @@ func (s *SyncService) Start() error {
 		return nil
 	}
 	log.Info("Initializing Sync Service", "eth1-chainid", s.eth1ChainId)
-	s.updateL2GasPrice(nil)
-	s.updateL1GasPrice()
+	if err := s.updateGasPriceOracleCache(nil); err != nil {
+		return err
+	}
+	if err := s.updateL1GasPrice(); err != nil {
+		return err
+	}
 
 	if s.verifier {
 		go s.VerifierLoop()
@@ -361,7 +383,7 @@ func (s *SyncService) VerifierLoop() {
 		if err := s.verify(); err != nil {
 			log.Error("Could not verify", "error", err)
 		}
-		if err := s.updateL2GasPrice(nil); err != nil {
+		if err := s.updateGasPriceOracleCache(nil); err != nil {
 			log.Error("Cannot update L2 gas price", "msg", err)
 		}
 	}
@@ -398,7 +420,7 @@ func (s *SyncService) SequencerLoop() {
 		}
 		s.txLock.Unlock()
 
-		if err := s.updateL2GasPrice(nil); err != nil {
+		if err := s.updateGasPriceOracleCache(nil); err != nil {
 			log.Error("Cannot update L2 gas price", "msg", err)
 		}
 		if err := s.updateContext(); err != nil {
@@ -462,25 +484,68 @@ func (s *SyncService) updateL1GasPrice() error {
 	return nil
 }
 
-// updateL2GasPrice accepts a state root and reads the gas price from the gas
-// price oracle at the state that corresponds to the state root. If no state
-// root is passed in, then the tip is used.
-func (s *SyncService) updateL2GasPrice(hash *common.Hash) error {
-	var state *state.StateDB
+// updateL2GasPrice accepts a state db and reads the gas price from the gas
+// price oracle at the state that corresponds to the state db. If no state db
+// is passed in, then the tip is used.
+func (s *SyncService) updateL2GasPrice(statedb *state.StateDB) error {
+	var err error
+	if statedb == nil {
+		statedb, err = s.bc.State()
+		if err != nil {
+			return err
+		}
+	}
+	result := statedb.GetState(l2GasPriceOracleAddress, l2GasPriceSlot)
+	s.RollupGpo.SetL2GasPrice(result.Big())
+	return nil
+}
+
+// cacheGasPriceOracleOwner accepts a statedb and caches the gas price oracle
+// owner address locally
+func (s *SyncService) cacheGasPriceOracleOwner(statedb *state.StateDB) error {
+	var err error
+	if statedb == nil {
+		statedb, err = s.bc.State()
+		if err != nil {
+			return err
+		}
+	}
+	s.gasPriceOracleOwnerAddressLock.Lock()
+	defer s.gasPriceOracleOwnerAddressLock.Unlock()
+	result := statedb.GetState(l2GasPriceOracleAddress, l2GasPriceOracleOwnerSlot)
+	s.gasPriceOracleOwnerAddress = common.BytesToAddress(result.Bytes())
+	return nil
+}
+
+// updateGasPriceOracleCache caches the owner as well as updating the
+// the L2 gas price from the OVM_GasPriceOracle
+func (s *SyncService) updateGasPriceOracleCache(hash *common.Hash) error {
+	var statedb *state.StateDB
 	var err error
 	if hash != nil {
-		state, err = s.bc.StateAt(*hash)
+		statedb, err = s.bc.StateAt(*hash)
 	} else {
-		state, err = s.bc.State()
+		statedb, err = s.bc.State()
 	}
 	if err != nil {
 		return err
 	}
-	result := state.GetState(l2GasPriceOracleAddress, l2GasPriceSlot)
-	s.RollupGpo.SetL2GasPrice(result.Big())
+	if err := s.cacheGasPriceOracleOwner(statedb); err != nil {
+		return err
+	}
+	if err := s.updateL2GasPrice(statedb); err != nil {
+		return err
+	}
 	return nil
 }
 
+// A thread safe getter for the gas price oracle owner address
+func (s *SyncService) GasPriceOracleOwnerAddress() *common.Address {
+	s.gasPriceOracleOwnerAddressLock.RLock()
+	defer s.gasPriceOracleOwnerAddressLock.RUnlock()
+	return &s.gasPriceOracleOwnerAddress
+}
+
 /// Update the execution context's timestamp and blocknumber
 /// over time. This is only necessary for the sequencer.
 func (s *SyncService) updateContext() error {
@@ -755,9 +820,21 @@ func (s *SyncService) applyBatchedTransaction(tx *types.Transaction) error {
 // verifyFee will verify that a valid fee is being paid.
 func (s *SyncService) verifyFee(tx *types.Transaction) error {
 	if tx.GasPrice().Cmp(common.Big0) == 0 {
+		// Allow 0 gas price transactions only if it is the owner of the gas
+		// price oracle
+		gpoOwner := s.GasPriceOracleOwnerAddress()
+		if gpoOwner != nil {
+			from, err := types.Sender(s.signer, tx)
+			if err != nil {
+				return fmt.Errorf("invalid transaction: %w", core.ErrInvalidSender)
+			}
+			if from == *gpoOwner {
+				return nil
+			}
+		}
 		// Exit early if fees are enforced and the gasPrice is set to 0
 		if s.enforceFees {
-			return errors.New("cannot accept 0 gas price transaction")
+			return errZeroGasPriceTx
 		}
 		// If fees are not enforced and the gas price is 0, return early
 		return nil
@@ -832,8 +909,7 @@ func (s *SyncService) ValidateAndApplySequencerTransaction(tx *types.Transaction
 	if qo != types.QueueOriginSequencer {
 		return fmt.Errorf("invalid transaction with queue origin %d", qo)
 	}
-	err := s.txpool.ValidateTx(tx)
-	if err != nil {
+	if err := s.txpool.ValidateTx(tx); err != nil {
 		return fmt.Errorf("invalid transaction: %w", err)
 	}
 	return s.applyTransaction(tx)

l2geth/rollup/sync_service_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/eth/gasprice"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/event"
@@ -533,9 +534,9 @@ func TestSyncServiceL2GasPrice(t *testing.T) {
 	}
 	l2GasPrice := big.NewInt(100000000000)
 	state.SetState(l2GasPriceOracleAddress, l2GasPriceSlot, common.BigToHash(l2GasPrice))
-	root, _ := state.Commit(false)
+	_, _ = state.Commit(false)
 
-	service.updateL2GasPrice(&root)
+	service.updateL2GasPrice(state)
 
 	post, err := service.RollupGpo.SuggestL2GasPrice(context.Background())
 	if err != nil {
@@ -547,6 +548,95 @@ func TestSyncServiceL2GasPrice(t *testing.T) {
 	}
 }
 
+func TestSyncServiceGasPriceOracleOwnerAddress(t *testing.T) {
+	service, _, _, err := newTestSyncService(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// newTestSyncService doesn't set the initial owner address
+	// so it initializes to the zero value
+	owner := service.GasPriceOracleOwnerAddress()
+	if *owner != (common.Address{}) {
+		t.Fatal("address not initialized to 0")
+	}
+
+	state, err := service.bc.State()
+	if err != nil {
+		t.Fatal("cannot get state db")
+	}
+
+	// Update the owner in the state to a non zero address
+	updatedOwner := common.HexToAddress("0xEA674fdDe714fd979de3EdF0F56AA9716B898ec8")
+	state.SetState(l2GasPriceOracleAddress, l2GasPriceOracleOwnerSlot, updatedOwner.Hash())
+	hash, _ := state.Commit(false)
+
+	// Update the cache based on the latest state root
+	if err := service.updateGasPriceOracleCache(&hash); err != nil {
+		t.Fatal(err)
+	}
+	got := service.GasPriceOracleOwnerAddress()
+	if *got != updatedOwner {
+		t.Fatalf("mismatch:\ngot %s\nexpected %s", got.Hex(), updatedOwner.Hex())
+	}
+}
+
+// Only the gas price oracle owner can send 0 gas price txs
+// when fees are enforced
+func TestFeeGasPriceOracleOwnerTransactions(t *testing.T) {
+	service, _, _, err := newTestSyncService(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	signer := types.NewEIP155Signer(big.NewInt(420))
+
+	// Fees must be enforced for this test
+	service.enforceFees = true
+	// Generate a key
+	key, _ := crypto.GenerateKey()
+	owner := crypto.PubkeyToAddress(key.PublicKey)
+	// Set as the owner on the SyncService
+	service.gasPriceOracleOwnerAddress = owner
+	if owner != *service.GasPriceOracleOwnerAddress() {
+		t.Fatal("owner mismatch")
+	}
+	// Create a mock transaction and sign using the
+	// owner's key
+	tx := mockTx()
+	// Make sure the gas price is 0 on the dummy tx
+	if tx.GasPrice().Cmp(common.Big0) != 0 {
+		t.Fatal("gas price not 0")
+	}
+	// Sign the dummy tx with the owner key
+	signedTx, err := types.SignTx(tx, signer, key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Verify the fee of the signed tx, ensure it does not error
+	if err := service.verifyFee(signedTx); err != nil {
+		t.Fatal(err)
+	}
+	// Generate a new random key that is not the owner
+	badKey, _ := crypto.GenerateKey()
+	// Ensure that it is not the owner
+	if owner == crypto.PubkeyToAddress(badKey.PublicKey) {
+		t.Fatal("key mismatch")
+	}
+	// Sign the transaction with the bad key
+	badSignedTx, err := types.SignTx(tx, signer, badKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Attempt to verify the fee of the bad tx
+	// It should error and be a errZeroGasPriceTx
+	if err := service.verifyFee(badSignedTx); err != nil {
+		if !errors.Is(errZeroGasPriceTx, err) {
+			t.Fatal(err)
+		}
+	} else {
+		t.Fatal("err is nil")
+	}
+}
+
 // Pass true to set as a verifier
 func TestSyncServiceSync(t *testing.T) {
 	service, txCh, sub, err := newTestSyncService(true)