Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix retryable gas estimation when overriding gas price to zero #1929

Merged
merged 5 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 35 additions & 17 deletions arbos/tx_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,17 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r
}

balance := statedb.GetBalance(tx.From)
basefee := evm.Context.BaseFee
effectiveBaseFee := evm.Context.BaseFee
usergas := p.msg.GasLimit

maxGasCost := arbmath.BigMulByUint(tx.GasFeeCap, usergas)
maxFeePerGasTooLow := arbmath.BigLessThan(tx.GasFeeCap, basefee)
if p.msg.TxRunMode == core.MessageGasEstimationMode && tx.GasFeeCap.BitLen() == 0 {
// In gas estimation mode, we permit a zero gas fee cap.
// This matches behavior with normal tx gas estimation.
maxFeePerGasTooLow = false
if p.msg.TxRunMode != core.MessageCommitMode && p.msg.GasFeeCap.BitLen() == 0 {
// In gas estimation or eth_call mode, we permit a zero gas fee cap.
// This matches behavior with normal tx gas estimation and eth_call.
effectiveBaseFee = common.Big0
}

maxGasCost := arbmath.BigMulByUint(tx.GasFeeCap, usergas)
maxFeePerGasTooLow := arbmath.BigLessThan(tx.GasFeeCap, effectiveBaseFee)
if arbmath.BigLessThan(balance, maxGasCost) || usergas < params.TxGas || maxFeePerGasTooLow {
// User either specified too low of a gas fee cap, didn't have enough balance to pay for gas,
// or the specified gas limit is below the minimum transaction gas cost.
Expand All @@ -268,15 +269,15 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r
}

// pay for the retryable's gas and update the pools
gascost := arbmath.BigMulByUint(basefee, usergas)
gascost := arbmath.BigMulByUint(effectiveBaseFee, usergas)
networkCost := gascost
if p.state.ArbOSVersion() >= 11 {
infraFeeAccount, err := p.state.InfraFeeAccount()
p.state.Restrict(err)
if infraFeeAccount != (common.Address{}) {
minBaseFee, err := p.state.L2PricingState().MinBaseFeeWei()
p.state.Restrict(err)
infraFee := arbmath.BigMin(minBaseFee, basefee)
infraFee := arbmath.BigMin(minBaseFee, effectiveBaseFee)
infraCost := arbmath.BigMulByUint(infraFee, usergas)
infraCost = takeFunds(networkCost, infraCost)
if err := transfer(&tx.From, &infraFeeAccount, infraCost); err != nil {
Expand All @@ -294,7 +295,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r
}

withheldGasFunds := takeFunds(availableRefund, gascost) // gascost is conceptually charged before the gas price refund
gasPriceRefund := arbmath.BigMulByUint(arbmath.BigSub(tx.GasFeeCap, basefee), tx.Gas)
gasPriceRefund := arbmath.BigMulByUint(arbmath.BigSub(tx.GasFeeCap, effectiveBaseFee), tx.Gas)
if gasPriceRefund.Sign() < 0 {
// This should only be possible during gas estimation mode
gasPriceRefund.SetInt64(0)
Expand All @@ -310,7 +311,7 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r
retryTxInner, err := retryable.MakeTx(
underlyingTx.ChainId(),
0,
basefee,
effectiveBaseFee,
usergas,
ticketId,
tx.FeeRefundAddr,
Expand Down Expand Up @@ -456,7 +457,6 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {

underlyingTx := p.msg.Tx
networkFeeAccount, _ := p.state.NetworkFeeAccount()
basefee := p.evm.Context.BaseFee
scenario := util.TracingAfterEVM

if gasLeft > p.msg.GasLimit {
Expand All @@ -466,9 +466,20 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {

if underlyingTx != nil && underlyingTx.Type() == types.ArbitrumRetryTxType {
inner, _ := underlyingTx.GetInner().(*types.ArbitrumRetryTx)
effectiveBaseFee := inner.GasFeeCap
if p.msg.TxRunMode == core.MessageCommitMode && !arbmath.BigEquals(effectiveBaseFee, p.evm.Context.BaseFee) {
log.Error(
"ArbitrumRetryTx GasFeeCap doesn't match basefee in commit mode",
"txHash", underlyingTx.Hash(),
"gasFeeCap", inner.GasFeeCap,
"baseFee", p.evm.Context.BaseFee,
)
// revert to the old behavior to avoid diverging from older nodes
effectiveBaseFee = p.evm.Context.BaseFee
}

// undo Geth's refund to the From address
gasRefund := arbmath.BigMulByUint(basefee, gasLeft)
gasRefund := arbmath.BigMulByUint(effectiveBaseFee, gasLeft)
err := util.BurnBalance(&inner.From, gasRefund, p.evm, scenario, "undoRefund")
if err != nil {
log.Error("Uh oh, Geth didn't refund the user", inner.From, gasRefund)
Expand Down Expand Up @@ -503,7 +514,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
takeFunds(maxRefund, inner.SubmissionFeeRefund)
}
// Conceptually, the gas charge is taken from the L1 deposit pool if possible.
takeFunds(maxRefund, arbmath.BigMulByUint(basefee, gasUsed))
takeFunds(maxRefund, arbmath.BigMulByUint(effectiveBaseFee, gasUsed))
// Refund any unused gas, without overdrafting the L1 deposit.
networkRefund := gasRefund
if p.state.ArbOSVersion() >= 11 {
Expand All @@ -513,7 +524,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
minBaseFee, err := p.state.L2PricingState().MinBaseFeeWei()
p.state.Restrict(err)
// TODO MinBaseFeeWei change during RetryTx execution may cause incorrect calculation of the part of the refund that should be taken from infraFeeAccount. Unless the balances of network and infra fee accounts are too low, the amount transferred to refund address should remain correct.
infraFee := arbmath.BigMin(minBaseFee, basefee)
infraFee := arbmath.BigMin(minBaseFee, effectiveBaseFee)
infraRefund := arbmath.BigMulByUint(infraFee, gasLeft)
infraRefund = takeFunds(networkRefund, infraRefund)
refund(infraFeeAccount, infraRefund)
Expand Down Expand Up @@ -541,6 +552,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
return
}

basefee := p.evm.Context.BaseFee
totalCost := arbmath.BigMul(basefee, arbmath.UintToBig(gasUsed)) // total cost = price of gas * gas burnt
computeCost := arbmath.BigSub(totalCost, p.PosterFee) // total cost = network's compute + poster's L1 costs
if computeCost.Sign() < 0 {
Expand Down Expand Up @@ -602,9 +614,15 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
func (p *TxProcessor) ScheduledTxes() types.Transactions {
scheduled := types.Transactions{}
time := p.evm.Context.Time
basefee := p.evm.Context.BaseFee
effectiveBaseFee := p.evm.Context.BaseFee
chainID := p.evm.ChainConfig().ChainID

if p.msg.TxRunMode != core.MessageCommitMode && p.msg.GasFeeCap.BitLen() == 0 {
// In gas estimation or eth_call mode, we permit a zero gas fee cap.
// This matches behavior with normal tx gas estimation and eth_call.
effectiveBaseFee = common.Big0
}

logs := p.evm.StateDB.GetCurrentTxLogs()
for _, log := range logs {
if log.Address != ArbRetryableTxAddress || log.Topics[0] != RedeemScheduledEventID {
Expand All @@ -623,7 +641,7 @@ func (p *TxProcessor) ScheduledTxes() types.Transactions {
redeem, _ := retryable.MakeTx(
chainID,
event.SequenceNum,
basefee,
effectiveBaseFee,
event.DonatedGas,
event.TicketId,
event.GasDonor,
Expand Down
2 changes: 1 addition & 1 deletion go-ethereum
Loading