From 75ccc47b973ac9d1e0d6d8bb279d8f9ac5449389 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 16:48:52 -0700 Subject: [PATCH 01/28] l2 geth: new fee logic --- l2geth/core/rollup_fee.go | 98 +++++++++++---------------- l2geth/core/rollup_fee_test.go | 117 +++++++++++++++++++++------------ 2 files changed, 113 insertions(+), 102 deletions(-) diff --git a/l2geth/core/rollup_fee.go b/l2geth/core/rollup_fee.go index 17662547df22..efcdbdfd9911 100644 --- a/l2geth/core/rollup_fee.go +++ b/l2geth/core/rollup_fee.go @@ -15,8 +15,12 @@ const overhead uint64 = 4200 // hundredMillion is a constant used in the gas encoding formula const hundredMillion uint64 = 100_000_000 +const hundredBillion uint64 = 100_000_000_000 +const feeScalar uint64 = 1000 var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) +var bigHundredBillion = new(big.Int).SetUint64(hundredBillion) +var bigFeeScalar = new(big.Int).SetUint64(feeScalar) // errInvalidGasPrice is the error returned when a user submits an incorrect gas // price. The gas price must satisfy a particular equation depending on if it @@ -24,22 +28,37 @@ var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) var errInvalidGasPrice = errors.New("rollup fee: invalid gas price") // CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into -// account the cost of publishing data to L1. -// l2_gas_price * l2_gas_limit + l1_gas_price * l1_gas_used -// where the l2 gas price must satisfy the equation `x % (10**8) = 1` -// and the l1 gas price must satisfy the equation `x % (10**8) = 0` +// fee = (floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit) * tx.gasPrice func CalculateRollupFee(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) (*big.Int, error) { - if err := VerifyL1GasPrice(l1GasPrice); err != nil { + if err := VerifyGasPrice(l1GasPrice); err != nil { return nil, fmt.Errorf("invalid L1 gas price %d: %w", l1GasPrice, err) } - if err := VerifyL2GasPrice(l2GasPrice); err != nil { + if err := VerifyGasPrice(l2GasPrice); err != nil { return nil, fmt.Errorf("invalid L2 gas price %d: %w", l2GasPrice, err) } l1GasLimit := calculateL1GasLimit(data, overhead) l1Fee := new(big.Int).Mul(l1GasPrice, l1GasLimit) l2Fee := new(big.Int).Mul(l2GasLimit, l2GasPrice) - fee := new(big.Int).Add(l1Fee, l2Fee) - return fee, nil + sum := new(big.Int).Add(l1Fee, l2Fee) + scaled := new(big.Int).Div(sum, bigFeeScalar) + result := new(big.Int).Add(scaled, l2GasLimit) + return result, nil +} + +// VerifyGasPrice returns an error if the gas price doesn't satisfy +// the constraints. +func VerifyGasPrice(gasPrice *big.Int) error { + if gasPrice.Cmp(common.Big0) == 0 { + return nil + } + if gasPrice.Cmp(bigHundredBillion) < 0 { + return fmt.Errorf("too small: %w", errInvalidGasPrice) + } + mod := new(big.Int).Mod(gasPrice, bigHundredMillion) + if mod.Cmp(common.Big0) != 0 { + return fmt.Errorf("incorrect multiple: %w", errInvalidGasPrice) + } + return nil } // calculateL1GasLimit computes the L1 gasLimit based on the calldata and @@ -54,60 +73,17 @@ func calculateL1GasLimit(data []byte, overhead uint64) *big.Int { return new(big.Int).SetUint64(gasLimit) } -// ceilModOneHundredMillion rounds the input integer up to the nearest modulus -// of one hundred million -func ceilModOneHundredMillion(num *big.Int) *big.Int { - if new(big.Int).Mod(num, bigHundredMillion).Cmp(common.Big0) == 0 { - return num - } - sum := new(big.Int).Add(num, bigHundredMillion) - mod := new(big.Int).Mod(num, bigHundredMillion) - return new(big.Int).Sub(sum, mod) -} - -// VerifyL1GasPrice returns an error if the number is an invalid possible L1 gas -// price -func VerifyL1GasPrice(l1GasPrice *big.Int) error { - if new(big.Int).Mod(l1GasPrice, bigHundredMillion).Cmp(common.Big0) != 0 { - return errInvalidGasPrice - } - return nil -} - -// VerifyL2GasPrice returns an error if the number is an invalid possible L2 gas -// price -func VerifyL2GasPrice(l2GasPrice *big.Int) error { - isNonZero := l2GasPrice.Cmp(common.Big0) != 0 - isNotModHundredMillion := new(big.Int).Mod(l2GasPrice, bigHundredMillion).Cmp(common.Big1) != 0 - if isNonZero && isNotModHundredMillion { - return errInvalidGasPrice - } - if l2GasPrice.Cmp(common.Big0) == 0 { - return errInvalidGasPrice - } - return nil -} - -// RoundL1GasPrice returns a ceilModOneHundredMillion where 0 -// is the identity function -func RoundL1GasPrice(gasPrice *big.Int) *big.Int { - return ceilModOneHundredMillion(gasPrice) -} - -// RoundL2GasPriceBig implements the algorithm: -// if gasPrice is 0; return 1 -// if gasPrice is 1; return 10**8 + 1 -// return ceilModOneHundredMillion(gasPrice-1)+1 -func RoundL2GasPrice(gasPrice *big.Int) *big.Int { +// RoundGasPrice rounds the gas price up to the next valid gas price +func RoundGasPrice(gasPrice *big.Int) *big.Int { if gasPrice.Cmp(common.Big0) == 0 { - return big.NewInt(1) + return new(big.Int) } - if gasPrice.Cmp(common.Big1) == 0 { - return new(big.Int).Add(bigHundredMillion, common.Big1) + mod := new(big.Int).Mod(gasPrice, bigHundredBillion) + if mod.Cmp(common.Big0) == 0 { + return gasPrice } - gp := new(big.Int).Sub(gasPrice, common.Big1) - mod := ceilModOneHundredMillion(gp) - return new(big.Int).Add(mod, common.Big1) + sum := new(big.Int).Add(gasPrice, bigHundredBillion) + return new(big.Int).Sub(sum, mod) } func DecodeL2GasLimit(gasLimit *big.Int) *big.Int { @@ -116,11 +92,13 @@ func DecodeL2GasLimit(gasLimit *big.Int) *big.Int { func zeroesAndOnes(data []byte) (uint64, uint64) { var zeroes uint64 + var ones uint64 for _, byt := range data { if byt == 0 { zeroes++ + } else { + ones++ } } - ones := uint64(len(data)) - zeroes return zeroes, ones } diff --git a/l2geth/core/rollup_fee_test.go b/l2geth/core/rollup_fee_test.go index dfe68107ad3f..bcc8025e69e9 100644 --- a/l2geth/core/rollup_fee_test.go +++ b/l2geth/core/rollup_fee_test.go @@ -7,49 +7,24 @@ import ( "testing" ) -var roundingL1GasPriceTests = map[string]struct { +var roundingGasPriceTests = map[string]struct { input uint64 expect uint64 }{ - "simple": {10, pow10(8)}, - "one-over": {pow10(8) + 1, 2 * pow10(8)}, - "exact": {pow10(8), pow10(8)}, - "one-under": {pow10(8) - 1, pow10(8)}, - "small": {3, pow10(8)}, - "two": {2, pow10(8)}, - "one": {1, pow10(8)}, + "simple": {10, hundredBillion}, + "one-over": {hundredBillion + 1, 2 * hundredBillion}, + "exact": {hundredBillion, hundredBillion}, + "one-under": {hundredBillion - 1, hundredBillion}, + "small": {3, hundredBillion}, + "two": {2, hundredBillion}, + "one": {1, hundredBillion}, "zero": {0, 0}, } -func TestRoundL1GasPrice(t *testing.T) { - for name, tt := range roundingL1GasPriceTests { +func TestRoundGasPrice(t *testing.T) { + for name, tt := range roundingGasPriceTests { t.Run(name, func(t *testing.T) { - got := RoundL1GasPrice(new(big.Int).SetUint64(tt.input)) - if got.Uint64() != tt.expect { - t.Fatalf("Mismatched rounding to nearest, got %d expected %d", got, tt.expect) - } - }) - } -} - -var roundingL2GasPriceTests = map[string]struct { - input uint64 - expect uint64 -}{ - "simple": {10, pow10(8) + 1}, - "one-over": {pow10(8) + 2, 2*pow10(8) + 1}, - "exact": {pow10(8) + 1, pow10(8) + 1}, - "one-under": {pow10(8), pow10(8) + 1}, - "small": {3, pow10(8) + 1}, - "two": {2, pow10(8) + 1}, - "one": {1, pow10(8) + 1}, - "zero": {0, 1}, -} - -func TestRoundL2GasPrice(t *testing.T) { - for name, tt := range roundingL2GasPriceTests { - t.Run(name, func(t *testing.T) { - got := RoundL2GasPrice(new(big.Int).SetUint64(tt.input)) + got := RoundGasPrice(new(big.Int).SetUint64(tt.input)) if got.Uint64() != tt.expect { t.Fatalf("Mismatched rounding to nearest, got %d expected %d", got, tt.expect) } @@ -86,17 +61,75 @@ var feeTests = map[string]struct { l2GasPrice uint64 err error }{ - "simple": {100, 100_000_000, 437118, 100_000_001, nil}, - "zero-l2-gasprice": {10, 100_000_000, 196205, 0, errInvalidGasPrice}, - "one-l2-gasprice": {10, 100_000_000, 196205, 1, nil}, - "zero-l1-gasprice": {10, 0, 196205, 100_000_001, nil}, - "one-l1-gasprice": {10, 1, 23255, 23254, errInvalidGasPrice}, + "simple": { + dataLen: 10, + l1GasPrice: hundredBillion, + l2GasLimit: 437118, + l2GasPrice: hundredBillion, + err: nil, + }, + "zero-l2-gasprice": { + dataLen: 10, + l1GasPrice: hundredBillion, + l2GasLimit: 196205, + l2GasPrice: 0, + err: nil, + }, + "one-l2-gasprice": { + dataLen: 10, + l1GasPrice: hundredBillion, + l2GasLimit: 196205, + l2GasPrice: 1, + err: errInvalidGasPrice, + }, + "zero-l1-gasprice": { + dataLen: 10, + l1GasPrice: 0, + l2GasLimit: 196205, + l2GasPrice: hundredBillion, + err: nil, + }, + "one-l1-gasprice": { + dataLen: 10, + l1GasPrice: 1, + l2GasLimit: 23255, + l2GasPrice: hundredBillion, + err: errInvalidGasPrice, + }, + "zero-gasprices": { + dataLen: 10, + l1GasPrice: 0, + l2GasLimit: 23255, + l2GasPrice: 0, + err: nil, + }, + "bad-l2-gasprice": { + dataLen: 10, + l1GasPrice: 0, + l2GasLimit: 23255, + l2GasPrice: hundredBillion - 1, + err: errInvalidGasPrice, + }, + "bad-l1-gasprice": { + dataLen: 10, + l1GasPrice: hundredBillion - 1, + l2GasLimit: 44654, + l2GasPrice: hundredBillion, + err: errInvalidGasPrice, + }, + "max-gaslimit": { + dataLen: 10, + l1GasPrice: hundredBillion, + l2GasLimit: 0x4ffffff, + l2GasPrice: hundredBillion, + err: nil, + }, } func TestCalculateRollupFee(t *testing.T) { for name, tt := range feeTests { t.Run(name, func(t *testing.T) { - data := make([]byte, 0, tt.dataLen) + data := make([]byte, tt.dataLen) l1GasPrice := new(big.Int).SetUint64(tt.l1GasPrice) l2GasLimit := new(big.Int).SetUint64(tt.l2GasLimit) l2GasPrice := new(big.Int).SetUint64(tt.l2GasPrice) From 9a5675ba8dfc9a48827e653e66693640f6b8f34c Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 16:49:51 -0700 Subject: [PATCH 02/28] l2 geth: migrate to fees package --- l2geth/eth/gasprice/rollup_gasprice.go | 14 +++++++------- l2geth/internal/ethapi/api.go | 8 +++++--- l2geth/{core => rollup/fees}/rollup_fee.go | 9 ++++++--- l2geth/{core => rollup/fees}/rollup_fee_test.go | 9 ++++++++- l2geth/rollup/sync_service.go | 12 ++++++++---- l2geth/rollup/sync_service_test.go | 7 ++++--- 6 files changed, 38 insertions(+), 21 deletions(-) rename l2geth/{core => rollup/fees}/rollup_fee.go (93%) rename l2geth/{core => rollup/fees}/rollup_fee_test.go (96%) diff --git a/l2geth/eth/gasprice/rollup_gasprice.go b/l2geth/eth/gasprice/rollup_gasprice.go index 388161febecc..05216c65aac6 100644 --- a/l2geth/eth/gasprice/rollup_gasprice.go +++ b/l2geth/eth/gasprice/rollup_gasprice.go @@ -5,8 +5,8 @@ import ( "math/big" "sync" - "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rollup/fees" ) // RollupOracle holds the L1 and L2 gas prices for fee calculation @@ -34,13 +34,13 @@ func (gpo *RollupOracle) SuggestL1GasPrice(ctx context.Context) (*big.Int, error } // SetL1GasPrice returns the current L1 gas price -func (gpo *RollupOracle) SetL1GasPrice(dataPrice *big.Int) error { +func (gpo *RollupOracle) SetL1GasPrice(gasPrice *big.Int) error { gpo.dataPriceLock.Lock() defer gpo.dataPriceLock.Unlock() - if err := core.VerifyL1GasPrice(dataPrice); err != nil { + if err := fees.VerifyGasPrice(gasPrice); err != nil { return err } - gpo.dataPrice = dataPrice + gpo.dataPrice = gasPrice log.Info("Set L1 Gas Price", "gasprice", gpo.dataPrice) return nil } @@ -54,13 +54,13 @@ func (gpo *RollupOracle) SuggestL2GasPrice(ctx context.Context) (*big.Int, error } // SetL2GasPrice returns the current L2 gas price -func (gpo *RollupOracle) SetL2GasPrice(executionPrice *big.Int) error { +func (gpo *RollupOracle) SetL2GasPrice(gasPrice *big.Int) error { gpo.executionPriceLock.Lock() defer gpo.executionPriceLock.Unlock() - if err := core.VerifyL2GasPrice(executionPrice); err != nil { + if err := fees.VerifyGasPrice(gasPrice); err != nil { return err } - gpo.executionPrice = executionPrice + gpo.executionPrice = gasPrice log.Info("Set L2 Gas Price", "gasprice", gpo.executionPrice) return nil } diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index 2828d83707bd..8a1cc5e5ba51 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -45,15 +45,17 @@ import ( "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rlp" + "github.com/ethereum/go-ethereum/rollup/fees" "github.com/ethereum/go-ethereum/rpc" "github.com/tyler-smith/go-bip39" ) const ( - defaultGasPrice = params.Wei + defaultGasPrice = params.Wei * fees.FeeScalar ) var errOVMUnsupported = errors.New("OVM: Unsupported RPC Method") +var bigDefaultGasPrice = new(big.Int).SetUint64(defaultGasPrice) // PublicEthereumAPI provides an API to access Ethereum related information. // It offers only methods that operate on public data that is freely available to anyone. @@ -68,7 +70,7 @@ func NewPublicEthereumAPI(b Backend) *PublicEthereumAPI { // GasPrice always returns 1 gwei. See `DoEstimateGas` below for context. func (s *PublicEthereumAPI) GasPrice(ctx context.Context) (*hexutil.Big, error) { - return (*hexutil.Big)(big.NewInt(defaultGasPrice)), nil + return (*hexutil.Big)(bigDefaultGasPrice), nil } // ProtocolVersion returns the current Ethereum protocol version this node supports @@ -1059,7 +1061,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash } // 3. calculate the fee l2GasLimit := new(big.Int).SetUint64(uint64(gasUsed)) - fee, err := core.CalculateRollupFee(data, l1GasPrice, l2GasLimit, l2GasPrice) + fee, err := fees.CalculateRollupFee(data, l1GasPrice, l2GasLimit, l2GasPrice) if err != nil { return 0, err } diff --git a/l2geth/core/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go similarity index 93% rename from l2geth/core/rollup_fee.go rename to l2geth/rollup/fees/rollup_fee.go index efcdbdfd9911..7dcf9130f88e 100644 --- a/l2geth/core/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -1,4 +1,4 @@ -package core +package fees import ( "errors" @@ -16,11 +16,11 @@ const overhead uint64 = 4200 // hundredMillion is a constant used in the gas encoding formula const hundredMillion uint64 = 100_000_000 const hundredBillion uint64 = 100_000_000_000 -const feeScalar uint64 = 1000 +const FeeScalar uint64 = 1000 var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) var bigHundredBillion = new(big.Int).SetUint64(hundredBillion) -var bigFeeScalar = new(big.Int).SetUint64(feeScalar) +var bigFeeScalar = new(big.Int).SetUint64(FeeScalar) // errInvalidGasPrice is the error returned when a user submits an incorrect gas // price. The gas price must satisfy a particular equation depending on if it @@ -28,7 +28,10 @@ var bigFeeScalar = new(big.Int).SetUint64(feeScalar) var errInvalidGasPrice = errors.New("rollup fee: invalid gas price") // CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into +// account both the cost of submitting the transaction to L1 as well as +// executing the transaction on L2 // fee = (floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit) * tx.gasPrice +// where tx.gasPrice is hard coded to 1000 * wei func CalculateRollupFee(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) (*big.Int, error) { if err := VerifyGasPrice(l1GasPrice); err != nil { return nil, fmt.Errorf("invalid L1 gas price %d: %w", l1GasPrice, err) diff --git a/l2geth/core/rollup_fee_test.go b/l2geth/rollup/fees/rollup_fee_test.go similarity index 96% rename from l2geth/core/rollup_fee_test.go rename to l2geth/rollup/fees/rollup_fee_test.go index bcc8025e69e9..f6372ed414c5 100644 --- a/l2geth/core/rollup_fee_test.go +++ b/l2geth/rollup/fees/rollup_fee_test.go @@ -1,4 +1,4 @@ -package core +package fees import ( "errors" @@ -124,6 +124,13 @@ var feeTests = map[string]struct { l2GasPrice: hundredBillion, err: nil, }, + "larger-divisor": { + dataLen: 10, + l1GasPrice: 0, + l2GasLimit: 10, + l2GasPrice: 0, + err: nil, + }, } func TestCalculateRollupFee(t *testing.T) { diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index b3f831559a79..b261df281d22 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -21,6 +21,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth/gasprice" + "github.com/ethereum/go-ethereum/rollup/fees" ) // errShortRemoteTip is an error for when the remote tip is shorter than the @@ -446,8 +447,11 @@ func (s *SyncService) updateL2GasPrice(hash *common.Hash) error { } result := state.GetState(s.gpoAddress, l2GasPriceSlot) gasPrice := result.Big() - if err := core.VerifyL2GasPrice(gasPrice); err != nil { - gp := core.RoundL2GasPrice(gasPrice) + if err := fees.VerifyGasPrice(gasPrice); err != nil { + // If an invalid gas price is found in the state, then + // round it up to the next valid gas price. The contract + // should technically never allow for an invalid gas price. + gp := fees.RoundGasPrice(gasPrice) log.Warn("Invalid gas price detected in state", "state", gasPrice, "using", gp) gasPrice = gp } @@ -743,8 +747,8 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { } // Calculate the fee based on decoded L2 gas limit gas := new(big.Int).SetUint64(tx.Gas()) - l2GasLimit := core.DecodeL2GasLimit(gas) - fee, err := core.CalculateRollupFee(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) + l2GasLimit := fees.DecodeL2GasLimit(gas) + fee, err := fees.CalculateRollupFee(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) if err != nil { return err } diff --git a/l2geth/rollup/sync_service_test.go b/l2geth/rollup/sync_service_test.go index d23f5ad304d4..43f8cf791c2c 100644 --- a/l2geth/rollup/sync_service_test.go +++ b/l2geth/rollup/sync_service_test.go @@ -20,6 +20,7 @@ import ( "github.com/ethereum/go-ethereum/eth/gasprice" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/rollup/fees" ) func setupLatestEthContextTest() (*SyncService, *EthContext) { @@ -507,7 +508,7 @@ func TestSyncServiceL1GasPrice(t *testing.T) { t.Fatal(err) } - if gasAfter.Cmp(core.RoundL1GasPrice(big.NewInt(1))) != 0 { + if gasAfter.Cmp(fees.RoundGasPrice(big.NewInt(1))) != 0 { t.Fatal("expected 100 gas price, got", gasAfter) } } @@ -533,7 +534,7 @@ func TestSyncServiceL2GasPrice(t *testing.T) { if err != nil { t.Fatal("Cannot get state db") } - l2GasPrice := big.NewInt(100000001) + l2GasPrice := big.NewInt(100000000000) state.SetState(service.gpoAddress, l2GasPriceSlot, common.BigToHash(l2GasPrice)) root, _ := state.Commit(false) @@ -824,7 +825,7 @@ func (m *mockClient) SyncStatus(backend Backend) (*SyncStatus, error) { } func (m *mockClient) GetL1GasPrice() (*big.Int, error) { - price := core.RoundL1GasPrice(big.NewInt(2)) + price := fees.RoundGasPrice(big.NewInt(2)) return price, nil } From ceaab45d93b043e1f8526e14ffa2c8be04b10aed Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 16:51:48 -0700 Subject: [PATCH 03/28] core-utils: new fee scheme --- packages/core-utils/src/fees.ts | 69 ++++++------- packages/core-utils/test/fees/fees.spec.ts | 109 +++++++++++++-------- 2 files changed, 100 insertions(+), 78 deletions(-) diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index cc754e5fcb81..b415fc6e082c 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -6,6 +6,8 @@ import { BigNumber } from 'ethers' import { remove0x } from './common' const hundredMillion = BigNumber.from(100_000_000) +const hundredBillion = BigNumber.from(100_000_000_000) +const feeScalar = BigNumber.from(1000) const txDataZeroGas = 4 const txDataNonZeroGasEIP2028 = 16 const overhead = 4200 @@ -30,16 +32,31 @@ function encode(input: EncodableL2GasLimit): BigNumber { l2GasPrice = BigNumber.from(l2GasPrice) } - if (!verifyL2GasPrice(l2GasPrice)) { + if (!verifyGasPrice(l2GasPrice)) { throw new Error(`Invalid L2 Gas Price: ${l2GasPrice.toString()}`) } - if (!verifyL1GasPrice(l1GasPrice)) { + if (!verifyGasPrice(l1GasPrice)) { throw new Error(`Invalid L1 Gas Price: ${l1GasPrice.toString()}`) } const l1GasLimit = calculateL1GasLimit(data) - const l1Fee = l1GasPrice.mul(l1GasLimit) + const l1Fee = l1GasLimit.mul(l1GasPrice) const l2Fee = l2GasLimit.mul(l2GasPrice) - return l1Fee.add(l2Fee) + const sum = l1Fee.add(l2Fee) + const scaled = sum.div(feeScalar) + return scaled.add(l2GasLimit) +} + +function verifyGasPrice(gasPrice: BigNumber | number): boolean { + if (typeof gasPrice === 'number') { + gasPrice = BigNumber.from(gasPrice) + } + if (gasPrice.eq(0)) { + return true + } + if (gasPrice.lt(hundredBillion)) { + return false + } + return gasPrice.mod(hundredMillion).eq(0) } function decode(fee: BigNumber | number): BigNumber { @@ -58,15 +75,7 @@ export function verifyL2GasPrice(gasPrice: BigNumber | number): boolean { if (typeof gasPrice === 'number') { gasPrice = BigNumber.from(gasPrice) } - // If the gas price is not equal to 0 and the gas price mod - // one hundred million is not one - if (!gasPrice.eq(0) && !gasPrice.mod(hundredMillion).eq(1)) { - return false - } - if (gasPrice.eq(0)) { - return false - } - return true + return gasPrice.mod(hundredMillion).eq(0) } export function verifyL1GasPrice(gasPrice: BigNumber | number): boolean { @@ -76,12 +85,12 @@ export function verifyL1GasPrice(gasPrice: BigNumber | number): boolean { return gasPrice.mod(hundredMillion).eq(0) } -export function calculateL1GasLimit(data: string | Buffer): number { +export function calculateL1GasLimit(data: string | Buffer): BigNumber { const [zeroes, ones] = zeroesAndOnes(data) const zeroesCost = zeroes * txDataZeroGas const onesCost = ones * txDataNonZeroGasEIP2028 const gasLimit = zeroesCost + onesCost + overhead - return gasLimit + return BigNumber.from(gasLimit) } export function zeroesAndOnes(data: Buffer | string): Array { @@ -100,33 +109,17 @@ export function zeroesAndOnes(data: Buffer | string): Array { return [zeros, ones] } -export function roundL1GasPrice(gasPrice: BigNumber | number): BigNumber { - if (typeof gasPrice === 'number') { - gasPrice = BigNumber.from(gasPrice) - } - return ceilModOneHundredMillion(gasPrice) -} - -function ceilModOneHundredMillion(num: BigNumber): BigNumber { - if (num.mod(hundredMillion).eq(0)) { - return num - } - const sum = num.add(hundredMillion) - const mod = num.mod(hundredMillion) - return sum.sub(mod) -} - -export function roundL2GasPrice(gasPrice: BigNumber | number): BigNumber { +export function roundGasPrice(gasPrice: BigNumber | number): BigNumber { if (typeof gasPrice === 'number') { gasPrice = BigNumber.from(gasPrice) } if (gasPrice.eq(0)) { - return BigNumber.from(1) + return gasPrice } - if (gasPrice.eq(1)) { - return hundredMillion.add(1) + if (gasPrice.mod(hundredBillion).eq(0)) { + return gasPrice } - const gp = gasPrice.sub(1) - const mod = ceilModOneHundredMillion(gp) - return mod.add(1) + const sum = gasPrice.add(hundredBillion) + const mod = gasPrice.mod(hundredBillion) + return sum.sub(mod) } diff --git a/packages/core-utils/test/fees/fees.spec.ts b/packages/core-utils/test/fees/fees.spec.ts index bc548c5baa67..bee3cf8c7215 100644 --- a/packages/core-utils/test/fees/fees.spec.ts +++ b/packages/core-utils/test/fees/fees.spec.ts @@ -2,6 +2,9 @@ import { expect } from '../setup' import * as fees from '../../src/fees' import { BigNumber } from 'ethers' +const hundredBillion = 10 ** 11 +const million = 10 ** 6 + describe('Fees', () => { it('should count zeros and ones', () => { const cases = [ @@ -18,42 +21,25 @@ describe('Fees', () => { } }) - describe('Round L1 Gas Price', () => { - const roundL1GasPriceTests = [ - { input: 10, expect: 10 ** 8, name: 'simple' }, - { input: 10 ** 8 + 1, expect: 2 * 10 ** 8, name: 'one-over' }, - { input: 10 ** 8, expect: 10 ** 8, name: 'exact' }, - { input: 10 ** 8 - 1, expect: 10 ** 8, name: 'one-under' }, - { input: 3, expect: 10 ** 8, name: 'small' }, - { input: 2, expect: 10 ** 8, name: 'two' }, - { input: 1, expect: 10 ** 8, name: 'one' }, + describe('Round Gas Price', () => { + const roundGasPriceTests = [ + { input: 10, expect: hundredBillion, name: 'simple' }, + { + input: hundredBillion + 1, + expect: 2 * hundredBillion, + name: 'one-over', + }, + { input: hundredBillion, expect: hundredBillion, name: 'exact' }, + { input: hundredBillion - 1, expect: hundredBillion, name: 'one-under' }, + { input: 3, expect: hundredBillion, name: 'small' }, + { input: 2, expect: hundredBillion, name: 'two' }, + { input: 1, expect: hundredBillion, name: 'one' }, { input: 0, expect: 0, name: 'zero' }, ] - for (const test of roundL1GasPriceTests) { - it(`should pass for ${test.name} case`, () => { - const got = fees.roundL1GasPrice(test.input) - const expected = BigNumber.from(test.expect) - expect(got).to.deep.equal(expected) - }) - } - }) - - describe('Round L2 Gas Price', () => { - const roundL2GasPriceTests = [ - { input: 10, expect: 10 ** 8 + 1, name: 'simple' }, - { input: 10 ** 8 + 2, expect: 2 * 10 ** 8 + 1, name: 'one-over' }, - { input: 10 ** 8 + 1, expect: 10 ** 8 + 1, name: 'exact' }, - { input: 10 ** 8, expect: 10 ** 8 + 1, name: 'one-under' }, - { input: 3, expect: 10 ** 8 + 1, name: 'small' }, - { input: 2, expect: 10 ** 8 + 1, name: 'two' }, - { input: 1, expect: 10 ** 8 + 1, name: 'one' }, - { input: 0, expect: 1, name: 'zero' }, - ] - - for (const test of roundL2GasPriceTests) { + for (const test of roundGasPriceTests) { it(`should pass for ${test.name} case`, () => { - const got = fees.roundL2GasPrice(test.input) + const got = fees.roundGasPrice(test.input) const expected = BigNumber.from(test.expect) expect(got).to.deep.equal(expected) }) @@ -65,32 +51,32 @@ describe('Fees', () => { { name: 'simple', dataLen: 10, - l1GasPrice: 100_000_000, - l2GasPrice: 100_000_001, + l1GasPrice: hundredBillion, + l2GasPrice: hundredBillion, l2GasLimit: 437118, error: false, }, { name: 'zero-l2-gasprice', dataLen: 10, - l1GasPrice: 100_000_000, + l1GasPrice: hundredBillion, l2GasPrice: 0, l2GasLimit: 196205, - error: true, + error: false, }, { name: 'one-l2-gasprice', dataLen: 10, - l1GasPrice: 100_000_000, + l1GasPrice: hundredBillion, l2GasPrice: 1, l2GasLimit: 196205, - error: false, + error: true, }, { name: 'zero-l1-gasprice', dataLen: 10, l1GasPrice: 0, - l2GasPrice: 100_000_001, + l2GasPrice: hundredBillion, l2GasLimit: 196205, error: false, }, @@ -98,10 +84,53 @@ describe('Fees', () => { name: 'one-l1-gasprice', dataLen: 10, l1GasPrice: 1, - l2GasPrice: 23254, + l2GasPrice: hundredBillion, l2GasLimit: 23255, error: true, }, + { + name: 'zero-gasprices', + dataLen: 10, + l1GasPrice: 0, + l2GasPrice: 0, + l2GasLimit: 23255, + error: false, + }, + { + name: 'bad-l2-gasprice', + dataLen: 10, + l1GasPrice: 0, + l2GasPrice: hundredBillion - 1, + l2GasLimit: 23255, + error: true, + }, + { + name: 'bad-l1-gasprice', + dataLen: 10, + l1GasPrice: hundredBillion - 1, + l2GasPrice: hundredBillion, + l2GasLimit: 44654, + error: true, + }, + // The largest possible gaslimit that can be represented + // is 0x04ffffff which is plenty high enough to cover the + // L2 gas limit + { + name: 'max-gaslimit', + dataLen: 10, + l1GasPrice: hundredBillion, + l2GasPrice: hundredBillion, + l2GasLimit: 0x4ffffff, + error: false, + }, + { + name: 'larger-divisor', + dataLen: 10, + l1GasPrice: 0, + l2GasLimit: 10, + l2GasPrice: 0, + error: false, + }, ] for (const test of rollupFeesTests) { From cce5a9665dc274def8fee3478958a0cbfa90afe6 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 17:49:41 -0700 Subject: [PATCH 04/28] chore: add changeset --- .changeset/fuzzy-dogs-share.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/fuzzy-dogs-share.md diff --git a/.changeset/fuzzy-dogs-share.md b/.changeset/fuzzy-dogs-share.md new file mode 100644 index 000000000000..3b890e4e9e8d --- /dev/null +++ b/.changeset/fuzzy-dogs-share.md @@ -0,0 +1,6 @@ +--- +'@eth-optimism/l2geth': patch +'@eth-optimism/core-utils': patch +--- + +Implement the next fee spec in both geth and in core-utils From fa042c4053ed3a2b49d9f6d297789549b6ae3f9c Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 17:53:28 -0700 Subject: [PATCH 05/28] l2geth: delete dead code --- l2geth/rollup/fees/rollup_fee_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/l2geth/rollup/fees/rollup_fee_test.go b/l2geth/rollup/fees/rollup_fee_test.go index f6372ed414c5..9a0d9aea0228 100644 --- a/l2geth/rollup/fees/rollup_fee_test.go +++ b/l2geth/rollup/fees/rollup_fee_test.go @@ -2,7 +2,6 @@ package fees import ( "errors" - "math" "math/big" "testing" ) @@ -155,7 +154,3 @@ func TestCalculateRollupFee(t *testing.T) { }) } } - -func pow10(x int) uint64 { - return uint64(math.Pow10(x)) -} From 5d54722f8b0572d97208be039c2271f63b44f3ae Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 18:07:39 -0700 Subject: [PATCH 06/28] integration-tests: fix typo --- integration-tests/test/rpc.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 5d9c7e441f5d..365d62308fa9 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -1,7 +1,7 @@ import { injectL2Context, L2GasLimit, - roundL1GasPrice, + roundGasPrice, } from '@eth-optimism/core-utils' import { Wallet, BigNumber, Contract } from 'ethers' import { ethers } from 'hardhat' @@ -352,7 +352,7 @@ describe('Basic RPC tests', () => { const l2GasPrice = BigNumber.from(1) const expected = L2GasLimit.encode({ data: tx.data, - l1GasPrice: roundL1GasPrice(l1GasPrice), + l1GasPrice: roundGasPrice(l1GasPrice), l2GasLimit: BigNumber.from(l2Gaslimit), l2GasPrice, }) From 2916d75ad6f43f7adfd60799570c440ac2b9c5bf Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 21:29:28 -0700 Subject: [PATCH 07/28] integration-tests: fixes --- integration-tests/test/erc20.spec.ts | 6 +++++- integration-tests/test/native-eth.spec.ts | 4 ++-- integration-tests/test/rpc.spec.ts | 15 ++++++++------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/integration-tests/test/erc20.spec.ts b/integration-tests/test/erc20.spec.ts index 925ec01e6f21..ea414a715cf9 100644 --- a/integration-tests/test/erc20.spec.ts +++ b/integration-tests/test/erc20.spec.ts @@ -1,5 +1,6 @@ import { Contract, ContractFactory, Wallet } from 'ethers' import { ethers } from 'hardhat' +import { L2GasLimit } from '@eth-optimism/core-utils' import chai, { expect } from 'chai' import { GWEI } from './shared/utils' import { OptimismEnv } from './shared/env' @@ -64,7 +65,10 @@ describe('Basic ERC20 interactions', async () => { const receipt = await transfer.wait() // The expected fee paid is the value returned by eth_estimateGas - const expectedFeePaid = await ERC20.estimateGas.transfer(other.address, 100) + const gasLimit = await ERC20.estimateGas.transfer(other.address, 100) + const gasPrice = await wallet.getGasPrice() + expect(gasPrice).to.deep.equal(L2GasLimit.feeScalar) + const expectedFeePaid = gasLimit.mul(gasPrice) // There are two events from the transfer with the first being // the ETH fee paid and the second of the value transfered (100) diff --git a/integration-tests/test/native-eth.spec.ts b/integration-tests/test/native-eth.spec.ts index 017199d28cf6..91d66a5b09c2 100644 --- a/integration-tests/test/native-eth.spec.ts +++ b/integration-tests/test/native-eth.spec.ts @@ -45,13 +45,13 @@ describe('Native ETH Integration Tests', async () => { const amount = utils.parseEther('0.5') const addr = '0x' + '1234'.repeat(10) const gas = await env.ovmEth.estimateGas.transfer(addr, amount) - expect(gas).to.be.deep.eq(BigNumber.from(0x23284d28fe6d)) + expect(gas).to.be.deep.eq(BigNumber.from(0x7080f9de6d)) }) it('Should estimate gas for ETH withdraw', async () => { const amount = utils.parseEther('0.5') const gas = await env.ovmEth.estimateGas.withdraw(amount) - expect(gas).to.be.deep.eq(BigNumber.from(0x207ad91a77b4)) + expect(gas).to.be.deep.eq(BigNumber.from(0x67ef8ae7b4)) }) }) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 365d62308fa9..c6e023a0f291 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -2,6 +2,7 @@ import { injectL2Context, L2GasLimit, roundGasPrice, + toRpcHexString, } from '@eth-optimism/core-utils' import { Wallet, BigNumber, Contract } from 'ethers' import { ethers } from 'hardhat' @@ -134,7 +135,7 @@ describe('Basic RPC tests', () => { } await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - 'fee too low: 1, use at least tx.gasLimit = 33600000000001 and tx.gasPrice = 1' + 'fee too low: 1, use at least tx.gasLimit = 420000000001 and tx.gasPrice = 1000' ) }) @@ -311,18 +312,18 @@ describe('Basic RPC tests', () => { }) describe('eth_gasPrice', () => { - it('gas price should be 1 gwei', async () => { - expect(await provider.getGasPrice()).to.be.deep.equal(1) + it('gas price should be the fee scalar', async () => { + expect(await provider.getGasPrice()).to.be.deep.equal(L2GasLimit.feeScalar.toNumber()) }) }) - describe('eth_estimateGas (returns the fee)', () => { + describe('eth_estimateGas (returns the scaled fee)', () => { it('should return a gas estimate for txs with empty data', async () => { const estimate = await l2Provider.estimateGas({ to: DEFAULT_TRANSACTION.to, value: 0, }) - expect(estimate).to.be.eq(33600000119751) + expect(estimate).to.be.eq(420000119751) }) it('should return a gas estimate that grows with the size of data', async () => { @@ -333,7 +334,7 @@ describe('Basic RPC tests', () => { for (const data of dataLen) { const tx = { to: '0x' + '1234'.repeat(10), - gasPrice: '0x1', + gasPrice: toRpcHexString(100_000_000_000), value: '0x0', data: '0x' + '00'.repeat(data), from: '0x' + '1234'.repeat(10), @@ -349,7 +350,7 @@ describe('Basic RPC tests', () => { // The L2GasPrice should be fetched from the L2GasPrice oracle contract, // but it does not yet exist. Use the default value for now - const l2GasPrice = BigNumber.from(1) + const l2GasPrice = BigNumber.from(0) const expected = L2GasLimit.encode({ data: tx.data, l1GasPrice: roundGasPrice(l1GasPrice), From 9f09eecbc67ea4540d7dd71dc101ecba06db1bf5 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 21:30:00 -0700 Subject: [PATCH 08/28] fees: use fee scalar --- l2geth/rollup/sync_service.go | 2 +- packages/core-utils/src/fees.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index b261df281d22..63fad8736637 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -762,7 +762,7 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { } // Make sure that the fee is paid if tx.Gas() < fee.Uint64() { - return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = 1", tx.Gas(), fee.Uint64()) + return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", tx.Gas(), fee.Uint64(), fees.FeeScalar) } return nil } diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index b415fc6e082c..f0301c86ffdc 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -69,6 +69,7 @@ function decode(fee: BigNumber | number): BigNumber { export const L2GasLimit = { encode, decode, + feeScalar, } export function verifyL2GasPrice(gasPrice: BigNumber | number): boolean { From ab4f9bc20132a3a87b6e75fbd0fa124abe691168 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 21:39:31 -0700 Subject: [PATCH 09/28] lint: fix --- integration-tests/test/rpc.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 3fd40a20cc1c..c1ab4ea9ba92 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -313,7 +313,9 @@ describe('Basic RPC tests', () => { describe('eth_gasPrice', () => { it('gas price should be the fee scalar', async () => { - expect(await provider.getGasPrice()).to.be.deep.equal(L2GasLimit.feeScalar.toNumber()) + expect(await provider.getGasPrice()).to.be.deep.equal( + L2GasLimit.feeScalar.toNumber() + ) }) }) From 538398ce0b3e630ed1898de604b9b571fda18e1e Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Thu, 27 May 2021 22:39:29 -0700 Subject: [PATCH 10/28] rollup: correct gas payment comparison --- l2geth/rollup/fees/rollup_fee.go | 5 +++-- l2geth/rollup/sync_service.go | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/l2geth/rollup/fees/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go index 7dcf9130f88e..9d5fa8140672 100644 --- a/l2geth/rollup/fees/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -20,7 +20,7 @@ const FeeScalar uint64 = 1000 var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) var bigHundredBillion = new(big.Int).SetUint64(hundredBillion) -var bigFeeScalar = new(big.Int).SetUint64(FeeScalar) +var BigFeeScalar = new(big.Int).SetUint64(FeeScalar) // errInvalidGasPrice is the error returned when a user submits an incorrect gas // price. The gas price must satisfy a particular equation depending on if it @@ -32,6 +32,7 @@ var errInvalidGasPrice = errors.New("rollup fee: invalid gas price") // executing the transaction on L2 // fee = (floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit) * tx.gasPrice // where tx.gasPrice is hard coded to 1000 * wei +// This function computes tx.gasLimit func CalculateRollupFee(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) (*big.Int, error) { if err := VerifyGasPrice(l1GasPrice); err != nil { return nil, fmt.Errorf("invalid L1 gas price %d: %w", l1GasPrice, err) @@ -43,7 +44,7 @@ func CalculateRollupFee(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int l1Fee := new(big.Int).Mul(l1GasPrice, l1GasLimit) l2Fee := new(big.Int).Mul(l2GasLimit, l2GasPrice) sum := new(big.Int).Add(l1Fee, l2Fee) - scaled := new(big.Int).Div(sum, bigFeeScalar) + scaled := new(big.Int).Div(sum, BigFeeScalar) result := new(big.Int).Add(scaled, l2GasLimit) return result, nil } diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index 63fad8736637..974d63299ebf 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -760,9 +760,11 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { if !fee.IsUint64() { return fmt.Errorf("fee overflow: %s", fee.String()) } + paying := new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice()) + expecting := new(big.Int).Mul(fee, fees.BigFeeScalar) // Make sure that the fee is paid - if tx.Gas() < fee.Uint64() { - return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", tx.Gas(), fee.Uint64(), fees.FeeScalar) + if paying.Cmp(expecting) < 0 { + return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.FeeScalar) } return nil } From 5657872ad2b7295c4379c38128db1f9af1886ea9 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Fri, 28 May 2021 14:23:25 +0300 Subject: [PATCH 11/28] fix(integration-tests): do not hardcode gas price --- integration-tests/test/fee-payment.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration-tests/test/fee-payment.spec.ts b/integration-tests/test/fee-payment.spec.ts index d57718539819..e1a5bcb174f7 100644 --- a/integration-tests/test/fee-payment.spec.ts +++ b/integration-tests/test/fee-payment.spec.ts @@ -38,8 +38,7 @@ describe('Fee Payment Integration Tests', async () => { const balanceBefore = await env.l2Wallet.getBalance() expect(balanceBefore.gt(amount)) - const gas = await env.ovmEth.estimateGas.transfer(other, amount) - const tx = await env.ovmEth.transfer(other, amount, { gasPrice: 1 }) + const tx = await env.ovmEth.transfer(other, amount) const receipt = await tx.wait() expect(receipt.status).to.eq(1) From dcf5a89bbf8f09d1a18a073e2dd1edc4f34c90c4 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:26:05 -0700 Subject: [PATCH 12/28] core-utils: update with new scheme --- packages/core-utils/src/fees.ts | 62 ++--------- packages/core-utils/test/fees/fees.spec.ts | 120 +++++++-------------- 2 files changed, 47 insertions(+), 135 deletions(-) diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index f0301c86ffdc..7ad629fc58de 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -6,11 +6,11 @@ import { BigNumber } from 'ethers' import { remove0x } from './common' const hundredMillion = BigNumber.from(100_000_000) -const hundredBillion = BigNumber.from(100_000_000_000) -const feeScalar = BigNumber.from(1000) +const feeScalar = 1000 +const L2GasPrice = BigNumber.from(feeScalar + (feeScalar / 2)) const txDataZeroGas = 4 const txDataNonZeroGasEIP2028 = 16 -const overhead = 4200 +const overhead = 4200 + (200 * txDataNonZeroGasEIP2028) export interface EncodableL2GasLimit { data: Buffer | string @@ -31,32 +31,15 @@ function encode(input: EncodableL2GasLimit): BigNumber { if (typeof l2GasPrice === 'number') { l2GasPrice = BigNumber.from(l2GasPrice) } - - if (!verifyGasPrice(l2GasPrice)) { - throw new Error(`Invalid L2 Gas Price: ${l2GasPrice.toString()}`) - } - if (!verifyGasPrice(l1GasPrice)) { - throw new Error(`Invalid L1 Gas Price: ${l1GasPrice.toString()}`) - } const l1GasLimit = calculateL1GasLimit(data) const l1Fee = l1GasLimit.mul(l1GasPrice) const l2Fee = l2GasLimit.mul(l2GasPrice) const sum = l1Fee.add(l2Fee) const scaled = sum.div(feeScalar) - return scaled.add(l2GasLimit) -} - -function verifyGasPrice(gasPrice: BigNumber | number): boolean { - if (typeof gasPrice === 'number') { - gasPrice = BigNumber.from(gasPrice) - } - if (gasPrice.eq(0)) { - return true - } - if (gasPrice.lt(hundredBillion)) { - return false - } - return gasPrice.mod(hundredMillion).eq(0) + const remainder = scaled.mod(hundredMillion) + const scaledSum = scaled.add(hundredMillion) + const rounded = scaledSum.sub(remainder) + return rounded.add(l2GasLimit) } function decode(fee: BigNumber | number): BigNumber { @@ -69,21 +52,7 @@ function decode(fee: BigNumber | number): BigNumber { export const L2GasLimit = { encode, decode, - feeScalar, -} - -export function verifyL2GasPrice(gasPrice: BigNumber | number): boolean { - if (typeof gasPrice === 'number') { - gasPrice = BigNumber.from(gasPrice) - } - return gasPrice.mod(hundredMillion).eq(0) -} - -export function verifyL1GasPrice(gasPrice: BigNumber | number): boolean { - if (typeof gasPrice === 'number') { - gasPrice = BigNumber.from(gasPrice) - } - return gasPrice.mod(hundredMillion).eq(0) + L2GasPrice, } export function calculateL1GasLimit(data: string | Buffer): BigNumber { @@ -109,18 +78,3 @@ export function zeroesAndOnes(data: Buffer | string): Array { } return [zeros, ones] } - -export function roundGasPrice(gasPrice: BigNumber | number): BigNumber { - if (typeof gasPrice === 'number') { - gasPrice = BigNumber.from(gasPrice) - } - if (gasPrice.eq(0)) { - return gasPrice - } - if (gasPrice.mod(hundredBillion).eq(0)) { - return gasPrice - } - const sum = gasPrice.add(hundredBillion) - const mod = gasPrice.mod(hundredBillion) - return sum.sub(mod) -} diff --git a/packages/core-utils/test/fees/fees.spec.ts b/packages/core-utils/test/fees/fees.spec.ts index bee3cf8c7215..c5cf18f1e76c 100644 --- a/packages/core-utils/test/fees/fees.spec.ts +++ b/packages/core-utils/test/fees/fees.spec.ts @@ -1,6 +1,6 @@ import { expect } from '../setup' import * as fees from '../../src/fees' -import { BigNumber } from 'ethers' +import { BigNumber, utils } from 'ethers' const hundredBillion = 10 ** 11 const million = 10 ** 6 @@ -21,40 +21,42 @@ describe('Fees', () => { } }) - describe('Round Gas Price', () => { - const roundGasPriceTests = [ - { input: 10, expect: hundredBillion, name: 'simple' }, - { - input: hundredBillion + 1, - expect: 2 * hundredBillion, - name: 'one-over', - }, - { input: hundredBillion, expect: hundredBillion, name: 'exact' }, - { input: hundredBillion - 1, expect: hundredBillion, name: 'one-under' }, - { input: 3, expect: hundredBillion, name: 'small' }, - { input: 2, expect: hundredBillion, name: 'two' }, - { input: 1, expect: hundredBillion, name: 'one' }, - { input: 0, expect: 0, name: 'zero' }, - ] - - for (const test of roundGasPriceTests) { - it(`should pass for ${test.name} case`, () => { - const got = fees.roundGasPrice(test.input) - const expected = BigNumber.from(test.expect) - expect(got).to.deep.equal(expected) - }) - } - }) - describe('Rollup Fees', () => { const rollupFeesTests = [ { name: 'simple', dataLen: 10, - l1GasPrice: hundredBillion, - l2GasPrice: hundredBillion, + l1GasPrice: utils.parseUnits('1', 'gwei'), + l2GasPrice: utils.parseUnits('1', 'gwei'), l2GasLimit: 437118, - error: false, + }, + { + name: 'small-gasprices-max-gaslimit', + dataLen: 10, + l1GasPrice: utils.parseUnits('1', 'wei'), + l2GasPrice: utils.parseUnits('1', 'wei'), + l2GasLimit: 0x4ffffff, + }, + { + name: 'large-gasprices-max-gaslimit', + dataLen: 10, + l1GasPrice: utils.parseUnits('1', 'ether'), + l2GasPrice: utils.parseUnits('1', 'ether'), + l2GasLimit: 0x4ffffff, + }, + { + name: 'small-gasprices-max-gaslimit', + dataLen: 10, + l1GasPrice: utils.parseUnits('1', 'ether'), + l2GasPrice: utils.parseUnits('1', 'ether'), + l2GasLimit: 1, + }, + { + name: 'max-gas-limit', + dataLen: 10, + l1GasPrice: utils.parseUnits('5', 'ether'), + l2GasPrice: utils.parseUnits('5', 'ether'), + l2GasLimit: 10**8-1, }, { name: 'zero-l2-gasprice', @@ -62,7 +64,6 @@ describe('Fees', () => { l1GasPrice: hundredBillion, l2GasPrice: 0, l2GasLimit: 196205, - error: false, }, { name: 'one-l2-gasprice', @@ -70,7 +71,6 @@ describe('Fees', () => { l1GasPrice: hundredBillion, l2GasPrice: 1, l2GasLimit: 196205, - error: true, }, { name: 'zero-l1-gasprice', @@ -78,7 +78,6 @@ describe('Fees', () => { l1GasPrice: 0, l2GasPrice: hundredBillion, l2GasLimit: 196205, - error: false, }, { name: 'one-l1-gasprice', @@ -86,7 +85,6 @@ describe('Fees', () => { l1GasPrice: 1, l2GasPrice: hundredBillion, l2GasLimit: 23255, - error: true, }, { name: 'zero-gasprices', @@ -94,34 +92,6 @@ describe('Fees', () => { l1GasPrice: 0, l2GasPrice: 0, l2GasLimit: 23255, - error: false, - }, - { - name: 'bad-l2-gasprice', - dataLen: 10, - l1GasPrice: 0, - l2GasPrice: hundredBillion - 1, - l2GasLimit: 23255, - error: true, - }, - { - name: 'bad-l1-gasprice', - dataLen: 10, - l1GasPrice: hundredBillion - 1, - l2GasPrice: hundredBillion, - l2GasLimit: 44654, - error: true, - }, - // The largest possible gaslimit that can be represented - // is 0x04ffffff which is plenty high enough to cover the - // L2 gas limit - { - name: 'max-gaslimit', - dataLen: 10, - l1GasPrice: hundredBillion, - l2GasPrice: hundredBillion, - l2GasLimit: 0x4ffffff, - error: false, }, { name: 'larger-divisor', @@ -129,33 +99,21 @@ describe('Fees', () => { l1GasPrice: 0, l2GasLimit: 10, l2GasPrice: 0, - error: false, }, ] for (const test of rollupFeesTests) { it(`should pass for ${test.name} case`, () => { const data = Buffer.alloc(test.dataLen) + const got = fees.L2GasLimit.encode({ + data, + l1GasPrice: test.l1GasPrice, + l2GasPrice: test.l2GasPrice, + l2GasLimit: test.l2GasLimit, + }) - let got - let err = false - try { - got = fees.L2GasLimit.encode({ - data, - l1GasPrice: test.l1GasPrice, - l2GasPrice: test.l2GasPrice, - l2GasLimit: test.l2GasLimit, - }) - } catch (e) { - err = true - } - - expect(err).to.equal(test.error) - - if (!err) { - const decoded = fees.L2GasLimit.decode(got) - expect(decoded).to.deep.eq(BigNumber.from(test.l2GasLimit)) - } + const decoded = fees.L2GasLimit.decode(got) + expect(decoded).to.deep.eq(BigNumber.from(test.l2GasLimit)) }) } }) From a1c1826f1734aa49d8471d3648bad3ebc6ed4a93 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:26:33 -0700 Subject: [PATCH 13/28] l2geth: refactor rollup oracle --- l2geth/eth/gasprice/rollup_gasprice.go | 49 +++++++++++--------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/l2geth/eth/gasprice/rollup_gasprice.go b/l2geth/eth/gasprice/rollup_gasprice.go index 05216c65aac6..7b0a062e6c41 100644 --- a/l2geth/eth/gasprice/rollup_gasprice.go +++ b/l2geth/eth/gasprice/rollup_gasprice.go @@ -6,61 +6,54 @@ import ( "sync" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/rollup/fees" ) // RollupOracle holds the L1 and L2 gas prices for fee calculation type RollupOracle struct { - dataPrice *big.Int - executionPrice *big.Int - dataPriceLock sync.RWMutex - executionPriceLock sync.RWMutex + l1GasPrice *big.Int + l2GasPrice *big.Int + l1GasPriceLock sync.RWMutex + l2GasPriceLock sync.RWMutex } // NewRollupOracle returns an initialized RollupOracle -func NewRollupOracle(dataPrice *big.Int, executionPrice *big.Int) *RollupOracle { +func NewRollupOracle(l1GasPrice *big.Int, l2GasPrice *big.Int) *RollupOracle { return &RollupOracle{ - dataPrice: dataPrice, - executionPrice: executionPrice, + l1GasPrice: l1GasPrice, + l2GasPrice: l2GasPrice, } } // SuggestL1GasPrice returns the gas price which should be charged per byte of published // data by the sequencer. func (gpo *RollupOracle) SuggestL1GasPrice(ctx context.Context) (*big.Int, error) { - gpo.dataPriceLock.RLock() - defer gpo.dataPriceLock.RUnlock() - return gpo.dataPrice, nil + gpo.l1GasPriceLock.RLock() + defer gpo.l1GasPriceLock.RUnlock() + return gpo.l1GasPrice, nil } // SetL1GasPrice returns the current L1 gas price func (gpo *RollupOracle) SetL1GasPrice(gasPrice *big.Int) error { - gpo.dataPriceLock.Lock() - defer gpo.dataPriceLock.Unlock() - if err := fees.VerifyGasPrice(gasPrice); err != nil { - return err - } - gpo.dataPrice = gasPrice - log.Info("Set L1 Gas Price", "gasprice", gpo.dataPrice) + gpo.l1GasPriceLock.Lock() + defer gpo.l1GasPriceLock.Unlock() + gpo.l1GasPrice = gasPrice + log.Info("Set L1 Gas Price", "gasprice", gpo.l1GasPrice) return nil } // SuggestL2GasPrice returns the gas price which should be charged per unit of gas // set manually by the sequencer depending on congestion func (gpo *RollupOracle) SuggestL2GasPrice(ctx context.Context) (*big.Int, error) { - gpo.executionPriceLock.RLock() - defer gpo.executionPriceLock.RUnlock() - return gpo.executionPrice, nil + gpo.l2GasPriceLock.RLock() + defer gpo.l2GasPriceLock.RUnlock() + return gpo.l2GasPrice, nil } // SetL2GasPrice returns the current L2 gas price func (gpo *RollupOracle) SetL2GasPrice(gasPrice *big.Int) error { - gpo.executionPriceLock.Lock() - defer gpo.executionPriceLock.Unlock() - if err := fees.VerifyGasPrice(gasPrice); err != nil { - return err - } - gpo.executionPrice = gasPrice - log.Info("Set L2 Gas Price", "gasprice", gpo.executionPrice) + gpo.l2GasPriceLock.Lock() + defer gpo.l2GasPriceLock.Unlock() + gpo.l2GasPrice = gasPrice + log.Info("Set L2 Gas Price", "gasprice", gpo.l2GasPrice) return nil } From bf2b5243825176aff4e01624290d12427d4fb163 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:27:08 -0700 Subject: [PATCH 14/28] l2geth: clean up DoEstimateGas --- l2geth/internal/ethapi/api.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index 8a1cc5e5ba51..6d1264fcb3db 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -51,7 +51,7 @@ import ( ) const ( - defaultGasPrice = params.Wei * fees.FeeScalar + defaultGasPrice = params.Wei * fees.L2GasPrice ) var errOVMUnsupported = errors.New("OVM: Unsupported RPC Method") @@ -1039,31 +1039,27 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash if err != nil { return 0, err } - // 2a. fetch the data price, depends on how the sequencer has chosen to update their values based on the // l1 gas prices l1GasPrice, err := b.SuggestL1GasPrice(ctx) if err != nil { return 0, err } - // 2b. fetch the execution gas price, by the typical mempool dynamics l2GasPrice, err := b.SuggestL2GasPrice(ctx) if err != nil { return 0, err } - - var data []byte - if args.Data == nil { - data = []byte{} - } else { + data := []byte{} + if args.Data != nil { data = *args.Data } - // 3. calculate the fee + // 3. calculate the fee using just the calldata. The additional overhead of + // RLP encoding is covered inside of EncodeL2GasLimit l2GasLimit := new(big.Int).SetUint64(uint64(gasUsed)) - fee, err := fees.CalculateRollupFee(data, l1GasPrice, l2GasLimit, l2GasPrice) - if err != nil { - return 0, err + fee := fees.EncodeL2GasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) + if !fee.IsUint64() { + return 0, fmt.Errorf("estimate gas overflow: %s", fee) } return (hexutil.Uint64)(fee.Uint64()), nil } From 481b83b640843a13c99d3b43c21ca385a19e58a7 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:27:34 -0700 Subject: [PATCH 15/28] l2geth: implement latest scheme --- l2geth/rollup/fees/rollup_fee.go | 99 +++++++++++---------------- l2geth/rollup/fees/rollup_fee_test.go | 83 ++++------------------ l2geth/rollup/sync_service.go | 43 ++++++------ 3 files changed, 74 insertions(+), 151 deletions(-) diff --git a/l2geth/rollup/fees/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go index 9d5fa8140672..e462fe6096bc 100644 --- a/l2geth/rollup/fees/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -1,68 +1,64 @@ package fees import ( - "errors" - "fmt" "math/big" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/params" ) // overhead represents the fixed cost of batch submission of a single -// transaction in gas -const overhead uint64 = 4200 +// transaction in gas. +const overhead uint64 = 4200 + 200*params.TxDataNonZeroGasEIP2028 // hundredMillion is a constant used in the gas encoding formula const hundredMillion uint64 = 100_000_000 -const hundredBillion uint64 = 100_000_000_000 -const FeeScalar uint64 = 1000 -var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) -var bigHundredBillion = new(big.Int).SetUint64(hundredBillion) -var BigFeeScalar = new(big.Int).SetUint64(FeeScalar) +// feeScalar is used to scale the calculations in EncodeL2GasLimit +// to prevent them from being too large +const feeScalar uint64 = 1000 + +// L2GasPrice is a constant that determines the result of `eth_gasPrice` +// It is scaled upwards by 50% +const L2GasPrice uint64 = feeScalar + (feeScalar / 2) -// errInvalidGasPrice is the error returned when a user submits an incorrect gas -// price. The gas price must satisfy a particular equation depending on if it -// is a L1 gas price or a L2 gas price -var errInvalidGasPrice = errors.New("rollup fee: invalid gas price") +// BigL2GasPrice is the L2GasPrice as type big.Int +var BigL2GasPrice = new(big.Int).SetUint64(L2GasPrice) +var bigFeeScalar = new(big.Int).SetUint64(feeScalar) +var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) -// CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into -// account both the cost of submitting the transaction to L1 as well as -// executing the transaction on L2 -// fee = (floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit) * tx.gasPrice +// EncodeL2GasLimit computes the `tx.gasLimit` based on the L1/L2 gas prices and +// the L2 gas limit. The L2 gas limit is encoded inside of the lower order bits +// of the number like so: [ | l2GasLimit ] +// [ tx.gaslimit ] +// The lower order bits must be large enough to fit the L2 gas limit, so 10**8 +// is chosen. If higher order bits collide with any bits from the L2 gas limit, +// the L2 gas limit will not be able to be decoded. +// An explicit design goal of this scheme was to make the L2 gas limit be human +// readable. The entire number is interpreted as the gas limit when computing +// the fee, so increasing the L2 Gas limit will increase the fee paid. +// The calculation is: +// floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit // where tx.gasPrice is hard coded to 1000 * wei -// This function computes tx.gasLimit -func CalculateRollupFee(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) (*big.Int, error) { - if err := VerifyGasPrice(l1GasPrice); err != nil { - return nil, fmt.Errorf("invalid L1 gas price %d: %w", l1GasPrice, err) - } - if err := VerifyGasPrice(l2GasPrice); err != nil { - return nil, fmt.Errorf("invalid L2 gas price %d: %w", l2GasPrice, err) - } +// Note that for simplicity purposes, only the calldata is passed into this +// function when in reality the RLP encoded transaction should be. The +// additional cost is added to the overhead constant to prevent the need to RLP +// encode transactions during calls to `eth_estimateGas` +func EncodeL2GasLimit(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) *big.Int { l1GasLimit := calculateL1GasLimit(data, overhead) l1Fee := new(big.Int).Mul(l1GasPrice, l1GasLimit) l2Fee := new(big.Int).Mul(l2GasLimit, l2GasPrice) sum := new(big.Int).Add(l1Fee, l2Fee) - scaled := new(big.Int).Div(sum, BigFeeScalar) - result := new(big.Int).Add(scaled, l2GasLimit) - return result, nil + scaled := new(big.Int).Div(sum, bigFeeScalar) + remainder := new(big.Int).Mod(scaled, bigHundredMillion) + scaledSum := new(big.Int).Add(scaled, bigHundredMillion) + rounded := new(big.Int).Sub(scaledSum, remainder) + result := new(big.Int).Add(rounded, l2GasLimit) + return result } -// VerifyGasPrice returns an error if the gas price doesn't satisfy -// the constraints. -func VerifyGasPrice(gasPrice *big.Int) error { - if gasPrice.Cmp(common.Big0) == 0 { - return nil - } - if gasPrice.Cmp(bigHundredBillion) < 0 { - return fmt.Errorf("too small: %w", errInvalidGasPrice) - } - mod := new(big.Int).Mod(gasPrice, bigHundredMillion) - if mod.Cmp(common.Big0) != 0 { - return fmt.Errorf("incorrect multiple: %w", errInvalidGasPrice) - } - return nil +// DecodeL2GasLimit decodes the L2 gas limit from an encoded L2 gas limit +func DecodeL2GasLimit(gasLimit *big.Int) *big.Int { + return new(big.Int).Mod(gasLimit, bigHundredMillion) } // calculateL1GasLimit computes the L1 gasLimit based on the calldata and @@ -77,23 +73,6 @@ func calculateL1GasLimit(data []byte, overhead uint64) *big.Int { return new(big.Int).SetUint64(gasLimit) } -// RoundGasPrice rounds the gas price up to the next valid gas price -func RoundGasPrice(gasPrice *big.Int) *big.Int { - if gasPrice.Cmp(common.Big0) == 0 { - return new(big.Int) - } - mod := new(big.Int).Mod(gasPrice, bigHundredBillion) - if mod.Cmp(common.Big0) == 0 { - return gasPrice - } - sum := new(big.Int).Add(gasPrice, bigHundredBillion) - return new(big.Int).Sub(sum, mod) -} - -func DecodeL2GasLimit(gasLimit *big.Int) *big.Int { - return new(big.Int).Mod(gasLimit, bigHundredMillion) -} - func zeroesAndOnes(data []byte) (uint64, uint64) { var zeroes uint64 var ones uint64 diff --git a/l2geth/rollup/fees/rollup_fee_test.go b/l2geth/rollup/fees/rollup_fee_test.go index 9a0d9aea0228..278f955200b5 100644 --- a/l2geth/rollup/fees/rollup_fee_test.go +++ b/l2geth/rollup/fees/rollup_fee_test.go @@ -1,35 +1,11 @@ package fees import ( - "errors" "math/big" "testing" -) -var roundingGasPriceTests = map[string]struct { - input uint64 - expect uint64 -}{ - "simple": {10, hundredBillion}, - "one-over": {hundredBillion + 1, 2 * hundredBillion}, - "exact": {hundredBillion, hundredBillion}, - "one-under": {hundredBillion - 1, hundredBillion}, - "small": {3, hundredBillion}, - "two": {2, hundredBillion}, - "one": {1, hundredBillion}, - "zero": {0, 0}, -} - -func TestRoundGasPrice(t *testing.T) { - for name, tt := range roundingGasPriceTests { - t.Run(name, func(t *testing.T) { - got := RoundGasPrice(new(big.Int).SetUint64(tt.input)) - if got.Uint64() != tt.expect { - t.Fatalf("Mismatched rounding to nearest, got %d expected %d", got, tt.expect) - } - }) - } -} + "github.com/ethereum/go-ethereum/params" +) var l1GasLimitTests = map[string]struct { data []byte @@ -58,77 +34,54 @@ var feeTests = map[string]struct { l1GasPrice uint64 l2GasLimit uint64 l2GasPrice uint64 - err error }{ "simple": { dataLen: 10, - l1GasPrice: hundredBillion, + l1GasPrice: params.GWei, l2GasLimit: 437118, - l2GasPrice: hundredBillion, - err: nil, + l2GasPrice: params.GWei, }, "zero-l2-gasprice": { dataLen: 10, - l1GasPrice: hundredBillion, + l1GasPrice: params.GWei, l2GasLimit: 196205, l2GasPrice: 0, - err: nil, }, "one-l2-gasprice": { dataLen: 10, - l1GasPrice: hundredBillion, + l1GasPrice: params.GWei, l2GasLimit: 196205, l2GasPrice: 1, - err: errInvalidGasPrice, }, "zero-l1-gasprice": { dataLen: 10, l1GasPrice: 0, l2GasLimit: 196205, - l2GasPrice: hundredBillion, - err: nil, + l2GasPrice: params.GWei, }, "one-l1-gasprice": { dataLen: 10, l1GasPrice: 1, l2GasLimit: 23255, - l2GasPrice: hundredBillion, - err: errInvalidGasPrice, + l2GasPrice: params.GWei, }, "zero-gasprices": { dataLen: 10, l1GasPrice: 0, l2GasLimit: 23255, l2GasPrice: 0, - err: nil, - }, - "bad-l2-gasprice": { - dataLen: 10, - l1GasPrice: 0, - l2GasLimit: 23255, - l2GasPrice: hundredBillion - 1, - err: errInvalidGasPrice, - }, - "bad-l1-gasprice": { - dataLen: 10, - l1GasPrice: hundredBillion - 1, - l2GasLimit: 44654, - l2GasPrice: hundredBillion, - err: errInvalidGasPrice, }, "max-gaslimit": { dataLen: 10, - l1GasPrice: hundredBillion, - l2GasLimit: 0x4ffffff, - l2GasPrice: hundredBillion, - err: nil, + l1GasPrice: params.GWei, + l2GasLimit: 99999999, + l2GasPrice: params.GWei, }, "larger-divisor": { dataLen: 10, l1GasPrice: 0, l2GasLimit: 10, l2GasPrice: 0, - err: nil, }, } @@ -140,16 +93,10 @@ func TestCalculateRollupFee(t *testing.T) { l2GasLimit := new(big.Int).SetUint64(tt.l2GasLimit) l2GasPrice := new(big.Int).SetUint64(tt.l2GasPrice) - fee, err := CalculateRollupFee(data, l1GasPrice, l2GasLimit, l2GasPrice) - if !errors.Is(err, tt.err) { - t.Fatalf("Cannot calculate fee: %s", err) - } - - if err == nil { - decodedGasLimit := DecodeL2GasLimit(fee) - if l2GasLimit.Cmp(decodedGasLimit) != 0 { - t.Errorf("rollup fee check failed: expected %d, got %d", l2GasLimit.Uint64(), decodedGasLimit) - } + fee := EncodeL2GasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) + decodedGasLimit := DecodeL2GasLimit(fee) + if l2GasLimit.Cmp(decodedGasLimit) != 0 { + t.Errorf("rollup fee check failed: expected %d, got %d", l2GasLimit.Uint64(), decodedGasLimit) } }) } diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index 974d63299ebf..c8e53ba69c9d 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -446,16 +446,7 @@ func (s *SyncService) updateL2GasPrice(hash *common.Hash) error { return err } result := state.GetState(s.gpoAddress, l2GasPriceSlot) - gasPrice := result.Big() - if err := fees.VerifyGasPrice(gasPrice); err != nil { - // If an invalid gas price is found in the state, then - // round it up to the next valid gas price. The contract - // should technically never allow for an invalid gas price. - gp := fees.RoundGasPrice(gasPrice) - log.Warn("Invalid gas price detected in state", "state", gasPrice, "using", gp) - gasPrice = gp - } - s.RollupGpo.SetL2GasPrice(gasPrice) + s.RollupGpo.SetL2GasPrice(result.Big()) return nil } @@ -732,11 +723,18 @@ func (s *SyncService) applyBatchedTransaction(tx *types.Transaction) error { // verifyFee will verify that a valid fee is being paid. func (s *SyncService) verifyFee(tx *types.Transaction) error { - // Exit early if fees are enforced and the gasPrice is set to 0 - if s.enforceFees && tx.GasPrice().Cmp(common.Big0) == 0 { - return errors.New("cannot accept 0 gas price transaction") + if tx.GasPrice().Cmp(common.Big0) == 0 { + // Exit early if fees are enforced and the gasPrice is set to 0 + if s.enforceFees { + return errors.New("cannot accept 0 gas price transaction") + } + // If fees are not enforced and the gas price is 0, return early + return nil + } + // When the gas price is non zero, it must be equal to the constant + if tx.GasPrice().Cmp(fees.BigL2GasPrice) != 0 { + return fmt.Errorf("tx.gasPrice must be %d", fees.L2GasPrice) } - l1GasPrice, err := s.RollupGpo.SuggestL1GasPrice(context.Background()) if err != nil { return err @@ -748,23 +746,22 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { // Calculate the fee based on decoded L2 gas limit gas := new(big.Int).SetUint64(tx.Gas()) l2GasLimit := fees.DecodeL2GasLimit(gas) - fee, err := fees.CalculateRollupFee(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) + // Only count the calldata here as the overhead of the fully encoded + // RLP transaction is handled inside of EncodeL2GasLimit + fee := fees.EncodeL2GasLimit(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) if err != nil { return err } - // If fees are not enforced and the gas price is 0, return early - if !s.enforceFees && tx.GasPrice().Cmp(common.Big0) == 0 { - return nil - } // This should only happen if the transaction fee is greater than 18.44 ETH if !fee.IsUint64() { return fmt.Errorf("fee overflow: %s", fee.String()) } + // Compute the user's fee paying := new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice()) - expecting := new(big.Int).Mul(fee, fees.BigFeeScalar) - // Make sure that the fee is paid - if paying.Cmp(expecting) < 0 { - return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.FeeScalar) + // Compute the minimum expected fee + expecting := new(big.Int).Mul(fee, fees.BigL2GasPrice) + if paying.Cmp(expecting) == -1 { + return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.BigL2GasPrice) } return nil } From 7ed10fb393d6cc31e57613bc602a3b86b87cce1b Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:27:47 -0700 Subject: [PATCH 16/28] tests: fix up --- integration-tests/test/erc20.spec.ts | 2 +- integration-tests/test/fee-payment.spec.ts | 2 +- integration-tests/test/native-eth.spec.ts | 4 +-- integration-tests/test/rpc.spec.ts | 30 +++++++++++++++------- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/integration-tests/test/erc20.spec.ts b/integration-tests/test/erc20.spec.ts index ea414a715cf9..fca9cb637258 100644 --- a/integration-tests/test/erc20.spec.ts +++ b/integration-tests/test/erc20.spec.ts @@ -67,7 +67,7 @@ describe('Basic ERC20 interactions', async () => { // The expected fee paid is the value returned by eth_estimateGas const gasLimit = await ERC20.estimateGas.transfer(other.address, 100) const gasPrice = await wallet.getGasPrice() - expect(gasPrice).to.deep.equal(L2GasLimit.feeScalar) + expect(gasPrice).to.deep.equal(L2GasLimit.L2GasPrice) const expectedFeePaid = gasLimit.mul(gasPrice) // There are two events from the transfer with the first being diff --git a/integration-tests/test/fee-payment.spec.ts b/integration-tests/test/fee-payment.spec.ts index e1a5bcb174f7..39f3e52a5d14 100644 --- a/integration-tests/test/fee-payment.spec.ts +++ b/integration-tests/test/fee-payment.spec.ts @@ -21,7 +21,7 @@ describe('Fee Payment Integration Tests', async () => { it('Should estimateGas with recoverable L2 gasLimit', async () => { const gas = await env.ovmEth.estimateGas.transfer( other, - utils.parseEther('0.5') + utils.parseEther('0.5'), ) const tx = await env.ovmEth.populateTransaction.transfer( other, diff --git a/integration-tests/test/native-eth.spec.ts b/integration-tests/test/native-eth.spec.ts index 91d66a5b09c2..fc11c82c8be6 100644 --- a/integration-tests/test/native-eth.spec.ts +++ b/integration-tests/test/native-eth.spec.ts @@ -45,13 +45,13 @@ describe('Native ETH Integration Tests', async () => { const amount = utils.parseEther('0.5') const addr = '0x' + '1234'.repeat(10) const gas = await env.ovmEth.estimateGas.transfer(addr, amount) - expect(gas).to.be.deep.eq(BigNumber.from(0x7080f9de6d)) + expect(gas).to.be.deep.eq(BigNumber.from(0x0ef897216d)) }) it('Should estimate gas for ETH withdraw', async () => { const amount = utils.parseEther('0.5') const gas = await env.ovmEth.estimateGas.withdraw(amount) - expect(gas).to.be.deep.eq(BigNumber.from(0x67ef8ae7b4)) + expect(gas).to.be.deep.eq(BigNumber.from(61400489396)) }) }) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index c1ab4ea9ba92..334434d511c7 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -1,13 +1,12 @@ import { injectL2Context, L2GasLimit, - roundGasPrice, toRpcHexString, } from '@eth-optimism/core-utils' import { Wallet, BigNumber, Contract } from 'ethers' import { ethers } from 'hardhat' import chai, { expect } from 'chai' -import { sleep, l2Provider, GWEI } from './shared/utils' +import { sleep, l2Provider, l1Provider } from './shared/utils' import chaiAsPromised from 'chai-as-promised' import { OptimismEnv } from './shared/env' import { @@ -131,11 +130,25 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: 1, + gasPrice: L2GasLimit.L2GasPrice, } + const fee = tx.gasPrice.mul(tx.gasLimit) + const gasLimit = 59300000001 await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - 'fee too low: 1, use at least tx.gasLimit = 420000000001 and tx.gasPrice = 1000' + `fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${L2GasLimit.L2GasPrice}` + ) + }) + + it('should reject a transaction with an incorrect gas price', async () => { + const tx = { + ...DEFAULT_TRANSACTION, + gasLimit: 1, + gasPrice: L2GasLimit.L2GasPrice.sub(1), + } + + await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( + `tx.gasPrice must be ${L2GasLimit.L2GasPrice}` ) }) @@ -314,7 +327,7 @@ describe('Basic RPC tests', () => { describe('eth_gasPrice', () => { it('gas price should be the fee scalar', async () => { expect(await provider.getGasPrice()).to.be.deep.equal( - L2GasLimit.feeScalar.toNumber() + L2GasLimit.L2GasPrice.toNumber() ) }) }) @@ -341,7 +354,7 @@ describe('Basic RPC tests', () => { to: DEFAULT_TRANSACTION.to, value: 0, }) - expect(estimate).to.be.eq(420000119751) + expect(estimate).to.be.eq(0x0dce9004c7) }) it('should return a gas estimate that grows with the size of data', async () => { @@ -352,7 +365,6 @@ describe('Basic RPC tests', () => { for (const data of dataLen) { const tx = { to: '0x' + '1234'.repeat(10), - gasPrice: toRpcHexString(100_000_000_000), value: '0x0', data: '0x' + '00'.repeat(data), from: '0x' + '1234'.repeat(10), @@ -366,12 +378,12 @@ describe('Basic RPC tests', () => { expect(decoded).to.deep.eq(BigNumber.from(l2Gaslimit)) expect(estimate.toString().endsWith(l2Gaslimit.toString())) + const l2GasPrice = BigNumber.from(0) // The L2GasPrice should be fetched from the L2GasPrice oracle contract, // but it does not yet exist. Use the default value for now - const l2GasPrice = BigNumber.from(0) const expected = L2GasLimit.encode({ data: tx.data, - l1GasPrice: roundGasPrice(l1GasPrice), + l1GasPrice, l2GasLimit: BigNumber.from(l2Gaslimit), l2GasPrice, }) From 0f9626125bc2d22216d3701b89cd5479c7415f0e Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:31:51 -0700 Subject: [PATCH 17/28] lint: fix --- integration-tests/test/fee-payment.spec.ts | 2 +- packages/core-utils/src/fees.ts | 4 ++-- packages/core-utils/test/fees/fees.spec.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/integration-tests/test/fee-payment.spec.ts b/integration-tests/test/fee-payment.spec.ts index 39f3e52a5d14..e1a5bcb174f7 100644 --- a/integration-tests/test/fee-payment.spec.ts +++ b/integration-tests/test/fee-payment.spec.ts @@ -21,7 +21,7 @@ describe('Fee Payment Integration Tests', async () => { it('Should estimateGas with recoverable L2 gasLimit', async () => { const gas = await env.ovmEth.estimateGas.transfer( other, - utils.parseEther('0.5'), + utils.parseEther('0.5') ) const tx = await env.ovmEth.populateTransaction.transfer( other, diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index 7ad629fc58de..fa441ef25522 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -7,10 +7,10 @@ import { remove0x } from './common' const hundredMillion = BigNumber.from(100_000_000) const feeScalar = 1000 -const L2GasPrice = BigNumber.from(feeScalar + (feeScalar / 2)) +const L2GasPrice = BigNumber.from(feeScalar + feeScalar / 2) const txDataZeroGas = 4 const txDataNonZeroGasEIP2028 = 16 -const overhead = 4200 + (200 * txDataNonZeroGasEIP2028) +const overhead = 4200 + 200 * txDataNonZeroGasEIP2028 export interface EncodableL2GasLimit { data: Buffer | string diff --git a/packages/core-utils/test/fees/fees.spec.ts b/packages/core-utils/test/fees/fees.spec.ts index c5cf18f1e76c..2e49ac8574d3 100644 --- a/packages/core-utils/test/fees/fees.spec.ts +++ b/packages/core-utils/test/fees/fees.spec.ts @@ -56,7 +56,7 @@ describe('Fees', () => { dataLen: 10, l1GasPrice: utils.parseUnits('5', 'ether'), l2GasPrice: utils.parseUnits('5', 'ether'), - l2GasLimit: 10**8-1, + l2GasLimit: 10 ** 8 - 1, }, { name: 'zero-l2-gasprice', From c4d76b0331d181041a7ab2cb24eaf44099297ee6 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 16:39:46 -0700 Subject: [PATCH 18/28] l2geth: better sycn service test --- l2geth/rollup/sync_service_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/l2geth/rollup/sync_service_test.go b/l2geth/rollup/sync_service_test.go index 43f8cf791c2c..705539be7fe4 100644 --- a/l2geth/rollup/sync_service_test.go +++ b/l2geth/rollup/sync_service_test.go @@ -20,7 +20,6 @@ import ( "github.com/ethereum/go-ethereum/eth/gasprice" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/params" - "github.com/ethereum/go-ethereum/rollup/fees" ) func setupLatestEthContextTest() (*SyncService, *EthContext) { @@ -508,7 +507,8 @@ func TestSyncServiceL1GasPrice(t *testing.T) { t.Fatal(err) } - if gasAfter.Cmp(fees.RoundGasPrice(big.NewInt(1))) != 0 { + expect, _ := service.client.GetL1GasPrice() + if gasAfter.Cmp(expect) != 0 { t.Fatal("expected 100 gas price, got", gasAfter) } } @@ -825,7 +825,7 @@ func (m *mockClient) SyncStatus(backend Backend) (*SyncStatus, error) { } func (m *mockClient) GetL1GasPrice() (*big.Int, error) { - price := fees.RoundGasPrice(big.NewInt(2)) + price := big.NewInt(1) return price, nil } From 636c41636bf1c7881637dd653a7d54121fbeefbb Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:05:20 -0700 Subject: [PATCH 19/28] optimism: rename to TxGasLimit --- integration-tests/test/erc20.spec.ts | 4 ++-- integration-tests/test/fee-payment.spec.ts | 4 ++-- integration-tests/test/rpc.spec.ts | 16 ++++++++-------- l2geth/internal/ethapi/api.go | 2 +- l2geth/rollup/fees/rollup_fee.go | 4 ++-- l2geth/rollup/fees/rollup_fee_test.go | 2 +- l2geth/rollup/sync_service.go | 2 +- packages/core-utils/src/fees.ts | 2 +- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/integration-tests/test/erc20.spec.ts b/integration-tests/test/erc20.spec.ts index fca9cb637258..3d356ab0e4ba 100644 --- a/integration-tests/test/erc20.spec.ts +++ b/integration-tests/test/erc20.spec.ts @@ -1,6 +1,6 @@ import { Contract, ContractFactory, Wallet } from 'ethers' import { ethers } from 'hardhat' -import { L2GasLimit } from '@eth-optimism/core-utils' +import { TxGasLimit } from '@eth-optimism/core-utils' import chai, { expect } from 'chai' import { GWEI } from './shared/utils' import { OptimismEnv } from './shared/env' @@ -67,7 +67,7 @@ describe('Basic ERC20 interactions', async () => { // The expected fee paid is the value returned by eth_estimateGas const gasLimit = await ERC20.estimateGas.transfer(other.address, 100) const gasPrice = await wallet.getGasPrice() - expect(gasPrice).to.deep.equal(L2GasLimit.L2GasPrice) + expect(gasPrice).to.deep.equal(TxGasLimit.L2GasPrice) const expectedFeePaid = gasLimit.mul(gasPrice) // There are two events from the transfer with the first being diff --git a/integration-tests/test/fee-payment.spec.ts b/integration-tests/test/fee-payment.spec.ts index e1a5bcb174f7..3583df4493c8 100644 --- a/integration-tests/test/fee-payment.spec.ts +++ b/integration-tests/test/fee-payment.spec.ts @@ -3,7 +3,7 @@ import chaiAsPromised from 'chai-as-promised' chai.use(chaiAsPromised) import { BigNumber, utils } from 'ethers' import { OptimismEnv } from './shared/env' -import { L2GasLimit } from '@eth-optimism/core-utils' +import { TxGasLimit } from '@eth-optimism/core-utils' describe('Fee Payment Integration Tests', async () => { let env: OptimismEnv @@ -29,7 +29,7 @@ describe('Fee Payment Integration Tests', async () => { ) const executionGas = await (env.ovmEth .provider as any).send('eth_estimateExecutionGas', [tx]) - const decoded = L2GasLimit.decode(gas) + const decoded = TxGasLimit.decode(gas) expect(BigNumber.from(executionGas)).deep.eq(decoded) }) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 334434d511c7..de6593a22f30 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -1,6 +1,6 @@ import { injectL2Context, - L2GasLimit, + TxGasLimit, toRpcHexString, } from '@eth-optimism/core-utils' import { Wallet, BigNumber, Contract } from 'ethers' @@ -130,13 +130,13 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: L2GasLimit.L2GasPrice, + gasPrice: TxGasLimit.L2GasPrice, } const fee = tx.gasPrice.mul(tx.gasLimit) const gasLimit = 59300000001 await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - `fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${L2GasLimit.L2GasPrice}` + `fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${TxGasLimit.L2GasPrice}` ) }) @@ -144,11 +144,11 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: L2GasLimit.L2GasPrice.sub(1), + gasPrice: TxGasLimit.L2GasPrice.sub(1), } await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - `tx.gasPrice must be ${L2GasLimit.L2GasPrice}` + `tx.gasPrice must be ${TxGasLimit.L2GasPrice}` ) }) @@ -327,7 +327,7 @@ describe('Basic RPC tests', () => { describe('eth_gasPrice', () => { it('gas price should be the fee scalar', async () => { expect(await provider.getGasPrice()).to.be.deep.equal( - L2GasLimit.L2GasPrice.toNumber() + TxGasLimit.L2GasPrice.toNumber() ) }) }) @@ -374,14 +374,14 @@ describe('Basic RPC tests', () => { tx, ]) - const decoded = L2GasLimit.decode(estimate) + const decoded = TxGasLimit.decode(estimate) expect(decoded).to.deep.eq(BigNumber.from(l2Gaslimit)) expect(estimate.toString().endsWith(l2Gaslimit.toString())) const l2GasPrice = BigNumber.from(0) // The L2GasPrice should be fetched from the L2GasPrice oracle contract, // but it does not yet exist. Use the default value for now - const expected = L2GasLimit.encode({ + const expected = TxGasLimit.encode({ data: tx.data, l1GasPrice, l2GasLimit: BigNumber.from(l2Gaslimit), diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index 6d1264fcb3db..b45d30a701d6 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -1057,7 +1057,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash // 3. calculate the fee using just the calldata. The additional overhead of // RLP encoding is covered inside of EncodeL2GasLimit l2GasLimit := new(big.Int).SetUint64(uint64(gasUsed)) - fee := fees.EncodeL2GasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) + fee := fees.EncodeTxGasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) if !fee.IsUint64() { return 0, fmt.Errorf("estimate gas overflow: %s", fee) } diff --git a/l2geth/rollup/fees/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go index e462fe6096bc..e665349fd411 100644 --- a/l2geth/rollup/fees/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -26,7 +26,7 @@ var BigL2GasPrice = new(big.Int).SetUint64(L2GasPrice) var bigFeeScalar = new(big.Int).SetUint64(feeScalar) var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) -// EncodeL2GasLimit computes the `tx.gasLimit` based on the L1/L2 gas prices and +// EncodeTxGasLimit computes the `tx.gasLimit` based on the L1/L2 gas prices and // the L2 gas limit. The L2 gas limit is encoded inside of the lower order bits // of the number like so: [ | l2GasLimit ] // [ tx.gaslimit ] @@ -43,7 +43,7 @@ var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) // function when in reality the RLP encoded transaction should be. The // additional cost is added to the overhead constant to prevent the need to RLP // encode transactions during calls to `eth_estimateGas` -func EncodeL2GasLimit(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) *big.Int { +func EncodeTxGasLimit(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) *big.Int { l1GasLimit := calculateL1GasLimit(data, overhead) l1Fee := new(big.Int).Mul(l1GasPrice, l1GasLimit) l2Fee := new(big.Int).Mul(l2GasLimit, l2GasPrice) diff --git a/l2geth/rollup/fees/rollup_fee_test.go b/l2geth/rollup/fees/rollup_fee_test.go index 278f955200b5..138a0d0bb16a 100644 --- a/l2geth/rollup/fees/rollup_fee_test.go +++ b/l2geth/rollup/fees/rollup_fee_test.go @@ -93,7 +93,7 @@ func TestCalculateRollupFee(t *testing.T) { l2GasLimit := new(big.Int).SetUint64(tt.l2GasLimit) l2GasPrice := new(big.Int).SetUint64(tt.l2GasPrice) - fee := EncodeL2GasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) + fee := EncodeTxGasLimit(data, l1GasPrice, l2GasLimit, l2GasPrice) decodedGasLimit := DecodeL2GasLimit(fee) if l2GasLimit.Cmp(decodedGasLimit) != 0 { t.Errorf("rollup fee check failed: expected %d, got %d", l2GasLimit.Uint64(), decodedGasLimit) diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index c8e53ba69c9d..7b58438b3cc6 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -748,7 +748,7 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { l2GasLimit := fees.DecodeL2GasLimit(gas) // Only count the calldata here as the overhead of the fully encoded // RLP transaction is handled inside of EncodeL2GasLimit - fee := fees.EncodeL2GasLimit(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) + fee := fees.EncodeTxGasLimit(tx.Data(), l1GasPrice, l2GasLimit, l2GasPrice) if err != nil { return err } diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index fa441ef25522..b3b44f2f62c2 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -49,7 +49,7 @@ function decode(fee: BigNumber | number): BigNumber { return fee.mod(hundredMillion) } -export const L2GasLimit = { +export const TxGasLimit = { encode, decode, L2GasPrice, From c21c2de443cbc2e0d8dc2b27e8bf164de3f55593 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:09:59 -0700 Subject: [PATCH 20/28] fee: fix docstring --- l2geth/rollup/fees/rollup_fee.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/l2geth/rollup/fees/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go index e665349fd411..70fdcd5ba154 100644 --- a/l2geth/rollup/fees/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -19,6 +19,8 @@ const feeScalar uint64 = 1000 // L2GasPrice is a constant that determines the result of `eth_gasPrice` // It is scaled upwards by 50% +// tx.gasPrice is hard coded to 1500 * wei and all transactions must set that +// gas price. const L2GasPrice uint64 = feeScalar + (feeScalar / 2) // BigL2GasPrice is the L2GasPrice as type big.Int @@ -37,8 +39,13 @@ var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) // readable. The entire number is interpreted as the gas limit when computing // the fee, so increasing the L2 Gas limit will increase the fee paid. // The calculation is: -// floor((l1GasLimit*l1GasPrice + l2GasLimit*l2GasPrice) / max(tx.gasPrice, 1)) + l2GasLimit -// where tx.gasPrice is hard coded to 1000 * wei +// l1GasLimit = zero_count(data) * 4 + non_zero_count(data) * 16 + overhead +// l1Fee = l1GasPrice * l1GasLimit +// l2Fee = l2GasPrice * l2GasLimit +// sum = l1Fee + l2Fee +// scaled = sum / scalar +// rounded = ceilmod(scaled, hundredMillion) +// result = rounded + l2GasLimit // Note that for simplicity purposes, only the calldata is passed into this // function when in reality the RLP encoded transaction should be. The // additional cost is added to the overhead constant to prevent the need to RLP @@ -46,7 +53,7 @@ var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) func EncodeTxGasLimit(data []byte, l1GasPrice, l2GasLimit, l2GasPrice *big.Int) *big.Int { l1GasLimit := calculateL1GasLimit(data, overhead) l1Fee := new(big.Int).Mul(l1GasPrice, l1GasLimit) - l2Fee := new(big.Int).Mul(l2GasLimit, l2GasPrice) + l2Fee := new(big.Int).Mul(l2GasPrice, l2GasLimit) sum := new(big.Int).Add(l1Fee, l2Fee) scaled := new(big.Int).Div(sum, bigFeeScalar) remainder := new(big.Int).Mod(scaled, bigHundredMillion) From b99a3905549324bf2b53ce4376cd0173f50e6cd4 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:10:52 -0700 Subject: [PATCH 21/28] tests: fix --- packages/core-utils/test/fees/fees.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core-utils/test/fees/fees.spec.ts b/packages/core-utils/test/fees/fees.spec.ts index 2e49ac8574d3..c624c0b3128b 100644 --- a/packages/core-utils/test/fees/fees.spec.ts +++ b/packages/core-utils/test/fees/fees.spec.ts @@ -105,14 +105,14 @@ describe('Fees', () => { for (const test of rollupFeesTests) { it(`should pass for ${test.name} case`, () => { const data = Buffer.alloc(test.dataLen) - const got = fees.L2GasLimit.encode({ + const got = fees.TxGasLimit.encode({ data, l1GasPrice: test.l1GasPrice, l2GasPrice: test.l2GasPrice, l2GasLimit: test.l2GasLimit, }) - const decoded = fees.L2GasLimit.decode(got) + const decoded = fees.TxGasLimit.decode(got) expect(decoded).to.deep.eq(BigNumber.from(test.l2GasLimit)) }) } From 008e1ff27459a063f73356749588e4b052178391 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:44:53 -0700 Subject: [PATCH 22/28] variables: rename --- integration-tests/test/rpc.spec.ts | 11 +++++---- l2geth/internal/ethapi/api.go | 2 +- l2geth/rollup/fees/rollup_fee.go | 8 +++--- l2geth/rollup/sync_service.go | 8 +++--- packages/core-utils/README.md | 39 ++++++++++++------------------ packages/core-utils/src/fees.ts | 3 +-- 6 files changed, 32 insertions(+), 39 deletions(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index de6593a22f30..cf2c0b2c5fcc 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -1,6 +1,7 @@ import { injectL2Context, TxGasLimit, + TxGasPrice, toRpcHexString, } from '@eth-optimism/core-utils' import { Wallet, BigNumber, Contract } from 'ethers' @@ -130,13 +131,13 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: TxGasLimit.L2GasPrice, + gasPrice: TxGasPrice.toNumber(), } const fee = tx.gasPrice.mul(tx.gasLimit) const gasLimit = 59300000001 await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - `fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${TxGasLimit.L2GasPrice}` + `fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${TxGasPrice.toString()}` ) }) @@ -144,11 +145,11 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: TxGasLimit.L2GasPrice.sub(1), + gasPrice: TxGasPrice.sub(1), } await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith( - `tx.gasPrice must be ${TxGasLimit.L2GasPrice}` + `tx.gasPrice must be ${TxGasPrice.toString()}` ) }) @@ -327,7 +328,7 @@ describe('Basic RPC tests', () => { describe('eth_gasPrice', () => { it('gas price should be the fee scalar', async () => { expect(await provider.getGasPrice()).to.be.deep.equal( - TxGasLimit.L2GasPrice.toNumber() + TxGasPrice.toNumber() ) }) }) diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index b45d30a701d6..648ddea679cd 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -51,7 +51,7 @@ import ( ) const ( - defaultGasPrice = params.Wei * fees.L2GasPrice + defaultGasPrice = params.Wei * fees.TxGasPrice ) var errOVMUnsupported = errors.New("OVM: Unsupported RPC Method") diff --git a/l2geth/rollup/fees/rollup_fee.go b/l2geth/rollup/fees/rollup_fee.go index 70fdcd5ba154..2f5455057e7d 100644 --- a/l2geth/rollup/fees/rollup_fee.go +++ b/l2geth/rollup/fees/rollup_fee.go @@ -17,14 +17,14 @@ const hundredMillion uint64 = 100_000_000 // to prevent them from being too large const feeScalar uint64 = 1000 -// L2GasPrice is a constant that determines the result of `eth_gasPrice` +// TxGasPrice is a constant that determines the result of `eth_gasPrice` // It is scaled upwards by 50% // tx.gasPrice is hard coded to 1500 * wei and all transactions must set that // gas price. -const L2GasPrice uint64 = feeScalar + (feeScalar / 2) +const TxGasPrice uint64 = feeScalar + (feeScalar / 2) -// BigL2GasPrice is the L2GasPrice as type big.Int -var BigL2GasPrice = new(big.Int).SetUint64(L2GasPrice) +// BigTxGasPrice is the L2GasPrice as type big.Int +var BigTxGasPrice = new(big.Int).SetUint64(TxGasPrice) var bigFeeScalar = new(big.Int).SetUint64(feeScalar) var bigHundredMillion = new(big.Int).SetUint64(hundredMillion) diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index 7b58438b3cc6..28a595a4f38c 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -732,8 +732,8 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { return nil } // When the gas price is non zero, it must be equal to the constant - if tx.GasPrice().Cmp(fees.BigL2GasPrice) != 0 { - return fmt.Errorf("tx.gasPrice must be %d", fees.L2GasPrice) + if tx.GasPrice().Cmp(fees.BigTxGasPrice) != 0 { + return fmt.Errorf("tx.gasPrice must be %d", fees.TxGasPrice) } l1GasPrice, err := s.RollupGpo.SuggestL1GasPrice(context.Background()) if err != nil { @@ -759,9 +759,9 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { // Compute the user's fee paying := new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice()) // Compute the minimum expected fee - expecting := new(big.Int).Mul(fee, fees.BigL2GasPrice) + expecting := new(big.Int).Mul(fee, fees.BigTxGasPrice) if paying.Cmp(expecting) == -1 { - return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.BigL2GasPrice) + return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.BigTxGasPrice) } return nil } diff --git a/packages/core-utils/README.md b/packages/core-utils/README.md index 20a6539f5293..fc01bdfdfe78 100644 --- a/packages/core-utils/README.md +++ b/packages/core-utils/README.md @@ -25,36 +25,29 @@ $ yarn lint ### L2 Fees -The Layer 2 fee is encoded in `tx.gasLimit`. The Layer 2 `gasLimit` is encoded -in the lower order bits of the `tx.gasLimit`. For this scheme to work, both the -L1 gas price and the L2 gas price must satisfy specific equations. There are -functions that help ensure that the correct gas prices are used. - -- `roundL1GasPrice` -- `roundL2GasPrice` - -The Layer 2 fee is based on both the cost of submitting the data to L1 as well -as the cost of execution on L2. To make libraries like `ethers` just work, the -return value of `eth_estimateGas` has been modified to return the fee. A new RPC -endpoint `eth_estimateExecutionGas` has been added that returns the L2 gas used. - -To locally encode the `tx.gasLimit`, the `L2GasLimit` methods `encode` and -`decode` should be used. +`TxGasLimit` can be used to `encode` and `decode` the L2 Gas Limit +locally. ```typescript -import { L2GasLimit, roundL1GasPrice, roundL2GasPrice } from '@eth-optimism/core-utils' +import { TxGasLimit } from '@eth-optimism/core-utils' import { JsonRpcProvider } from 'ethers' -const provider = new JsonRpcProvider('https://mainnet.optimism.io') -const gasLimit = await provider.send('eth_estimateExecutionGas', [tx]) +const L2Provider = new JsonRpcProvider('https://mainnet.optimism.io') +const L1Provider = new JsonRpcProvider('http://127.0.0.1:8545') -const encoded = L2GasLimit.encode({ +const l2GasLimit = await L2Provider.send('eth_estimateExecutionGas', [tx]) +const l1GasPrice = await L1Provider.getGasPrice() + +const encoded = TxGasLimit.encode({ data: '0x', - l1GasPrice: roundL1GasPrice(1), - l2GasLimit: gasLimit, - l2GasPrice: roundL2GasPrice(1), + l1GasPrice, + l2GasLimit, + l2GasPrice: 10000000, }) -const decoded = L2GasLimit.decode(encoded) +const decoded = TxGasLimit.decode(encoded) assert(decoded.eq(gasLimit)) + +const estimate = await L2Provider.estimateGas() +assert(estimate.eq(encoded)) ``` diff --git a/packages/core-utils/src/fees.ts b/packages/core-utils/src/fees.ts index b3b44f2f62c2..3910d19196df 100644 --- a/packages/core-utils/src/fees.ts +++ b/packages/core-utils/src/fees.ts @@ -7,7 +7,7 @@ import { remove0x } from './common' const hundredMillion = BigNumber.from(100_000_000) const feeScalar = 1000 -const L2GasPrice = BigNumber.from(feeScalar + feeScalar / 2) +export const TxGasPrice = BigNumber.from(feeScalar + feeScalar / 2) const txDataZeroGas = 4 const txDataNonZeroGasEIP2028 = 16 const overhead = 4200 + 200 * txDataNonZeroGasEIP2028 @@ -52,7 +52,6 @@ function decode(fee: BigNumber | number): BigNumber { export const TxGasLimit = { encode, decode, - L2GasPrice, } export function calculateL1GasLimit(data: string | Buffer): BigNumber { From f2c7bf24d02f9248ac66ff53e2b3709a20af52d0 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:57:27 -0700 Subject: [PATCH 23/28] l2geth: prevent users from sending txs with too high of a fee --- l2geth/rollup/sync_service.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/l2geth/rollup/sync_service.go b/l2geth/rollup/sync_service.go index 28a595a4f38c..98460aa7387a 100644 --- a/l2geth/rollup/sync_service.go +++ b/l2geth/rollup/sync_service.go @@ -763,6 +763,12 @@ func (s *SyncService) verifyFee(tx *types.Transaction) error { if paying.Cmp(expecting) == -1 { return fmt.Errorf("fee too low: %d, use at least tx.gasLimit = %d and tx.gasPrice = %d", paying, fee.Uint64(), fees.BigTxGasPrice) } + // Protect users from overpaying by too much + overpaying := new(big.Int).Sub(paying, expecting) + threshold := new(big.Int).Mul(expecting, common.Big3) + if overpaying.Cmp(threshold) == 1 { + return fmt.Errorf("fee too large: %d", paying) + } return nil } @@ -779,7 +785,6 @@ func (s *SyncService) ValidateAndApplySequencerTransaction(tx *types.Transaction if err := s.verifyFee(tx); err != nil { return err } - s.txLock.Lock() defer s.txLock.Unlock() log.Trace("Sequencer transaction validation", "hash", tx.Hash().Hex()) From 96d38858baaa43e6322bbe9fd7caab93516aace0 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 17:58:39 -0700 Subject: [PATCH 24/28] integration-tests: fix import --- integration-tests/test/erc20.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/test/erc20.spec.ts b/integration-tests/test/erc20.spec.ts index 3d356ab0e4ba..1c03476592a2 100644 --- a/integration-tests/test/erc20.spec.ts +++ b/integration-tests/test/erc20.spec.ts @@ -1,6 +1,6 @@ import { Contract, ContractFactory, Wallet } from 'ethers' import { ethers } from 'hardhat' -import { TxGasLimit } from '@eth-optimism/core-utils' +import { TxGasLimit, TxGasPrice } from '@eth-optimism/core-utils' import chai, { expect } from 'chai' import { GWEI } from './shared/utils' import { OptimismEnv } from './shared/env' @@ -67,7 +67,7 @@ describe('Basic ERC20 interactions', async () => { // The expected fee paid is the value returned by eth_estimateGas const gasLimit = await ERC20.estimateGas.transfer(other.address, 100) const gasPrice = await wallet.getGasPrice() - expect(gasPrice).to.deep.equal(TxGasLimit.L2GasPrice) + expect(gasPrice).to.deep.equal(TxGasPrice) const expectedFeePaid = gasLimit.mul(gasPrice) // There are two events from the transfer with the first being From 48dd383c14ca3ead3cfec93c629125780e82313c Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 18:15:20 -0700 Subject: [PATCH 25/28] integration-tests: fix type --- integration-tests/test/rpc.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index cf2c0b2c5fcc..af480578e647 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -131,7 +131,7 @@ describe('Basic RPC tests', () => { const tx = { ...DEFAULT_TRANSACTION, gasLimit: 1, - gasPrice: TxGasPrice.toNumber(), + gasPrice: TxGasPrice, } const fee = tx.gasPrice.mul(tx.gasLimit) const gasLimit = 59300000001 From 102c64a668a2918574c1aa987ee64b1330558960 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 18:41:11 -0700 Subject: [PATCH 26/28] integration-tests: fix gas limits --- integration-tests/test/rpc.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index af480578e647..998ad34e332f 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -209,11 +209,11 @@ describe('Basic RPC tests', () => { }) }) - describe('eth_getTransactionReceipt', () => { + describe.only('eth_getTransactionReceipt', () => { it('correctly exposes revert data for contract calls', async () => { const req: TransactionRequest = { ...revertingTx, - gasLimit: 934111908999999, // override gas estimation + gasLimit: 59808999999, // override gas estimation } const tx = await wallet.sendTransaction(req) @@ -236,7 +236,7 @@ describe('Basic RPC tests', () => { it('correctly exposes revert data for contract creations', async () => { const req: TransactionRequest = { ...revertingDeployTx, - gasLimit: 1051391908999999, // override gas estimation + gasLimit: 177008999999 // override gas estimation } const tx = await wallet.sendTransaction(req) From e1fe7e4efcc14c3a7abe780de858bbfa50be2df8 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 19:07:16 -0700 Subject: [PATCH 27/28] lint: fix --- integration-tests/test/rpc.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 998ad34e332f..4c88cc4952a7 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -209,7 +209,7 @@ describe('Basic RPC tests', () => { }) }) - describe.only('eth_getTransactionReceipt', () => { + describe('eth_getTransactionReceipt', () => { it('correctly exposes revert data for contract calls', async () => { const req: TransactionRequest = { ...revertingTx, @@ -236,7 +236,7 @@ describe('Basic RPC tests', () => { it('correctly exposes revert data for contract creations', async () => { const req: TransactionRequest = { ...revertingDeployTx, - gasLimit: 177008999999 // override gas estimation + gasLimit: 177008999999, // override gas estimation } const tx = await wallet.sendTransaction(req) From d03775b9f68aeb562deb42c2775e260a9ce99f18 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Fri, 28 May 2021 19:09:25 -0700 Subject: [PATCH 28/28] l2geth: log error --- l2geth/rollup/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l2geth/rollup/client.go b/l2geth/rollup/client.go index b8b08222c73b..2e6402bfd4e6 100644 --- a/l2geth/rollup/client.go +++ b/l2geth/rollup/client.go @@ -570,7 +570,7 @@ func (c *Client) GetTransactionBatch(index uint64) (*Batch, []*types.Transaction Get("/batch/transaction/index/{index}") if err != nil { - return nil, nil, fmt.Errorf("Cannot get transaction batch %d", index) + return nil, nil, fmt.Errorf("Cannot get transaction batch %d: %w", index, err) } txBatch, ok := response.Result().(*TransactionBatchResponse) if !ok {