From cdfd513e5fad42a6fe50c2436ca130738885566d Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 31 Aug 2022 19:38:35 -0500 Subject: [PATCH 1/4] Improve data cost estimation when full tx hurts compression ratio --- arbos/l1pricing/l1pricing.go | 62 +++++++++++++++++++++++-------- arbos/l1pricing/l1pricing_test.go | 38 ------------------- precompiles/ArbGasInfo.go | 10 +++-- util/arbmath/bips.go | 4 ++ 4 files changed, 57 insertions(+), 57 deletions(-) diff --git a/arbos/l1pricing/l1pricing.go b/arbos/l1pricing/l1pricing.go index 6ada92d547..29e6b4c18f 100644 --- a/arbos/l1pricing/l1pricing.go +++ b/arbos/l1pricing/l1pricing.go @@ -4,12 +4,14 @@ package l1pricing import ( + "encoding/binary" "errors" "fmt" "math/big" "sync/atomic" "github.com/ethereum/go-ethereum/common/math" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/params" @@ -645,27 +647,57 @@ func (ps *L1PricingState) GetPosterInfo(tx *types.Transaction, poster common.Add return am.BigMulByUint(pricePerUnit, units), units } -const TxFixedCostEstimate = 140 // assumed maximum size in bytes of a typical RLP-encoded tx, not including its calldata +// We don't have the full tx in gas estimation, so we assume it might be a bit bigger in practice. +const estimationPaddingUnits = 16 * params.TxDataNonZeroGasEIP2028 +const estimationPaddingBasisPoints = 100 + +var randomNonce = binary.BigEndian.Uint64(crypto.Keccak256([]byte("Nonce"))[:8]) +var randomGasTipCap = new(big.Int).SetBytes(crypto.Keccak256([]byte("GasTipCap"))[:4]) +var randomGasFeeCap = new(big.Int).SetBytes(crypto.Keccak256([]byte("GasFeeCap"))[:4]) +var randV = arbmath.BigMulByUint(params.ArbitrumOneChainConfig().ChainID, 3) +var randR = crypto.Keccak256Hash([]byte("R")).Big() +var randS = crypto.Keccak256Hash([]byte("S")).Big() + +func makeFakeTxForMessage(message core.Message) *types.Transaction { + nonce := message.Nonce() + if nonce == 0 { + nonce = randomNonce + } + gasTipCap := message.GasTipCap() + if nonce == 0 { + gasTipCap = randomGasTipCap + } + gasFeeCap := message.GasFeeCap() + if nonce == 0 { + gasFeeCap = randomGasFeeCap + } + return types.NewTx(&types.DynamicFeeTx{ + Nonce: nonce, + GasTipCap: gasTipCap, + GasFeeCap: gasFeeCap, + Gas: message.Gas(), + To: message.To(), + Value: message.Value(), + Data: message.Data(), + AccessList: message.AccessList(), + V: randV, + R: randR, + S: randS, + }) +} func (ps *L1PricingState) PosterDataCost(message core.Message, poster common.Address) (*big.Int, uint64) { - if tx := message.UnderlyingTransaction(); tx != nil { + tx := message.UnderlyingTransaction() + if tx != nil { return ps.GetPosterInfo(tx, poster) } - if poster != BatchPosterAddress { - return common.Big0, 0 - } - - byteCount, err := byteCountAfterBrotli0(message.Data()) - if err != nil { - panic(fmt.Sprintf("failed to compress tx: %v", err)) - } - - // Approximate the l1 fee charged for posting this tx's calldata - l1Bytes := byteCount + TxFixedCostEstimate + // Otherwise, we don't have an underlying transaction, so we're likely in gas estimation. + // We'll instead make a fake tx from the message info we do have, and then pad our cost a bit to be safe. + tx = makeFakeTxForMessage(message) + units := ps.getPosterUnitsWithoutCache(tx, poster) + units = arbmath.UintMulByBips(units+estimationPaddingUnits, arbmath.OneInBips+estimationPaddingBasisPoints) pricePerUnit, _ := ps.PricePerUnit() - - units := l1Bytes * params.TxDataNonZeroGasEIP2028 return am.BigMulByUint(pricePerUnit, units), units } diff --git a/arbos/l1pricing/l1pricing_test.go b/arbos/l1pricing/l1pricing_test.go index aeae5aeea4..ec0ecc275e 100644 --- a/arbos/l1pricing/l1pricing_test.go +++ b/arbos/l1pricing/l1pricing_test.go @@ -4,53 +4,15 @@ package l1pricing import ( - "math" - "math/big" "testing" am "github.com/offchainlabs/nitro/util/arbmath" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/params" "github.com/offchainlabs/nitro/arbos/burn" "github.com/offchainlabs/nitro/arbos/storage" ) -func TestTxFixedCost(t *testing.T) { - maxChainId := am.UintToBig(math.MaxUint64) - maxValue := big.NewInt(1_000_000) - maxValue.Mul(maxValue, big.NewInt(params.Ether)) - var address common.Address - for i := range address { - address[i] = 0xFF - } - maxSigVal := big.NewInt(2) - maxSigVal.Exp(maxSigVal, big.NewInt(256), nil) - maxSigVal.Sub(maxSigVal, common.Big1) - maxGasPrice := big.NewInt(1000 * params.GWei) - largeTx := types.NewTx(&types.DynamicFeeTx{ - ChainID: maxChainId, - Nonce: 1 << 32, - GasTipCap: maxGasPrice, - GasFeeCap: maxGasPrice, - Gas: 100_000_000, - To: &address, - Value: maxValue, - Data: []byte{}, - AccessList: []types.AccessTuple{}, - V: common.Big1, - R: maxSigVal, - S: maxSigVal, - }) - largeTxEncoded, err := largeTx.MarshalBinary() - Require(t, err) - - if len(largeTxEncoded) > TxFixedCostEstimate { - Fail(t, "large tx is", len(largeTxEncoded), "bytes but tx fixed cost is", TxFixedCostEstimate) - } -} - func TestL1PriceUpdate(t *testing.T) { sto := storage.NewMemoryBacked(burn.NewSystemBurner(nil, false)) err := InitializeL1PricingState(sto, common.Address{}) diff --git a/precompiles/ArbGasInfo.go b/precompiles/ArbGasInfo.go index b759a50ca2..21fb504c52 100644 --- a/precompiles/ArbGasInfo.go +++ b/precompiles/ArbGasInfo.go @@ -20,6 +20,8 @@ type ArbGasInfo struct { var storageArbGas = big.NewInt(int64(storage.StorageWriteCost)) +const AssumedSimpleTxSize = 140 + // Get prices in wei when using the provided aggregator func (con ArbGasInfo) GetPricesInWeiWithAggregator( c ctx, @@ -40,7 +42,7 @@ func (con ArbGasInfo) GetPricesInWeiWithAggregator( weiForL1Calldata := arbmath.BigMulByUint(l1GasPrice, params.TxDataNonZeroGasEIP2028) // the cost of a simple tx without calldata - perL2Tx := arbmath.BigMulByUint(weiForL1Calldata, l1pricing.TxFixedCostEstimate) + perL2Tx := arbmath.BigMulByUint(weiForL1Calldata, AssumedSimpleTxSize) // nitro's compute-centric l2 gas pricing has no special compute component that rises independently perArbGasBase, err := c.State.L2PricingState().MinBaseFeeWei() @@ -73,7 +75,7 @@ func (con ArbGasInfo) _preVersion4_GetPricesInWeiWithAggregator( weiForL1Calldata := arbmath.BigMulByUint(l1GasPrice, params.TxDataNonZeroGasEIP2028) // the cost of a simple tx without calldata - perL2Tx := arbmath.BigMulByUint(weiForL1Calldata, l1pricing.TxFixedCostEstimate) + perL2Tx := arbmath.BigMulByUint(weiForL1Calldata, AssumedSimpleTxSize) // nitro's compute-centric l2 gas pricing has no special compute component that rises independently perArbGasBase := l2GasPrice @@ -103,7 +105,7 @@ func (con ArbGasInfo) GetPricesInArbGasWithAggregator(c ctx, evm mech, aggregato // aggregators compress calldata, so we must estimate accordingly weiForL1Calldata := arbmath.BigMulByUint(l1GasPrice, params.TxDataNonZeroGasEIP2028) - weiPerL2Tx := arbmath.BigMulByUint(weiForL1Calldata, l1pricing.TxFixedCostEstimate) + weiPerL2Tx := arbmath.BigMulByUint(weiForL1Calldata, AssumedSimpleTxSize) gasForL1Calldata := common.Big0 gasPerL2Tx := common.Big0 if l2GasPrice.Sign() > 0 { @@ -128,7 +130,7 @@ func (con ArbGasInfo) _preVersion4_GetPricesInArbGasWithAggregator(c ctx, evm me gasForL1Calldata = arbmath.BigDiv(weiForL1Calldata, l2GasPrice) } - perL2Tx := big.NewInt(l1pricing.TxFixedCostEstimate) + perL2Tx := big.NewInt(AssumedSimpleTxSize) return perL2Tx, gasForL1Calldata, storageArbGas, nil } diff --git a/util/arbmath/bips.go b/util/arbmath/bips.go index 7b44556612..1e788df064 100644 --- a/util/arbmath/bips.go +++ b/util/arbmath/bips.go @@ -29,6 +29,10 @@ func IntMulByBips(value int64, bips Bips) int64 { return value * int64(bips) / int64(OneInBips) } +func UintMulByBips(value uint64, bips Bips) uint64 { + return value * uint64(bips) / uint64(OneInBips) +} + func SaturatingCastToBips(value uint64) Bips { return Bips(SaturatingCast(value)) } From 36db71f6d2d535f496ca17407ca5501a7e118fd2 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 31 Aug 2022 19:45:23 -0500 Subject: [PATCH 2/4] Fix zero check copy-paste --- arbos/l1pricing/l1pricing.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arbos/l1pricing/l1pricing.go b/arbos/l1pricing/l1pricing.go index 29e6b4c18f..5bfd577dc3 100644 --- a/arbos/l1pricing/l1pricing.go +++ b/arbos/l1pricing/l1pricing.go @@ -664,11 +664,11 @@ func makeFakeTxForMessage(message core.Message) *types.Transaction { nonce = randomNonce } gasTipCap := message.GasTipCap() - if nonce == 0 { + if gasTipCap.Sign() == 0 { gasTipCap = randomGasTipCap } gasFeeCap := message.GasFeeCap() - if nonce == 0 { + if gasFeeCap.Sign() == 0 { gasFeeCap = randomGasFeeCap } return types.NewTx(&types.DynamicFeeTx{ From 9e92eb191370f4035ac6ca8826bf7dbdc000ac95 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 31 Aug 2022 20:02:26 -0500 Subject: [PATCH 3/4] Fix GasEstimateComponents --- nodeInterface/NodeInterface.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nodeInterface/NodeInterface.go b/nodeInterface/NodeInterface.go index e0ee0f33cf..346c1cf41e 100644 --- a/nodeInterface/NodeInterface.go +++ b/nodeInterface/NodeInterface.go @@ -470,6 +470,8 @@ func (n NodeInterface) GasEstimateComponents( pricing := c.State.L1PricingState() + // Setting the gas will affect the poster data cost + args.Gas = &totalRaw msg, err = args.ToMessage(gasCap, n.header, evm.StateDB.(*state.StateDB)) if err != nil { return 0, 0, nil, nil, err From f1d660397690e9bf8042089706ef66a830c7a9cd Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 31 Aug 2022 20:03:27 -0500 Subject: [PATCH 4/4] Add comment to makeFakeTxForMessage clarifying that it's invalid --- arbos/l1pricing/l1pricing.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arbos/l1pricing/l1pricing.go b/arbos/l1pricing/l1pricing.go index 5bfd577dc3..3b3954bfe4 100644 --- a/arbos/l1pricing/l1pricing.go +++ b/arbos/l1pricing/l1pricing.go @@ -658,6 +658,8 @@ var randV = arbmath.BigMulByUint(params.ArbitrumOneChainConfig().ChainID, 3) var randR = crypto.Keccak256Hash([]byte("R")).Big() var randS = crypto.Keccak256Hash([]byte("S")).Big() +// The returned tx will be invalid, likely for a number of reasons such as an invalid signature. +// It's only used to check how large it is after brotli level 0 compression. func makeFakeTxForMessage(message core.Message) *types.Transaction { nonce := message.Nonce() if nonce == 0 {