diff --git a/.changeset/forty-pigs-study.md b/.changeset/forty-pigs-study.md
new file mode 100644
index 0000000000000000000000000000000000000000..122311dc65431bfed9fb5b7b402e941cdc2acadc
--- /dev/null
+++ b/.changeset/forty-pigs-study.md
@@ -0,0 +1,5 @@
+---
+'@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
diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go
index e959219d0539bb355dd30df22bafa2bcd509ea96..d240a12fbc294c811d31b8d85ef70282fb3c24d6 100644
--- a/l2geth/rollup/sync_service.go
+++ b/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)
diff --git a/l2geth/rollup/sync_service_test.go b/l2geth/rollup/sync_service_test.go
index 448f7ff3bc3c54429dce2ea6e95d208d57336eb8..8d5b57dfd16a7d6ebe23c9a6dc8f639d05c60530 100644
--- a/l2geth/rollup/sync_service_test.go
+++ b/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)