Skip to content

Commit cf1867f

Browse files
Review fixes: use max base fee as a refund backup
1 parent 2af4015 commit cf1867f

File tree

3 files changed

+31
-43
lines changed

3 files changed

+31
-43
lines changed

arbos/l2pricing/model.go

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -352,33 +352,21 @@ func (ps *L2PricingState) calcBaseFeeFromExponent(exponent arbmath.Bips) (*big.I
352352
}
353353

354354
func (ps *L2PricingState) MultiDimensionalPriceForRefund(gasUsed multigas.MultiGas) (*big.Int, error) {
355-
if ps.ArbosVersion < ArbosMultiGasConstraintsVersion {
356-
baseFeeWei, err := ps.BaseFeeWei()
357-
if err != nil {
358-
return nil, err
359-
}
360-
361-
return new(big.Int).Mul(
362-
new(big.Int).SetUint64(gasUsed.SingleGas()),
363-
baseFeeWei,
364-
), nil
365-
}
366-
367-
total := new(big.Int)
368-
minBaseFeeWei, err := ps.MinBaseFeeWei()
355+
// Base fee is max of per-resource-kind base fees
356+
baseFeeWei, err := ps.BaseFeeWei()
369357
if err != nil {
370358
return nil, err
371359
}
372360

361+
total := new(big.Int)
373362
for kind := range multigas.ResourceKind(multigas.NumResourceKind) {
374363
baseFee, err := ps.multiGasBaseFees.Get(kind)
375364
if err != nil {
376365
return nil, err
377366
}
378-
// Override zero with minimum base fee,
379-
// it may happen if there is no constraint configured for this resource kind
380-
if baseFee.Cmp(big.NewInt(0)) == 0 {
381-
baseFee = minBaseFeeWei
367+
// Force L1 calldata (and the unlikely zero-basefee case) to use the max base fee.
368+
if kind == multigas.ResourceKindL1Calldata || baseFee.Cmp(big.NewInt(0)) == 0 {
369+
baseFee = baseFeeWei
382370
}
383371

384372
amount := gasUsed.Get(kind)

arbos/l2pricing/model_test.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -200,17 +200,10 @@ func TestMultiDimensionalPriceForRefund(t *testing.T) {
200200
expectedPrice := minPrice.Mul(minPrice, singleGas)
201201
Require(t, err)
202202

203-
// Should use base fee under ArbosMultiGasConstraintsVersion
204-
price, err := pricing.MultiDimensionalPriceForRefund(multiGas)
205-
Require(t, err)
206-
if price.Cmp(expectedPrice) != 0 {
207-
t.Errorf("Unexpected initial price: got %v, want %v", price, expectedPrice)
208-
}
209-
210203
pricing.ArbosVersion = ArbosMultiGasConstraintsVersion
211204

212-
// Should still use base fee before any multi gas constraints are set
213-
price, err = pricing.MultiDimensionalPriceForRefund(multiGas)
205+
// Initial price check
206+
price, err := pricing.MultiDimensionalPriceForRefund(multiGas)
214207
Require(t, err)
215208
if price.Cmp(expectedPrice) != 0 {
216209
t.Errorf("Unexpected initial price: got %v, want %v", price, expectedPrice)

arbos/tx_processor.go

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"github.com/offchainlabs/nitro/arbos/arbosState"
2323
"github.com/offchainlabs/nitro/arbos/l1pricing"
24+
"github.com/offchainlabs/nitro/arbos/l2pricing"
2425
"github.com/offchainlabs/nitro/arbos/retryables"
2526
"github.com/offchainlabs/nitro/arbos/util"
2627
"github.com/offchainlabs/nitro/util/arbmath"
@@ -534,8 +535,12 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, usedMultiGas multigas.MultiGas,
534535
}
535536
gasUsed := p.msg.GasLimit - gasLeft
536537

537-
multiDimensionalCost, err := p.state.L2PricingState().MultiDimensionalPriceForRefund(usedMultiGas)
538-
p.state.Restrict(err)
538+
var multiDimensionalCost *big.Int
539+
var err error
540+
if p.state.L2PricingState().ArbosVersion >= l2pricing.ArbosMultiGasConstraintsVersion {
541+
multiDimensionalCost, err = p.state.L2PricingState().MultiDimensionalPriceForRefund(usedMultiGas)
542+
p.state.Restrict(err)
543+
}
539544

540545
if underlyingTx != nil && underlyingTx.Type() == types.ArbitrumRetryTxType {
541546
inner, _ := underlyingTx.GetInner().(*types.ArbitrumRetryTx)
@@ -559,7 +564,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, usedMultiGas multigas.MultiGas,
559564
}
560565

561566
singleGasCost := arbmath.BigMulByUint(effectiveBaseFee, gasUsed)
562-
shouldRefundMultiGas := arbmath.BigGreaterThan(singleGasCost, multiDimensionalCost)
567+
shouldRefundMultiGas := multiDimensionalCost != nil && arbmath.BigGreaterThan(singleGasCost, multiDimensionalCost)
563568

564569
maxRefund := new(big.Int).Set(inner.MaxRefund)
565570
refund := func(refundFrom common.Address, amount *big.Int, reason tracing.BalanceChangeReason) {
@@ -703,19 +708,21 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, usedMultiGas multigas.MultiGas,
703708
}
704709

705710
// Multi-dimensional refund (normal tx path)
706-
amount := new(big.Int).Sub(totalCost, multiDimensionalCost)
707-
if amount.Sign() > 0 {
708-
err := util.TransferBalance(
709-
&networkFeeAccount,
710-
&p.msg.From,
711-
amount,
712-
p.evm,
713-
scenario,
714-
tracing.BalanceChangeMultiGasRefund,
715-
)
716-
p.state.Restrict(err)
717-
} else if amount.Sign() < 0 {
718-
log.Warn("multi dimensional gas price exceeded simple gas price", "amount", amount)
711+
if multiDimensionalCost != nil {
712+
amount := new(big.Int).Sub(totalCost, multiDimensionalCost)
713+
if amount.Sign() > 0 {
714+
err := util.TransferBalance(
715+
&networkFeeAccount,
716+
&p.msg.From,
717+
amount,
718+
p.evm,
719+
scenario,
720+
tracing.BalanceChangeMultiGasRefund,
721+
)
722+
p.state.Restrict(err)
723+
} else if amount.Sign() < 0 {
724+
log.Warn("multi dimensional gas price exceeded simple gas price", "amount", amount)
725+
}
719726
}
720727

721728
if p.msg.GasPrice.Sign() > 0 { // in tests, gas price could be 0

0 commit comments

Comments
 (0)