Skip to content

Commit

Permalink
[R4R]-[fee]fix: fix estimateGas; discard invalid tx (#67)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tri-stone authored Mar 13, 2024
1 parent db24062 commit 4d49b52
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 13 deletions.
8 changes: 8 additions & 0 deletions core/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ var (
// than required to start the invocation.
ErrIntrinsicGas = errors.New("intrinsic gas too low")

// ErrInsufficientGasForL1Cost is returned if the transaction is specified to use less gas
// than required for l1Cost.
ErrInsufficientGasForL1Cost = errors.New("insufficient gas for l1Cost. Please use estimateGas to get gasLimit")

// ErrTxTypeNotSupported is returned if a transaction is not supported in the
// current network configuration.
ErrTxTypeNotSupported = types.ErrTxTypeNotSupported
Expand All @@ -98,6 +102,10 @@ var (
// base fee of the block.
ErrFeeCapTooLow = errors.New("max fee per gas less than block base fee")

// ErrGasPriceTooLow is returned if the transaction gasPrice is less than the
// base fee of the block for legacy tx
ErrGasPriceTooLow = errors.New("legacy tx's gasPrice less than block base fee")

// ErrSenderNoEOA is returned if the sender of a transaction is a contract.
ErrSenderNoEOA = errors.New("sender not an eoa")

Expand Down
6 changes: 2 additions & 4 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ func (st *StateTransition) innerTransitionDb() (*ExecutionResult, error) {
if err != nil {
return nil, err
}

if !st.msg.IsDepositTx && !st.msg.IsSystemTx {
gas = gas * tokenRatio
}
Expand All @@ -535,12 +536,9 @@ func (st *StateTransition) innerTransitionDb() (*ExecutionResult, error) {
if !st.msg.IsDepositTx && !st.msg.IsSystemTx {
if st.msg.GasPrice.Cmp(common.Big0) > 0 && l1Cost != nil {
l1Gas = new(big.Int).Div(l1Cost, st.msg.GasPrice).Uint64()
if st.msg.GasLimit < l1Gas {
return nil, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gasRemaining, l1Gas)
}
}
if st.gasRemaining < l1Gas {
return nil, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gasRemaining, l1Gas)
return nil, fmt.Errorf("%w: have %d, want %d", ErrInsufficientGasForL1Cost, st.gasRemaining, l1Gas)
}
st.gasRemaining -= l1Gas
if tokenRatio > 0 {
Expand Down
37 changes: 31 additions & 6 deletions core/txpool/txpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

"github.com/ethereum/go-ethereum/common"
cmath "github.com/ethereum/go-ethereum/common/math"
"github.com/ethereum/go-ethereum/common/prque"
"github.com/ethereum/go-ethereum/consensus/misc"
"github.com/ethereum/go-ethereum/core"
Expand Down Expand Up @@ -688,7 +689,8 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
// Transactor should have enough funds to cover the costs
// cost == V + GP * GL
cost := tx.Cost()
if l1Cost := pool.l1CostFn(tx.RollupDataGas(), tx.IsDepositTx(), tx.To()); l1Cost != nil { // add rollup cost
var l1Cost *big.Int
if l1Cost = pool.l1CostFn(tx.RollupDataGas(), tx.IsDepositTx(), tx.To()); l1Cost != nil { // add rollup cost
cost = cost.Add(cost, l1Cost)
}

Expand Down Expand Up @@ -731,9 +733,9 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
sum := new(big.Int).Add(cost, list.totalcost)
if repl := list.txs.Get(tx.Nonce()); repl != nil {
// Deduct the cost of a transaction replaced by this
replL1Cost := repl.Cost()
if l1Cost := pool.l1CostFn(tx.RollupDataGas(), tx.IsDepositTx(), tx.To()); l1Cost != nil { // add rollup cost
replL1Cost = replL1Cost.Add(cost, l1Cost)
replCost := repl.Cost()
if replL1Cost := pool.l1CostFn(repl.RollupDataGas(), repl.IsDepositTx(), repl.To()); replL1Cost != nil { // add rollup cost
replCost = replCost.Add(cost, replL1Cost)
}
replMetaTxParams, err := types.DecodeAndVerifyMetaTxParams(repl, pool.chainconfig.IsMetaTxV2(pool.chain.CurrentBlock().Time))
if err != nil {
Expand All @@ -742,9 +744,9 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
if replMetaTxParams != nil {
replTxGasCost := new(big.Int).Sub(repl.Cost(), repl.Value())
sponsorAmount, _ := types.CalculateSponsorPercentAmount(replMetaTxParams, replTxGasCost)
replL1Cost = new(big.Int).Sub(replL1Cost, sponsorAmount)
replCost = new(big.Int).Sub(replCost, sponsorAmount)
}
sum.Sub(sum, replL1Cost)
sum.Sub(sum, replCost)
}
if userBalance.Cmp(sum) < 0 {
log.Trace("Replacing transactions would overdraft", "sender", from, "balance", userBalance, "required", sum)
Expand All @@ -762,6 +764,29 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
if tx.Gas() < intrGas*tokenRatio {
return core.ErrIntrinsicGas
}

gasRemaining := big.NewInt(int64(tx.Gas() - intrGas*tokenRatio))
baseFee := pool.chain.CurrentBlock().BaseFee

if tx.Type() == types.LegacyTxType {
if tx.GasPrice().Cmp(baseFee) < 0 {
return core.ErrGasPriceTooLow
}

// legacyTxL1Cost gas used to cover L1 Cost for legacy tx
legacyTxL1Cost := new(big.Int).Mul(tx.GasPrice(), gasRemaining)
if l1Cost != nil && legacyTxL1Cost.Cmp(l1Cost) <= 0 {
return core.ErrInsufficientGasForL1Cost
}
} else if tx.Type() == types.DynamicFeeTxType {
// dynamicBaseFeeTxL1Cost gas used to cover L1 Cost for dynamic fee tx
effectiveGas := cmath.BigMin(new(big.Int).Add(tx.GasTipCap(), baseFee), tx.GasFeeCap())
dynamicFeeTxL1Cost := new(big.Int).Mul(effectiveGas, gasRemaining)
if l1Cost != nil && dynamicFeeTxL1Cost.Cmp(l1Cost) <= 0 {
return core.ErrInsufficientGasForL1Cost
}
}

return nil
}

Expand Down
6 changes: 5 additions & 1 deletion internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,10 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
feeCap = gasPriceForEstimateGas
}

if feeCap.Cmp(gasPriceForEstimateGas) < 0 {
feeCap = gasPriceForEstimateGas
}

runMode := core.GasEstimationMode
if args.GasPrice == nil && args.MaxFeePerGas == nil && args.MaxPriorityFeePerGas == nil {
runMode = core.GasEstimationWithSkipCheckBalanceMode
Expand Down Expand Up @@ -1309,7 +1313,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr

result, err := DoCall(ctx, b, args, blockNrOrHash, nil, 0, gasCap, runMode, (*hexutil.Big)(gasPriceForEstimateGas))
if err != nil {
if errors.Is(err, core.ErrIntrinsicGas) {
if errors.Is(err, core.ErrIntrinsicGas) || errors.Is(err, core.ErrInsufficientGasForL1Cost) {
return true, nil, nil // Special case, raise gas limit
}
return true, nil, err // Bail out
Expand Down
7 changes: 7 additions & 0 deletions internal/ethapi/transaction_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@ func (args *TransactionArgs) ToMessage(globalGasCap uint64, baseFee *big.Int, ru
gasFeeCap = gasPriceForEstimate.ToInt()
gasTipCap = gasPriceForEstimate.ToInt()
}

if gasPrice.Cmp(gasPriceForEstimate.ToInt()) < 0 {
gasPrice = gasPriceForEstimate.ToInt()
}
if gasFeeCap.Cmp(gasPriceForEstimate.ToInt()) < 0 {
gasFeeCap = gasPriceForEstimate.ToInt()
}
}

value := new(big.Int)
Expand Down
4 changes: 2 additions & 2 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,8 +986,8 @@ func (w *worker) commitTransactions(env *environment, txs *types.TransactionsByP
default:
// Strange error, discard the transaction and get the next in line (note, the
// nonce-too-high clause will prevent us from executing in vain).
log.Debug("Transaction failed, account skipped", "hash", tx.Hash(), "err", err)
txs.Shift()
log.Info("Transaction failed, account skipped", "hash", tx.Hash(), "err", err)
txs.Pop()
}
}
if !w.isRunning() && len(coalescedLogs) > 0 {
Expand Down

0 comments on commit 4d49b52

Please sign in to comment.