Fix retryable gas calculation bug and add ArbOS51 version (#4047) (#4068)

* l2pricing: add arbosVersion parameter

* create free-storage and use for gas constraints

* l2pricing-simulator: build fix

* common test: test by default with multiple constraints

* fix TestRetryableSubmissionAndRedeemFees

* lint fixes

* system_test: don't take ownership if no owner

* fix more system tests

* fix more tests

* Add test proving issue with redeem and constraints

* Refactor redeem gasPoolUpdateCost to a function

* Revert "create free-storage and use for gas constraints"

This reverts commit f48b38f.

* Correct GasPoolUpdateCost for ArbOS 51

* Increase redeemAllAndCreateAddresses gas for retryable

* recreate_rpc: do take ownership

* l2pricingstate: restructure arbos version checking

* l2pricingmodel: fixes

* rework gas constraints

* fix typo in test

* Post-merge fix: use gasConstraintsMaxNum

---------

Co-authored-by: Tsahi Zidenberg <[email protected]>

Author: MishkaRogachev

arbos/arbosState/arbosstate.go

@@ -85,7 +85,7 @@ func OpenArbosState(stateDB vm.StateDB, burner burn.Burner) (*ArbosState, error)
 		upgradeTimestamp:       backingStorage.OpenStorageBackedUint64(uint64(upgradeTimestampOffset)),
 		networkFeeAccount:      backingStorage.OpenStorageBackedAddress(uint64(networkFeeAccountOffset)),
 		l1PricingState:         l1pricing.OpenL1PricingState(backingStorage.OpenCachedSubStorage(l1PricingSubspace), arbosVersion),
-		l2PricingState:         l2pricing.OpenL2PricingState(backingStorage.OpenCachedSubStorage(l2PricingSubspace)),
+		l2PricingState:         l2pricing.OpenL2PricingState(backingStorage.OpenCachedSubStorage(l2PricingSubspace), arbosVersion),
 		retryableState:         retryables.OpenRetryableState(backingStorage.OpenCachedSubStorage(retryablesSubspace), stateDB),
 		addressTable:           addressTable.Open(backingStorage.OpenCachedSubStorage(addressTableSubspace)),
 		chainOwners:            addressSet.OpenAddressSet(backingStorage.OpenCachedSubStorage(chainOwnerSubspace)),
@@ -438,6 +438,10 @@ func (state *ArbosState) UpgradeArbosVersion(
 			ensure(p.UpgradeToArbosVersion(nextArbosVersion))
 			ensure(p.Save())
 			ensure(state.l2PricingState.SetMaxPerTxGasLimit(l2pricing.InitialPerTxGasLimitV50))
+
+		case params.ArbosVersion_51:
+			// nothing
+
 		default:
 			return fmt.Errorf(
 				"the chain is upgrading to unsupported ArbOS version %v, %w",
@@ -456,6 +460,7 @@ func (state *ArbosState) UpgradeArbosVersion(
 		state.arbosVersion = nextArbosVersion
 		state.programs.ArbosVersion = nextArbosVersion
 		state.l1PricingState.ArbosVersion = nextArbosVersion
+		state.l2PricingState.ArbosVersion = nextArbosVersion
 	}
 
 	if firstTime && upgradeTo >= params.ArbosVersion_6 {

arbos/internal_tx.go

@@ -104,7 +104,7 @@ func ApplyInternalTxUpdate(tx *types.ArbitrumInternalTx, state *arbosState.Arbos
 		_ = state.RetryableState().TryToReapOneRetryable(currentTime, evm, util.TracingDuringEVM)
 		_ = state.RetryableState().TryToReapOneRetryable(currentTime, evm, util.TracingDuringEVM)
 
-		state.L2PricingState().UpdatePricingModel(timePassed, state.ArbOSVersion())
+		state.L2PricingState().UpdatePricingModel(timePassed)
 
 		return state.UpgradeArbosVersionIfNecessary(currentTime, evm.StateDB, evm.ChainConfig())
 	case InternalTxBatchPostingReportMethodID:

arbos/l2pricing/l2pricing.go

@@ -74,6 +74,8 @@ type L2PricingState struct {
 	backlogTolerance    storage.StorageBackedUint64
 	perTxGasLimit       storage.StorageBackedUint64
 	constraints         *storage.SubStorageVector
+
+	ArbosVersion uint64
 }
 
 const (
@@ -90,6 +92,7 @@ const (
 var constraintsKey []byte = []byte{0}
 
 const GethBlockGasLimit = 1 << 50
+const gasConstraintsMaxNum = 20
 
 func InitializeL2PricingState(sto *storage.Storage) error {
 	_ = sto.SetUint64ByUint64(speedLimitPerSecondOffset, InitialSpeedLimitPerSecondV0)
@@ -101,7 +104,7 @@ func InitializeL2PricingState(sto *storage.Storage) error {
 	return sto.SetUint64ByUint64(minBaseFeeWeiOffset, InitialMinimumBaseFeeWei)
 }
 
-func OpenL2PricingState(sto *storage.Storage) *L2PricingState {
+func OpenL2PricingState(sto *storage.Storage, arbosVersion uint64) *L2PricingState {
 	return &L2PricingState{
 		storage:             sto,
 		speedLimitPerSecond: sto.OpenStorageBackedUint64(speedLimitPerSecondOffset),
@@ -113,6 +116,7 @@ func OpenL2PricingState(sto *storage.Storage) *L2PricingState {
 		backlogTolerance:    sto.OpenStorageBackedUint64(backlogToleranceOffset),
 		perTxGasLimit:       sto.OpenStorageBackedUint64(perTxGasLimitOffset),
 		constraints:         storage.OpenSubStorageVector(sto.OpenSubStorage(constraintsKey)),
+		ArbosVersion:        arbosVersion,
 	}
 }
 
@@ -254,3 +258,7 @@ func (ps *L2PricingState) ClearConstraints() error {
 	}
 	return nil
 }
+
+func (ps *L2PricingState) GasConstraintsMaxNum() int {
+	return gasConstraintsMaxNum
+}

arbos/l2pricing/l2pricing_test.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/params"
+
 	"github.com/offchainlabs/nitro/arbos/burn"
 	"github.com/offchainlabs/nitro/arbos/storage"
 	"github.com/offchainlabs/nitro/util/arbmath"
@@ -18,7 +20,7 @@ func PricingForTest(t *testing.T) *L2PricingState {
 	storage := storage.NewMemoryBacked(burn.NewSystemBurner(nil, false))
 	err := InitializeL2PricingState(storage)
 	Require(t, err)
-	return OpenL2PricingState(storage)
+	return OpenL2PricingState(storage, params.MaxDebugArbosVersionSupported)
 }
 
 func fakeBlockUpdate(t *testing.T, pricing *L2PricingState, gasUsed int64, timePassed uint64) {

arbos/l2pricing/model.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/params"
 
+	"github.com/offchainlabs/nitro/arbos/storage"
 	"github.com/offchainlabs/nitro/util/arbmath"
 )
 
@@ -24,8 +25,8 @@ const InitialPricingInertia = 102
 const InitialBacklogTolerance = 10
 const InitialPerTxGasLimitV50 uint64 = 32 * 1000000
 
-func (ps *L2PricingState) ShouldUseMultiConstraints(arbosVersion uint64) (bool, error) {
-	if arbosVersion >= ArbosMultiConstraintsVersion {
+func (ps *L2PricingState) ShouldUseGasConstraints() (bool, error) {
+	if ps.ArbosVersion >= ArbosMultiConstraintsVersion {
 		constraintsLength, err := ps.ConstraintsLength()
 		if err != nil {
 			return false, err
@@ -35,8 +36,8 @@ func (ps *L2PricingState) ShouldUseMultiConstraints(arbosVersion uint64) (bool,
 	return false, nil
 }
 
-func (ps *L2PricingState) AddToGasPool(gas int64, arbosVersion uint64) error {
-	shouldUseMultiConstraints, err := ps.ShouldUseMultiConstraints(arbosVersion)
+func (ps *L2PricingState) AddToGasPool(gas int64) error {
+	shouldUseMultiConstraints, err := ps.ShouldUseGasConstraints()
 	if err != nil {
 		return err
 	}
@@ -74,10 +75,32 @@ func (ps *L2PricingState) addToGasPoolMultiConstraints(gas int64) error {
 	return nil
 }
 
+func (ps *L2PricingState) GasPoolUpdateCost() uint64 {
+	result := storage.StorageReadCost + storage.StorageWriteCost
+
+	// Multi-Constraint pricer requires an extra storage read, since ArbOS must load the constraints from state.
+	// This overhead applies even when no constraints are configured.
+	if ps.ArbosVersion >= params.ArbosVersion_50 {
+		result += storage.StorageReadCost // read length for "souldUseGasConstraints"
+	}
+
+	if ps.ArbosVersion >= params.ArbosVersion_MultiConstraintFix {
+		// addToGasPoolWithGasConstraints costs (ArbOS 51 and later)
+		constraintsLength, _ := ps.constraints.Length()
+		if constraintsLength > 0 {
+			result += storage.StorageReadCost // read length to traverse
+			// updating (read+write) all constraints, first one was already accounted for
+			result += uint64(constraintsLength-1) * (storage.StorageReadCost + storage.StorageWriteCost)
+		}
+	}
+
+	return result
+}
+
 // UpdatePricingModel updates the pricing model with info from the last block
-func (ps *L2PricingState) UpdatePricingModel(timePassed uint64, arbosVersion uint64) {
-	shouldUseMultiConstraints, _ := ps.ShouldUseMultiConstraints(arbosVersion)
-	if shouldUseMultiConstraints {
+func (ps *L2PricingState) UpdatePricingModel(timePassed uint64) {
+	shouldUseGasConstraints, _ := ps.ShouldUseGasConstraints()
+	if shouldUseGasConstraints {
 		ps.updatePricingModelMultiConstraints(timePassed)
 	} else {
 		ps.updatePricingModelLegacy(timePassed)

arbos/tx_processor.go

@@ -632,7 +632,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
 			}
 		}
 		// we've already credited the network fee account, but we didn't charge the gas pool yet
-		p.state.Restrict(p.state.L2PricingState().AddToGasPool(-arbmath.SaturatingCast[int64](gasUsed), p.state.ArbOSVersion()))
+		p.state.Restrict(p.state.L2PricingState().AddToGasPool(-arbmath.SaturatingCast[int64](gasUsed)))
 		return
 	}
 
@@ -695,7 +695,7 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) {
 			log.Error("total gas used < poster gas component", "gasUsed", gasUsed, "posterGas", p.posterGas)
 			computeGas = gasUsed
 		}
-		p.state.Restrict(p.state.L2PricingState().AddToGasPool(-arbmath.SaturatingCast[int64](computeGas), p.state.ArbOSVersion()))
+		p.state.Restrict(p.state.L2PricingState().AddToGasPool(-arbmath.SaturatingCast[int64](computeGas)))
 	}
 }
 

cmd/chaininfo/arbitrum_chain_info.json

@@ -173,7 +173,7 @@
         "EnableArbOS": true,
         "AllowDebugPrecompiles": true,
         "DataAvailabilityCommittee": false,
-        "InitialArbOSVersion": 50,
+        "InitialArbOSVersion": 51,
         "InitialChainOwner": "0x0000000000000000000000000000000000000000",
         "GenesisBlockNum": 0
       }

cmd/l2pricing-simulator/main.go

@@ -20,13 +20,13 @@ import (
 
 var minBaseFee = big.NewInt(params.GWei)
 
-func newPricingState() (*l2pricing.L2PricingState, error) {
+func newPricingState(arbosVersion uint64) (*l2pricing.L2PricingState, error) {
 	storage := storage.NewMemoryBacked(burn.NewSystemBurner(nil, false))
 	err := l2pricing.InitializeL2PricingState(storage)
 	if err != nil {
 		return nil, err
 	}
-	pricing := l2pricing.OpenL2PricingState(storage)
+	pricing := l2pricing.OpenL2PricingState(storage, arbosVersion)
 	_ = pricing.SetMinBaseFeeWei(minBaseFee)
 	_ = pricing.SetBaseFeeWei(minBaseFee)
 	return pricing, nil
@@ -38,7 +38,7 @@ func runLegacyModel(args []string) error {
 		return err
 	}
 
-	pricing, err := newPricingState()
+	pricing, err := newPricingState(params.ArbosVersion_40)
 	if err != nil {
 		return err
 	}
@@ -53,8 +53,8 @@ func runLegacyModel(args []string) error {
 	for i := range config.Iterations() {
 		baseFee, _ := pricing.BaseFeeWei()
 		gas := gasSimulator.compute(i, baseFee)
-		_ = pricing.AddToGasPool(-arbmath.SaturatingCast[int64](gas), l2pricing.ArbosMultiConstraintsVersion)
-		pricing.UpdatePricingModel(1, l2pricing.ArbosMultiConstraintsVersion)
+		_ = pricing.AddToGasPool(-arbmath.SaturatingCast[int64](gas))
+		pricing.UpdatePricingModel(1)
 		baseFee, _ = pricing.BaseFeeWei()
 		results = append(results, Result{
 			baseFee:  baseFee,
@@ -72,7 +72,7 @@ func runConstraintsModel(args []string) error {
 		return err
 	}
 
-	pricing, err := newPricingState()
+	pricing, err := newPricingState(params.ArbosVersion_MultiConstraintFix)
 	if err != nil {
 		return err
 	}
@@ -102,8 +102,8 @@ func runConstraintsModel(args []string) error {
 	for i := range config.Iterations() {
 		baseFee, _ := pricing.BaseFeeWei()
 		gas := gasSimulator.compute(i, baseFee)
-		_ = pricing.AddToGasPool(-arbmath.SaturatingCast[int64](gas), l2pricing.ArbosMultiConstraintsVersion)
-		pricing.UpdatePricingModel(1, l2pricing.ArbosMultiConstraintsVersion)
+		_ = pricing.AddToGasPool(-arbmath.SaturatingCast[int64](gas))
+		pricing.UpdatePricingModel(1)
 		baseFee, _ = pricing.BaseFeeWei()
 		results = append(results, Result{
 			baseFee:  baseFee,

contracts-local/src/mocks/Simple.sol

@@ -64,7 +64,7 @@ contract Simple {
     ) external payable {
         for (uint256 i = 0; i < ticketIds.length; i++) {
             // Attempt to redeem each retryable ticket
-            try ArbRetryableTx(address(110)).redeem{gas: 100000}(ticketIds[i]) {
+            try ArbRetryableTx(address(110)).redeem{gas: 200000}(ticketIds[i]) {
                 // Ticket redeemed successfully
                 counter++;
             } catch {