Commit 3c56126c authored by Mark Tyneway's avatar Mark Tyneway Committed by Kelvin Fichter

l2geth: handle race condition for fee logic

Manually handle locking and unlocking to prevent race conditions
if the fee logic. The case that needs to be handled is as follows:
a `gas-oracle` transaction updates the gas price after an queue
origin sequencer tx is sent via RPC and already goes through the
fee check. This would cause the transaction to be accepted and then
fail during the state transition. This is bad because we do not
have logic to hold on to the failed transaction to execute later,
which is what the user would expect when sending transactions to
L1. All transactions that are sent to the miner *must* be valid
transactions.
parent 8500cb4a
---
'@eth-optimism/l2geth': patch
---
Handle race condition in L2 geth for fee logic
...@@ -758,6 +758,8 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error { ...@@ -758,6 +758,8 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
// Queue Origin L1 to L2 transactions must have a timestamp that is set by // Queue Origin L1 to L2 transactions must have a timestamp that is set by
// the L1 block that holds the transaction. This should never happen but is // the L1 block that holds the transaction. This should never happen but is
// a sanity check to prevent fraudulent execution. // a sanity check to prevent fraudulent execution.
// No need to unlock here as the lock is only taken when its a queue origin
// sequencer transaction.
if tx.QueueOrigin() == types.QueueOriginL1ToL2 { if tx.QueueOrigin() == types.QueueOriginL1ToL2 {
if tx.L1Timestamp() == 0 { if tx.L1Timestamp() == 0 {
return fmt.Errorf("Queue origin L1 to L2 transaction without a timestamp: %s", tx.Hash().Hex()) return fmt.Errorf("Queue origin L1 to L2 transaction without a timestamp: %s", tx.Hash().Hex())
...@@ -817,9 +819,20 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error { ...@@ -817,9 +819,20 @@ func (s *SyncService) applyTransactionToTip(tx *types.Transaction) error {
owner := s.GasPriceOracleOwnerAddress() owner := s.GasPriceOracleOwnerAddress()
if owner != nil && sender == *owner { if owner != nil && sender == *owner {
if err := s.updateGasPriceOracleCache(nil); err != nil { if err := s.updateGasPriceOracleCache(nil); err != nil {
if tx.QueueOrigin() == types.QueueOriginSequencer {
s.txLock.Unlock()
}
return err return err
} }
} }
// Unlock the txLock after the transaction is added to the chain.
// This is locked up the stack on incoming queue origin sequencer
// transactions. For L1 to L2 transactions, there are no race
// conditions with the fees
if tx.QueueOrigin() == types.QueueOriginSequencer {
s.txLock.Unlock()
}
return nil return nil
} }
...@@ -928,21 +941,30 @@ func (s *SyncService) ValidateAndApplySequencerTransaction(tx *types.Transaction ...@@ -928,21 +941,30 @@ func (s *SyncService) ValidateAndApplySequencerTransaction(tx *types.Transaction
if tx == nil { if tx == nil {
return errors.New("nil transaction passed to ValidateAndApplySequencerTransaction") return errors.New("nil transaction passed to ValidateAndApplySequencerTransaction")
} }
// Grab the txLock. This must be unlocked manually in all error cases
// to prevent race conditions with updating the gas prices. In the happy
// path case, it is unlocked after the transaction is confirmed in the chain
s.txLock.Lock()
if err := s.verifyFee(tx); err != nil { if err := s.verifyFee(tx); err != nil {
s.txLock.Unlock()
return err return err
} }
s.txLock.Lock()
defer s.txLock.Unlock()
log.Trace("Sequencer transaction validation", "hash", tx.Hash().Hex()) log.Trace("Sequencer transaction validation", "hash", tx.Hash().Hex())
qo := tx.QueueOrigin() qo := tx.QueueOrigin()
if qo != types.QueueOriginSequencer { if qo != types.QueueOriginSequencer {
return fmt.Errorf("invalid transaction with queue origin %d", qo) s.txLock.Unlock()
return fmt.Errorf("invalid transaction with queue origin %s", qo.String())
} }
if err := s.txpool.ValidateTx(tx); err != nil { if err := s.txpool.ValidateTx(tx); err != nil {
s.txLock.Unlock()
return fmt.Errorf("invalid transaction: %w", err) return fmt.Errorf("invalid transaction: %w", err)
} }
return s.applyTransaction(tx) if err := s.applyTransaction(tx); err != nil {
s.txLock.Unlock()
return err
}
return nil
} }
// syncer represents a function that can sync remote items and then returns the // syncer represents a function that can sync remote items and then returns the
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment