From 4290b085705c654d3c8a05fe6156258a19aa98cf Mon Sep 17 00:00:00 2001 From: Ben Jones Date: Thu, 11 Mar 2021 12:32:03 +0200 Subject: [PATCH 01/23] fix: add nil check when estimating gas to avoid panic during contract creation --- core/state_transition_ovm.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/state_transition_ovm.go b/core/state_transition_ovm.go index ee4882c88..7d21a2c8c 100644 --- a/core/state_transition_ovm.go +++ b/core/state_transition_ovm.go @@ -96,12 +96,17 @@ func AsOvmMessage(tx *types.Transaction, signer types.Signer, decompressor commo } func EncodeSimulatedMessage(msg Message, timestamp, blockNumber *big.Int, executionManager, stateManager dump.OvmDumpAccount) (Message, error) { + to := msg.To() + if to == nil { + to = &common.Address{0} + } + tx := ovmTransaction{ timestamp, blockNumber, // TODO (what's the correct block number?) uint8(msg.QueueOrigin().Uint64()), *msg.L1MessageSender(), - *msg.To(), + *to, big.NewInt(int64(msg.Gas())), msg.Data(), } From ad78282acf6c21f99ba34c65d5c41f1b5a12e63c Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 11 Mar 2021 12:36:19 +0200 Subject: [PATCH 02/23] fix: revert DoEstimateGas changes introduced as a temp fix in #22 --- internal/ethapi/api.go | 65 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index c26cf52aa..88f8b2968 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -997,13 +997,66 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr } func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { - // Retrieve the block to act as the gas ceiling - block, err := b.BlockByNumberOrHash(ctx, blockNrOrHash) - if err != nil { - return 0, err + // Binary search the gas requirement, as it may be higher than the amount used + var ( + lo uint64 = params.TxGas - 1 + hi uint64 + cap uint64 + ) + if args.Gas != nil && uint64(*args.Gas) >= params.TxGas { + hi = uint64(*args.Gas) + } else { + // Retrieve the block to act as the gas ceiling + block, err := b.BlockByNumberOrHash(ctx, blockNrOrHash) + if err != nil { + return 0, err + } + hi = block.GasLimit() + } + if gasCap != nil && hi > gasCap.Uint64() { + log.Warn("Caller gas above allowance, capping", "requested", hi, "cap", gasCap) + hi = gasCap.Uint64() + } + cap = hi + + // Set sender address or use a default if none specified + if args.From == nil { + if wallets := b.AccountManager().Wallets(); len(wallets) > 0 { + if accounts := wallets[0].Accounts(); len(accounts) > 0 { + args.From = &accounts[0].Address + } + } + } + // Use zero-address if none other is available + if args.From == nil { + args.From = &common.Address{} + } + // Create a helper to check if a gas allowance results in an executable transaction + executable := func(gas uint64) bool { + args.Gas = (*hexutil.Uint64)(&gas) + + _, _, failed, err := DoCall(ctx, b, args, blockNrOrHash, nil, vm.Config{}, 0, gasCap) + if err != nil || failed { + return false + } + return true + } + // Execute the binary search and hone in on an executable gas limit + for lo+1 < hi { + mid := (hi + lo) / 2 + if !executable(mid) { + lo = mid + } else { + hi = mid + } + } + // Reject the transaction as invalid if it still fails at the highest allowance + if hi == cap { + if !executable(hi) { + return 0, fmt.Errorf("gas required exceeds allowance (%d) or always failing transaction", cap) + } } - // For now always return the gas limit - return hexutil.Uint64(block.GasLimit() - 1), nil + return hexutil.Uint64(hi), nil } // EstimateGas returns an estimate of the amount of gas needed to execute the From a1dabb69910e71c58012cef07a989958f61be7db Mon Sep 17 00:00:00 2001 From: smartcontracts Date: Mon, 29 Mar 2021 16:50:12 -0700 Subject: [PATCH 03/23] Tweak fudge factor for Geth gas estimation (#292) * experimenting with gas fudge factor * fix formatting * try using 1m gas constant * Add log statement for the gas estimate * Add more detail to gas estimate log * one more log edit * Try new formula * Bump base gas * Even more base gas * even more * Just 1m? * one more time * Final cleanup * Minor fix * Update internal/ethapi/api.go * don't use cap-1 * Make sure data is not nil Co-authored-by: Karl Floersch --- internal/ethapi/api.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 88f8b2968..0e4f04b29 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1050,6 +1050,25 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash hi = mid } } + + // Fudging to account for gas required to verify signatures + pass around data. + // Specifically, this line accounts for the fact that there's a bit of computation performed in + // a "real" transaction that won't be covered by an eth_call: + // 1. Going into the OVM_SequencerEntrypoint. + // 2. Going into the OVM_ProxyEOA + OVM_ECDSAContractAccount. + // 3. Verify signatures in various places. + // eth_call skips all of this and therefore won't correctly estimate gas by default. We need to + // tweak the gas estimate to account for this discrepancy. Cost is quite high here because of + // the EVM limitation that CALL can only pass 63/64 of total gas available -- so most of this + // gas isn't actually spent during execution but needs to be provided to avoid a revert. + hi += 1000000 + if args.Data != nil { + hi += uint64(len([]byte(*args.Data))) * 128 + } + if hi > cap { + hi = cap + } + // Reject the transaction as invalid if it still fails at the highest allowance if hi == cap { if !executable(hi) { From 486d555fc3227609ffe3b020c8d0a7b6bab88379 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Wed, 10 Mar 2021 14:12:04 +0200 Subject: [PATCH 04/23] feat(api): make eth_gasPrice always return 1 gwei --- internal/ethapi/api.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 0e4f04b29..7b29895cc 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -63,10 +63,9 @@ func NewPublicEthereumAPI(b Backend) *PublicEthereumAPI { return &PublicEthereumAPI{b} } -// GasPrice returns a suggestion for a gas price. +// GasPrice returns 1 gwei always. Rationale: https://github.com/ethereum-optimism/roadmap/issues/715 func (s *PublicEthereumAPI) GasPrice(ctx context.Context) (*hexutil.Big, error) { - price, err := s.b.SuggestPrice(ctx) - return (*hexutil.Big)(price), err + return (*hexutil.Big)(big.NewInt(defaultGasPrice)), nil } // ProtocolVersion returns the current Ethereum protocol version this node supports From e99c376a181c4d8421778d3d6c4810de4e5589ff Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Wed, 10 Mar 2021 14:51:33 +0200 Subject: [PATCH 05/23] feat: overload estimateGas to return the data+execution fee --- internal/ethapi/api.go | 41 ++++++++++++++++++++++++++++++++++++++ internal/ethapi/backend.go | 1 + 2 files changed, 42 insertions(+) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 7b29895cc..229fa10c6 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -995,7 +995,48 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr return (hexutil.Bytes)(result), err } +// Optimism note: The gasPrice in Optimism is modified to always return 1 gwei. We +// use the gasLimit field to communicate the entire user fee. This is done for +// for compatibility reasons with the existing Ethereum toolchain, so that the user +// fees can compensate for the additional costs the sequencer pays for publishing the +// transaction calldata func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { + // 1a. fetch the data price, depends on how the sequencer has chosen to update their values based on the + // l1 gas prices + dataPrice, err := b.SuggestDataPrice(ctx) + if err != nil { + return 0, err + } + + // 1b. Get the data length + // TODO: Should this instead be calculated by serializing the transaction? + dataLen := int64(96 + len(*args.Data)) + + // 1c. Multiply them together + dataFee := new(big.Int).Mul(dataPrice, big.NewInt(dataLen)) + + // 2a. fetch the execution gas price, by the typical mempool dynamics + executionPrice, err := b.SuggestPrice(ctx) + if err != nil { + return 0, err + } + + // 2b. get the gas that would be used by the tx + gasUsed, err := legacyDoEstimateGas(ctx, b, args, blockNrOrHash, gasCap) + if err != nil { + return 0, err + } + + // 2c. Multiply them together to get the execution fee + executionFee := new(big.Int).Mul(executionPrice, big.NewInt(int64(gasUsed))) + + // 3. add them up + fee := new(big.Int).Add(dataFee, executionFee) + + return (hexutil.Uint64)(fee.Uint64()), nil +} + +func legacyDoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { // Binary search the gas requirement, as it may be higher than the amount used var ( lo uint64 = params.TxGas - 1 diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index 0ff8643a2..49ea24836 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -94,6 +94,7 @@ type Backend interface { GetRollupContext() (uint64, uint64) GasLimit() uint64 GetDiff(*big.Int) (diffdb.Diff, error) + SuggestDataPrice(ctx context.Context) (*big.Int, error) } func GetAPIs(apiBackend Backend) []rpc.API { From 0abd7300af9926d4c8aa61bfdc01b033ff8b4826 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Wed, 10 Mar 2021 15:00:38 +0200 Subject: [PATCH 06/23] feat: implement static L1 Gas Price oracle --- eth/api_backend.go | 5 +++++ eth/api_backend_test.go | 1 + eth/backend.go | 5 ++++- eth/gasprice/l1_gasprice.go | 20 ++++++++++++++++++++ les/api_backend.go | 5 +++++ 5 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 eth/gasprice/l1_gasprice.go diff --git a/eth/api_backend.go b/eth/api_backend.go index c59269836..b4aec6025 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -46,6 +46,7 @@ type EthAPIBackend struct { extRPCEnabled bool eth *Ethereum gpo *gasprice.Oracle + l1gpo *gasprice.L1Oracle verifier bool gasLimit uint64 UsingOVM bool @@ -371,6 +372,10 @@ func (b *EthAPIBackend) SuggestPrice(ctx context.Context) (*big.Int, error) { return b.gpo.SuggestPrice(ctx) } +func (b *EthAPIBackend) SuggestDataPrice(ctx context.Context) (*big.Int, error) { + return b.l1gpo.SuggestDataPrice(ctx) +} + func (b *EthAPIBackend) ChainDb() ethdb.Database { return b.eth.ChainDb() } diff --git a/eth/api_backend_test.go b/eth/api_backend_test.go index 7f84758c6..3f4d91683 100644 --- a/eth/api_backend_test.go +++ b/eth/api_backend_test.go @@ -15,6 +15,7 @@ func TestGasLimit(t *testing.T) { extRPCEnabled: false, eth: nil, gpo: nil, + l1gpo: nil, verifier: false, gasLimit: 0, UsingOVM: true, diff --git a/eth/backend.go b/eth/backend.go index ba3f43eca..a8625f3c3 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -227,12 +227,15 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData)) log.Info("Backend Config", "max-calldata-size", config.Rollup.MaxCallDataSize, "gas-limit", config.Rollup.GasLimit, "is-verifier", config.Rollup.IsVerifier, "using-ovm", vm.UsingOVM) - eth.APIBackend = &EthAPIBackend{ctx.ExtRPCEnabled(), eth, nil, config.Rollup.IsVerifier, config.Rollup.GasLimit, vm.UsingOVM, config.Rollup.MaxCallDataSize} + eth.APIBackend = &EthAPIBackend{ctx.ExtRPCEnabled(), eth, nil, nil, config.Rollup.IsVerifier, config.Rollup.GasLimit, vm.UsingOVM, config.Rollup.MaxCallDataSize} gpoParams := config.GPO if gpoParams.Default == nil { gpoParams.Default = config.Miner.GasPrice } eth.APIBackend.gpo = gasprice.NewOracle(eth.APIBackend, gpoParams) + + // TODO: Provide configuration options for the L1 Gas Price oracle + eth.APIBackend.l1gpo = gasprice.NewL1Oracle() return eth, nil } diff --git a/eth/gasprice/l1_gasprice.go b/eth/gasprice/l1_gasprice.go new file mode 100644 index 000000000..11ffb1f0b --- /dev/null +++ b/eth/gasprice/l1_gasprice.go @@ -0,0 +1,20 @@ +package gasprice + +import ( + "context" + "github.com/ethereum/go-ethereum/params" + "math/big" +) + +type L1Oracle struct { +} + +func NewL1Oracle() *L1Oracle { + return &L1Oracle{} +} + +/// SuggestDataPrice returns the gas price which should be charged per byte of published +/// data by the sequencer. +func (gpo *L1Oracle) SuggestDataPrice(ctx context.Context) (*big.Int, error) { + return big.NewInt(100 * params.GWei), nil +} diff --git a/les/api_backend.go b/les/api_backend.go index d0007cf92..a3df8f6d7 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -281,6 +281,11 @@ func (b *LesApiBackend) SuggestPrice(ctx context.Context) (*big.Int, error) { return b.gpo.SuggestPrice(ctx) } +// NB: Non sequencer nodes cannot suggest L1 gas prices. +func (b *LesApiBackend) SuggestDataPrice(ctx context.Context) (*big.Int, error) { + panic("not implemented") +} + func (b *LesApiBackend) ChainDb() ethdb.Database { return b.eth.chainDb } From eb3e487dd59da9b87b01129fe9aabedf755b9484 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Wed, 10 Mar 2021 15:25:50 +0200 Subject: [PATCH 07/23] feat: allow configuring the L1GasPrice via CLI at node startup --- cmd/geth/main.go | 1 + cmd/geth/usage.go | 1 + cmd/utils/flags.go | 9 +++++++++ eth/backend.go | 3 +-- eth/config.go | 1 + eth/gasprice/l1_gasprice.go | 8 ++++---- rollup/config.go | 2 ++ 7 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 56f557661..ff0ad41f3 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -164,6 +164,7 @@ var ( utils.RollupStateDumpPathFlag, utils.RollupDiffDbFlag, utils.RollupMaxCalldataSizeFlag, + utils.RollupL1GasPriceFlag, } rpcFlags = []cli.Flag{ diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index 0d81d7a13..c59d214db 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -78,6 +78,7 @@ var AppHelpFlagGroups = []flagGroup{ utils.RollupStateDumpPathFlag, utils.RollupDiffDbFlag, utils.RollupMaxCalldataSizeFlag, + utils.RollupL1GasPriceFlag, }, }, { diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 0db507395..432599ae4 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -879,6 +879,12 @@ var ( Value: eth.DefaultConfig.Rollup.MaxCallDataSize, EnvVar: "ROLLUP_MAX_CALLDATA_SIZE", } + RollupL1GasPriceFlag = BigFlag{ + Name: "rollup.l1gasprice", + Usage: "The L1 gas price to use for the sequencer fees", + Value: eth.DefaultConfig.Rollup.L1GasPrice, + EnvVar: "ROLLUP_L1_GASPRICE", + } ) // MakeDataDir retrieves the currently requested data directory, terminating @@ -1152,6 +1158,9 @@ func setRollup(ctx *cli.Context, cfg *rollup.Config) { if ctx.GlobalIsSet(RollupTimstampRefreshFlag.Name) { cfg.TimestampRefreshThreshold = ctx.GlobalDuration(RollupTimstampRefreshFlag.Name) } + if ctx.GlobalIsSet(RollupL1GasPriceFlag.Name) { + cfg.L1GasPrice = GlobalBig(ctx, RollupL1GasPriceFlag.Name) + } } // setLes configures the les server and ultra light client settings from the command line flags. diff --git a/eth/backend.go b/eth/backend.go index a8625f3c3..b734bf96f 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -234,8 +234,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { } eth.APIBackend.gpo = gasprice.NewOracle(eth.APIBackend, gpoParams) - // TODO: Provide configuration options for the L1 Gas Price oracle - eth.APIBackend.l1gpo = gasprice.NewL1Oracle() + eth.APIBackend.l1gpo = gasprice.NewL1Oracle(config.Rollup.L1GasPrice) return eth, nil } diff --git a/eth/config.go b/eth/config.go index 63b0e1ad0..56ae60837 100644 --- a/eth/config.go +++ b/eth/config.go @@ -81,6 +81,7 @@ var DefaultConfig = Config{ // is additional overhead that is unaccounted. Round down to 127000 for // safety. MaxCallDataSize: 127000, + L1GasPrice: big.NewInt(100 * params.GWei), }, DiffDbCache: 256, } diff --git a/eth/gasprice/l1_gasprice.go b/eth/gasprice/l1_gasprice.go index 11ffb1f0b..7caf10c6d 100644 --- a/eth/gasprice/l1_gasprice.go +++ b/eth/gasprice/l1_gasprice.go @@ -2,19 +2,19 @@ package gasprice import ( "context" - "github.com/ethereum/go-ethereum/params" "math/big" ) type L1Oracle struct { + gasPrice *big.Int } -func NewL1Oracle() *L1Oracle { - return &L1Oracle{} +func NewL1Oracle(gasPrice *big.Int) *L1Oracle { + return &L1Oracle{gasPrice} } /// SuggestDataPrice returns the gas price which should be charged per byte of published /// data by the sequencer. func (gpo *L1Oracle) SuggestDataPrice(ctx context.Context) (*big.Int, error) { - return big.NewInt(100 * params.GWei), nil + return gpo.gasPrice, nil } diff --git a/rollup/config.go b/rollup/config.go index fe5855e97..c79ee411c 100644 --- a/rollup/config.go +++ b/rollup/config.go @@ -33,4 +33,6 @@ type Config struct { PollInterval time.Duration // Interval for updating the timestamp TimestampRefreshThreshold time.Duration + // The gas price to use when estimating L1 calldata publishing costs + L1GasPrice *big.Int } From 40fe63a67a2b6ca92517c8c9c45662b7d61385d3 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Wed, 10 Mar 2021 15:27:00 +0200 Subject: [PATCH 08/23] feat: allow configuring the L1GasPrice remotely over a new private RPC namespace --- eth/api_backend.go | 4 ++++ eth/gasprice/l1_gasprice.go | 4 ++++ internal/ethapi/api.go | 18 ++++++++++++++++++ internal/ethapi/backend.go | 5 +++++ les/api_backend.go | 7 ++++++- 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/eth/api_backend.go b/eth/api_backend.go index b4aec6025..bd242e727 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -376,6 +376,10 @@ func (b *EthAPIBackend) SuggestDataPrice(ctx context.Context) (*big.Int, error) return b.l1gpo.SuggestDataPrice(ctx) } +func (b *EthAPIBackend) SetL1GasPrice(ctx context.Context, gasPrice *big.Int) { + b.l1gpo.SetL1GasPrice(gasPrice) +} + func (b *EthAPIBackend) ChainDb() ethdb.Database { return b.eth.ChainDb() } diff --git a/eth/gasprice/l1_gasprice.go b/eth/gasprice/l1_gasprice.go index 7caf10c6d..b6870c3e6 100644 --- a/eth/gasprice/l1_gasprice.go +++ b/eth/gasprice/l1_gasprice.go @@ -18,3 +18,7 @@ func NewL1Oracle(gasPrice *big.Int) *L1Oracle { func (gpo *L1Oracle) SuggestDataPrice(ctx context.Context) (*big.Int, error) { return gpo.gasPrice, nil } + +func (gpo *L1Oracle) SetL1GasPrice(gasPrice *big.Int) { + gpo.gasPrice = gasPrice +} diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 229fa10c6..a994f1613 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1974,6 +1974,24 @@ func (api *PublicRollupAPI) GetInfo(ctx context.Context) rollupInfo { } } +// PrivatelRollupAPI provides private RPC methods to control the sequencer. +// These methods can be abused by external users and must be considered insecure for use by untrusted users. +type PrivateRollupAPI struct { + b Backend +} + +// NewPrivateRollupAPI creates a new API definition for the rollup methods of the +// Ethereum service. +func NewPrivateRollupAPI(b Backend) *PrivateRollupAPI { + return &PrivateRollupAPI{b: b} +} + +// SetGasPrice sets the gas price to be used when quoting calldata publishing costs +// to users +func (api *PrivateRollupAPI) SetL1GasPrice(ctx context.Context, gasPrice hexutil.Big) { + api.b.SetL1GasPrice(ctx, (*big.Int)(&gasPrice)) +} + // PublicDebugAPI is the collection of Ethereum APIs exposed over the public // debugging endpoint. type PublicDebugAPI struct { diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index 49ea24836..734a83702 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -95,6 +95,7 @@ type Backend interface { GasLimit() uint64 GetDiff(*big.Int) (diffdb.Diff, error) SuggestDataPrice(ctx context.Context) (*big.Int, error) + SetL1GasPrice(context.Context, *big.Int) } func GetAPIs(apiBackend Backend) []rpc.API { @@ -120,6 +121,10 @@ func GetAPIs(apiBackend Backend) []rpc.API { Version: "1.0", Service: NewPublicRollupAPI(apiBackend), Public: true, + }, { + Namespace: "rollup_personal", + Version: "1.0", + Service: NewPrivateRollupAPI(apiBackend), }, { Namespace: "txpool", Version: "1.0", diff --git a/les/api_backend.go b/les/api_backend.go index a3df8f6d7..75e30bbbd 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -283,7 +283,12 @@ func (b *LesApiBackend) SuggestPrice(ctx context.Context) (*big.Int, error) { // NB: Non sequencer nodes cannot suggest L1 gas prices. func (b *LesApiBackend) SuggestDataPrice(ctx context.Context) (*big.Int, error) { - panic("not implemented") + panic("SuggestDataPrice not implemented") +} + +// NB: Non sequencer nodes cannot set L1 gas prices. +func (b *LesApiBackend) SetL1GasPrice(ctx context.Context, gasPrice *big.Int) { + panic("SetL1GasPrice is not implemented") } func (b *LesApiBackend) ChainDb() ethdb.Database { From 4c7823351baf0051bd0d1288d9bd3503aae9e088 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:00:23 +0200 Subject: [PATCH 09/23] Sequencer Fee Pricing Part 3, feat: Pull L1Gasprice from the Data Service (#277) * feat: pull L1GasPrice from the data transport service * refactor: split out single-iteration of sequencer loop * test(sync-service): ensure L1GasPrice is set correctly * chore: gofmt * fix: parse json response as string and test --- eth/backend.go | 5 ++++- go.mod | 1 + go.sum | 2 ++ rollup/client.go | 27 ++++++++++++++++++++++++ rollup/client_test.go | 42 +++++++++++++++++++++++++++++++++++++ rollup/sync_service.go | 11 ++++++++++ rollup/sync_service_test.go | 37 ++++++++++++++++++++++++++++++++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 rollup/client_test.go diff --git a/eth/backend.go b/eth/backend.go index b734bf96f..6cff26e72 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -234,7 +234,10 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { } eth.APIBackend.gpo = gasprice.NewOracle(eth.APIBackend, gpoParams) - eth.APIBackend.l1gpo = gasprice.NewL1Oracle(config.Rollup.L1GasPrice) + // create the L1 GPO and allow the API backend and the sync service to access it + l1Gpo := gasprice.NewL1Oracle(config.Rollup.L1GasPrice) + eth.APIBackend.l1gpo = l1Gpo + eth.syncService.l1gpo = l1Gpo return eth, nil } diff --git a/go.mod b/go.mod index 6026ad14f..47277f950 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,7 @@ require ( github.com/huin/goupnp v1.0.0 github.com/influxdata/influxdb v1.2.3-0.20180221223340-01288bdb0883 github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458 + github.com/jarcoal/httpmock v1.0.8 github.com/jmoiron/sqlx v1.2.0 github.com/julienschmidt/httprouter v1.1.1-0.20170430222011-975b5c4c7c21 github.com/karalabe/usb v0.0.0-20190919080040-51dc0efba356 diff --git a/go.sum b/go.sum index d0ac6c0f9..5c53cca7b 100644 --- a/go.sum +++ b/go.sum @@ -108,6 +108,8 @@ github.com/influxdata/influxdb v1.2.3-0.20180221223340-01288bdb0883 h1:FSeK4fZCo github.com/influxdata/influxdb v1.2.3-0.20180221223340-01288bdb0883/go.mod h1:qZna6X/4elxqT3yI9iZYdZrWWdeFOOprn86kgg4+IzY= github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458 h1:6OvNmYgJyexcZ3pYbTI9jWx5tHo1Dee/tWbLMfPe2TA= github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458/go.mod h1:QPH045xvCAeXUZOxsnwmrtiCoxIr9eob+4orBN1SBKc= +github.com/jarcoal/httpmock v1.0.8 h1:8kI16SoO6LQKgPE7PvQuV+YuD/inwHd7fOOe2zMbo4k= +github.com/jarcoal/httpmock v1.0.8/go.mod h1:ATjnClrvW/3tijVmpL/va5Z3aAyGvqU3gCT8nX0Txik= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmoiron/sqlx v1.2.0 h1:41Ip0zITnmWNR/vHV+S4m+VoUivnWY5E4OJfLZjCJMA= diff --git a/rollup/client.go b/rollup/client.go index bfa7952ce..d890d8e67 100644 --- a/rollup/client.go +++ b/rollup/client.go @@ -42,6 +42,10 @@ type SyncStatus struct { CurrentTransactionIndex uint64 `json:"currentTransactionIndex"` } +type L1GasPrice struct { + GasPrice string `json:"gasPrice"` +} + type transaction struct { Index uint64 `json:"index"` BatchIndex uint64 `json:"batchIndex"` @@ -92,6 +96,7 @@ type RollupClient interface { GetLatestEthContext() (*EthContext, error) GetLastConfirmedEnqueue() (*types.Transaction, error) SyncStatus() (*SyncStatus, error) + GetL1GasPrice() (*big.Int, error) } type Client struct { @@ -439,3 +444,25 @@ func (c *Client) SyncStatus() (*SyncStatus, error) { return status, nil } + +func (c *Client) GetL1GasPrice() (*big.Int, error) { + response, err := c.client.R(). + SetResult(&L1GasPrice{}). + Get("/eth/gasprice") + + if err != nil { + return nil, fmt.Errorf("Cannot fetch L1 gas price: %w", err) + } + + gasPriceResp, ok := response.Result().(*L1GasPrice) + if !ok { + return nil, fmt.Errorf("Cannot parse L1 gas price response") + } + + gasPrice, ok := new(big.Int).SetString(gasPriceResp.GasPrice, 10) + if !ok { + return nil, fmt.Errorf("Cannot parse response as big number") + } + + return gasPrice, nil +} diff --git a/rollup/client_test.go b/rollup/client_test.go new file mode 100644 index 000000000..46ab44497 --- /dev/null +++ b/rollup/client_test.go @@ -0,0 +1,42 @@ +package rollup + +import ( + "fmt" + "github.com/jarcoal/httpmock" + "math/big" + "testing" +) + +func TestRollupClientGetL1GasPrice(t *testing.T) { + url := "http://localhost:9999" + endpoint := fmt.Sprintf("%s/eth/gasprice", url) + // url/chain-id does not matter, we'll mock the responses + client := NewClient(url, big.NewInt(1)) + // activate the mock + httpmock.ActivateNonDefault(client.client.GetClient()) + + // The API responds with a string value + expectedGasPrice, _ := new(big.Int).SetString("123132132151245817421893", 10) + body := map[string]interface{}{ + "gasPrice": expectedGasPrice.String(), + } + response, _ := httpmock.NewJsonResponder( + 200, + body, + ) + httpmock.RegisterResponder( + "GET", + endpoint, + response, + ) + + gasPrice, err := client.GetL1GasPrice() + + if err != nil { + t.Fatal("could not get mocked gas price", err) + } + + if gasPrice.Cmp(expectedGasPrice) != 0 { + t.Fatal("gasPrice is not parsed properly in the client") + } +} diff --git a/rollup/sync_service.go b/rollup/sync_service.go index 19c74bc34..a21e57959 100644 --- a/rollup/sync_service.go +++ b/rollup/sync_service.go @@ -19,6 +19,8 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" + + "github.com/ethereum/go-ethereum/eth/gasprice" ) // OVMContext represents the blocknumber and timestamp @@ -42,6 +44,7 @@ type SyncService struct { eth1ChainId uint64 bc *core.BlockChain txpool *core.TxPool + l1gpo *gasprice.L1Oracle client RollupClient syncing atomic.Value OVMContext OVMContext @@ -357,6 +360,13 @@ func (s *SyncService) SequencerLoop() { } func (s *SyncService) sequence() error { + // Update to the latest L1 gas price + l1GasPrice, err := s.client.GetL1GasPrice() + if err != nil { + return err + } + s.l1gpo.SetL1GasPrice(l1GasPrice) + // Only the sequencer needs to poll for enqueue transactions // and then can choose when to apply them. We choose to apply // transactions such that it makes for efficient batch submitting. @@ -441,6 +451,7 @@ func (s *SyncService) updateContext() error { if err != nil { return err } + current := time.Unix(int64(s.GetLatestL1Timestamp()), 0) next := time.Unix(int64(context.Timestamp), 0) if next.Sub(current) > s.timestampRefreshThreshold { diff --git a/rollup/sync_service_test.go b/rollup/sync_service_test.go index 017dfb6bf..ddc9c1c2e 100644 --- a/rollup/sync_service_test.go +++ b/rollup/sync_service_test.go @@ -14,6 +14,7 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/eth/gasprice" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/params" ) @@ -150,6 +151,37 @@ func TestSyncServiceTransactionEnqueued(t *testing.T) { } } +func TestSyncServiceL1GasPrice(t *testing.T) { + service, _, _, err := newTestSyncService(true) + setupMockClient(service, map[string]interface{}{}) + service.l1gpo = gasprice.NewL1Oracle(big.NewInt(0)) + + if err != nil { + t.Fatal(err) + } + + gasBefore, err := service.l1gpo.SuggestDataPrice(context.Background()) + if err != nil { + t.Fatal(err) + } + + if gasBefore.Cmp(big.NewInt(0)) != 0 { + t.Fatal("expected 0 gas price, got", gasBefore) + } + + // run 1 iteration of the eloop + service.sequence() + + gasAfter, err := service.l1gpo.SuggestDataPrice(context.Background()) + if err != nil { + t.Fatal(err) + } + + if gasAfter.Cmp(big.NewInt(100*int64(params.GWei))) != 0 { + t.Fatal("expected 100 gas price, got", gasAfter) + } +} + // Pass true to set as a verifier func TestSyncServiceSync(t *testing.T) { service, txCh, sub, err := newTestSyncService(true) @@ -314,6 +346,7 @@ type mockClient struct { func setupMockClient(service *SyncService, responses map[string]interface{}) { client := newMockClient(responses) service.client = client + service.l1gpo = gasprice.NewL1Oracle(big.NewInt(0)) } func newMockClient(responses map[string]interface{}) *mockClient { @@ -400,3 +433,7 @@ func (m *mockClient) SyncStatus() (*SyncStatus, error) { Syncing: false, }, nil } + +func (m *mockClient) GetL1GasPrice() (*big.Int, error) { + return big.NewInt(100 * int64(params.GWei)), nil +} From a7eead8d384edaf2cb2d1a505d95190ab8bb3a29 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:32:37 +0200 Subject: [PATCH 10/23] chore: expose L1Gpo in the sync service --- eth/backend.go | 4 ++-- rollup/sync_service.go | 4 ++-- rollup/sync_service_test.go | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/eth/backend.go b/eth/backend.go index 6cff26e72..0c020fae4 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -226,7 +226,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { eth.miner = miner.New(eth, &config.Miner, chainConfig, eth.EventMux(), eth.engine, eth.isLocalBlock) eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData)) - log.Info("Backend Config", "max-calldata-size", config.Rollup.MaxCallDataSize, "gas-limit", config.Rollup.GasLimit, "is-verifier", config.Rollup.IsVerifier, "using-ovm", vm.UsingOVM) + log.Info("Backend Config", "max-calldata-size", config.Rollup.MaxCallDataSize, "gas-limit", config.Rollup.GasLimit, "is-verifier", config.Rollup.IsVerifier, "using-ovm", vm.UsingOVM, "l1-gasprice", config.Rollup.L1GasPrice) eth.APIBackend = &EthAPIBackend{ctx.ExtRPCEnabled(), eth, nil, nil, config.Rollup.IsVerifier, config.Rollup.GasLimit, vm.UsingOVM, config.Rollup.MaxCallDataSize} gpoParams := config.GPO if gpoParams.Default == nil { @@ -237,7 +237,7 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { // create the L1 GPO and allow the API backend and the sync service to access it l1Gpo := gasprice.NewL1Oracle(config.Rollup.L1GasPrice) eth.APIBackend.l1gpo = l1Gpo - eth.syncService.l1gpo = l1Gpo + eth.syncService.L1gpo = l1Gpo return eth, nil } diff --git a/rollup/sync_service.go b/rollup/sync_service.go index a21e57959..56a9d0827 100644 --- a/rollup/sync_service.go +++ b/rollup/sync_service.go @@ -44,7 +44,7 @@ type SyncService struct { eth1ChainId uint64 bc *core.BlockChain txpool *core.TxPool - l1gpo *gasprice.L1Oracle + L1gpo *gasprice.L1Oracle client RollupClient syncing atomic.Value OVMContext OVMContext @@ -365,7 +365,7 @@ func (s *SyncService) sequence() error { if err != nil { return err } - s.l1gpo.SetL1GasPrice(l1GasPrice) + s.L1gpo.SetL1GasPrice(l1GasPrice) // Only the sequencer needs to poll for enqueue transactions // and then can choose when to apply them. We choose to apply diff --git a/rollup/sync_service_test.go b/rollup/sync_service_test.go index ddc9c1c2e..2f3a98168 100644 --- a/rollup/sync_service_test.go +++ b/rollup/sync_service_test.go @@ -154,13 +154,13 @@ func TestSyncServiceTransactionEnqueued(t *testing.T) { func TestSyncServiceL1GasPrice(t *testing.T) { service, _, _, err := newTestSyncService(true) setupMockClient(service, map[string]interface{}{}) - service.l1gpo = gasprice.NewL1Oracle(big.NewInt(0)) + service.L1gpo = gasprice.NewL1Oracle(big.NewInt(0)) if err != nil { t.Fatal(err) } - gasBefore, err := service.l1gpo.SuggestDataPrice(context.Background()) + gasBefore, err := service.L1gpo.SuggestDataPrice(context.Background()) if err != nil { t.Fatal(err) } @@ -172,7 +172,7 @@ func TestSyncServiceL1GasPrice(t *testing.T) { // run 1 iteration of the eloop service.sequence() - gasAfter, err := service.l1gpo.SuggestDataPrice(context.Background()) + gasAfter, err := service.L1gpo.SuggestDataPrice(context.Background()) if err != nil { t.Fatal(err) } @@ -346,7 +346,7 @@ type mockClient struct { func setupMockClient(service *SyncService, responses map[string]interface{}) { client := newMockClient(responses) service.client = client - service.l1gpo = gasprice.NewL1Oracle(big.NewInt(0)) + service.L1gpo = gasprice.NewL1Oracle(big.NewInt(0)) } func newMockClient(responses map[string]interface{}) *mockClient { From 0603746f79cbbd8a4dcb8f2b5eae00e82e830523 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:33:29 +0200 Subject: [PATCH 11/23] refactor: create helper function for calculating the rollup fee --- core/rollup_fee.go | 19 +++++++++++++++++++ core/rollup_fee_test.go | 34 ++++++++++++++++++++++++++++++++++ internal/ethapi/api.go | 29 +++++++++-------------------- 3 files changed, 62 insertions(+), 20 deletions(-) create mode 100644 core/rollup_fee.go create mode 100644 core/rollup_fee_test.go diff --git a/core/rollup_fee.go b/core/rollup_fee.go new file mode 100644 index 000000000..007dbb960 --- /dev/null +++ b/core/rollup_fee.go @@ -0,0 +1,19 @@ +package core + +import ( + "math/big" +) + +var ROLLUP_BASE_TX_SIZE int = 96 + +/// CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into +/// account the cost of publishing data to L1. +/// Returns: (ROLLUP_BASE_TX_SIZE + len(data)) * dataPrice + executionPrice * gasUsed +func CalculateRollupFee(data []byte, gasUsed uint64, dataPrice, executionPrice *big.Int) *big.Int { + dataLen := int64(ROLLUP_BASE_TX_SIZE + len(data)) + // get the data fee + dataFee := new(big.Int).Mul(dataPrice, big.NewInt(dataLen)) + executionFee := new(big.Int).Mul(executionPrice, big.NewInt(int64(gasUsed))) + fee := new(big.Int).Add(dataFee, executionFee) + return fee +} diff --git a/core/rollup_fee_test.go b/core/rollup_fee_test.go new file mode 100644 index 000000000..3ae86dd1b --- /dev/null +++ b/core/rollup_fee_test.go @@ -0,0 +1,34 @@ +package core + +import ( + "math/big" + "testing" +) + +var feeTests = map[string]struct { + dataLen int + gasUsed uint64 + dataPrice int64 + executionPrice int64 +}{ + "simple": {10000, 10, 20, 30}, + "zero gas used": {10000, 0, 20, 30}, + "zero data price": {10000, 0, 0, 30}, + "zero execution price": {10000, 0, 0, 0}, +} + +func TestCalculateRollupFee(t *testing.T) { + for name, tt := range feeTests { + t.Run(name, func(t *testing.T) { + data := make([]byte, 0, tt.dataLen) + fee := CalculateRollupFee(data, tt.gasUsed, big.NewInt(tt.dataPrice), big.NewInt(tt.executionPrice)) + + dataFee := uint64((ROLLUP_BASE_TX_SIZE + len(data)) * int(tt.dataPrice)) + executionFee := uint64(tt.executionPrice) * tt.gasUsed + expectedFee := dataFee + executionFee + if fee.Cmp(big.NewInt(int64(expectedFee))) != 0 { + t.Errorf("rollup fee check failed: expected %d, got %s", expectedFee, fee.String()) + } + }) + } +} diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a994f1613..33f005093 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1001,38 +1001,27 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr // fees can compensate for the additional costs the sequencer pays for publishing the // transaction calldata func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { - // 1a. fetch the data price, depends on how the sequencer has chosen to update their values based on the - // l1 gas prices - dataPrice, err := b.SuggestDataPrice(ctx) + // 1. get the gas that would be used by the transaction + gasUsed, err := legacyDoEstimateGas(ctx, b, args, blockNrOrHash, gasCap) if err != nil { return 0, err } - // 1b. Get the data length - // TODO: Should this instead be calculated by serializing the transaction? - dataLen := int64(96 + len(*args.Data)) - - // 1c. Multiply them together - dataFee := new(big.Int).Mul(dataPrice, big.NewInt(dataLen)) - - // 2a. fetch the execution gas price, by the typical mempool dynamics - executionPrice, err := b.SuggestPrice(ctx) + // 2a. fetch the data price, depends on how the sequencer has chosen to update their values based on the + // l1 gas prices + dataPrice, err := b.SuggestDataPrice(ctx) if err != nil { return 0, err } - // 2b. get the gas that would be used by the tx - gasUsed, err := legacyDoEstimateGas(ctx, b, args, blockNrOrHash, gasCap) + // 2b. fetch the execution gas price, by the typical mempool dynamics + executionPrice, err := b.SuggestPrice(ctx) if err != nil { return 0, err } - // 2c. Multiply them together to get the execution fee - executionFee := new(big.Int).Mul(executionPrice, big.NewInt(int64(gasUsed))) - - // 3. add them up - fee := new(big.Int).Add(dataFee, executionFee) - + // 3. calculate the fee + fee := core.CalculateRollupFee(*args.Data, uint64(gasUsed), dataPrice, executionPrice) return (hexutil.Uint64)(fee.Uint64()), nil } From b7a2c6cf513c054106c42087d8e0deb4d5c50bc5 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:36:09 +0200 Subject: [PATCH 12/23] docs: add doc for rollup tx size constant --- core/rollup_fee.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/rollup_fee.go b/core/rollup_fee.go index 007dbb960..d1d1614cb 100644 --- a/core/rollup_fee.go +++ b/core/rollup_fee.go @@ -4,6 +4,9 @@ import ( "math/big" ) +/// ROLLUP_BASE_TX_SIZE is the encoded rollup transaction's compressed size excluding +/// the variable length data. +/// Ref: https://github.com/ethereum-optimism/contracts/blob/409f190518b90301db20d0d4f53760021bc203a8/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol#L47 var ROLLUP_BASE_TX_SIZE int = 96 /// CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into From 147b81c78a9d34233d22ea866ae5d6d8fbbe3719 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:40:36 +0200 Subject: [PATCH 13/23] chore(console_test): add rollup_personal to test --- cmd/geth/consolecmd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/geth/consolecmd_test.go b/cmd/geth/consolecmd_test.go index b16819e4f..a31c88a65 100644 --- a/cmd/geth/consolecmd_test.go +++ b/cmd/geth/consolecmd_test.go @@ -31,7 +31,7 @@ import ( ) const ( - ipcAPIs = "admin:1.0 debug:1.0 eth:1.0 ethash:1.0 miner:1.0 net:1.0 personal:1.0 rollup:1.0 rpc:1.0 shh:1.0 txpool:1.0 web3:1.0" + ipcAPIs = "admin:1.0 debug:1.0 eth:1.0 ethash:1.0 miner:1.0 net:1.0 personal:1.0 rollup:1.0 rollup_personal:1.0 rpc:1.0 shh:1.0 txpool:1.0 web3:1.0" httpAPIs = "eth:1.0 net:1.0 rpc:1.0 web3:1.0" ) From 68ba95ae9270c287bf13680e8eb340fa7a610309 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Thu, 18 Mar 2021 12:43:07 +0200 Subject: [PATCH 14/23] chore: remove empty line --- eth/backend.go | 1 - 1 file changed, 1 deletion(-) diff --git a/eth/backend.go b/eth/backend.go index 0c020fae4..657422cb7 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -233,7 +233,6 @@ func New(ctx *node.ServiceContext, config *Config) (*Ethereum, error) { gpoParams.Default = config.Miner.GasPrice } eth.APIBackend.gpo = gasprice.NewOracle(eth.APIBackend, gpoParams) - // create the L1 GPO and allow the API backend and the sync service to access it l1Gpo := gasprice.NewL1Oracle(config.Rollup.L1GasPrice) eth.APIBackend.l1gpo = l1Gpo From ea47616ee4020f1b7831fbc3d38344b183508ae9 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 23 Mar 2021 12:45:19 +0200 Subject: [PATCH 15/23] chore: adjust review comments * do not cast gasUsed * adjust comment on GasPrice method * add nil check for args.Data * log gas price changes in the sync service --- core/rollup_fee.go | 2 +- internal/ethapi/api.go | 6 +++++- rollup/sync_service.go | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/core/rollup_fee.go b/core/rollup_fee.go index d1d1614cb..bee222ec0 100644 --- a/core/rollup_fee.go +++ b/core/rollup_fee.go @@ -16,7 +16,7 @@ func CalculateRollupFee(data []byte, gasUsed uint64, dataPrice, executionPrice * dataLen := int64(ROLLUP_BASE_TX_SIZE + len(data)) // get the data fee dataFee := new(big.Int).Mul(dataPrice, big.NewInt(dataLen)) - executionFee := new(big.Int).Mul(executionPrice, big.NewInt(int64(gasUsed))) + executionFee := new(big.Int).Mul(executionPrice, new(big.Int).SetUint64(gasUsed)) fee := new(big.Int).Add(dataFee, executionFee) return fee } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 33f005093..b5faca348 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -63,7 +63,7 @@ func NewPublicEthereumAPI(b Backend) *PublicEthereumAPI { return &PublicEthereumAPI{b} } -// GasPrice returns 1 gwei always. Rationale: https://github.com/ethereum-optimism/roadmap/issues/715 +// 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 } @@ -1001,6 +1001,10 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr // fees can compensate for the additional costs the sequencer pays for publishing the // transaction calldata func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { + if args.Data == nil { + return 0, errors.New("transaction data cannot be nil") + } + // 1. get the gas that would be used by the transaction gasUsed, err := legacyDoEstimateGas(ctx, b, args, blockNrOrHash, gasCap) if err != nil { diff --git a/rollup/sync_service.go b/rollup/sync_service.go index 56a9d0827..b38683448 100644 --- a/rollup/sync_service.go +++ b/rollup/sync_service.go @@ -366,6 +366,7 @@ func (s *SyncService) sequence() error { return err } s.L1gpo.SetL1GasPrice(l1GasPrice) + log.Info("Adjusted L1 Gas Price", "gasprice", l1GasPrice) // Only the sequencer needs to poll for enqueue transactions // and then can choose when to apply them. We choose to apply From 32a4e5db4f082dc84ab1075854dd7df0927980d0 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Sat, 27 Mar 2021 19:13:20 +0200 Subject: [PATCH 16/23] chore: re-order imports --- rollup/client_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rollup/client_test.go b/rollup/client_test.go index 46ab44497..a391f6c87 100644 --- a/rollup/client_test.go +++ b/rollup/client_test.go @@ -2,9 +2,10 @@ package rollup import ( "fmt" - "github.com/jarcoal/httpmock" "math/big" "testing" + + "github.com/jarcoal/httpmock" ) func TestRollupClientGetL1GasPrice(t *testing.T) { From 9113ae90a41d22e52425abbb292557ea79a8e8ed Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 30 Mar 2021 09:26:30 +0300 Subject: [PATCH 17/23] fix: skip txpool max gas check for rollup txs Security Question: does this introduce a DoS vector? --- core/tx_pool.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index a5a581c83..c59cf4679 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -92,8 +92,9 @@ var ( ) var ( - evictionInterval = time.Minute // Time interval to check for evictable transactions - statsReportInterval = 8 * time.Second // Time interval to report transaction pool stats + evictionInterval = time.Minute // Time interval to check for evictable transactions + statsReportInterval = 8 * time.Second // Time interval to report transaction pool stats + gwei = big.NewInt(params.GWei) // 1 gwei, used as a flag for "rollup" transactions ) var ( @@ -537,10 +538,14 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { if tx.Value().Sign() < 0 { return ErrNegativeValue } + // Ensure the transaction doesn't exceed the current block limit gas. - if pool.currentMaxGas < tx.Gas() { + // We skip this condition check if the transaction's gasPrice is set to 1gwei, + // which indicates a "rollup" transaction that's paying for its data. + if pool.currentMaxGas < tx.Gas() && tx.GasPrice().Cmp(gwei) != 0 { return ErrGasLimit } + // Make sure the transaction is signed properly from, err := types.Sender(pool.signer, tx) if err != nil { From 42c8e307e928945ea1636cfcfa264b8039840384 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 30 Mar 2021 09:26:36 +0300 Subject: [PATCH 18/23] chore: debug log --- core/tx_pool.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/tx_pool.go b/core/tx_pool.go index c59cf4679..281da9fe6 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -590,6 +590,7 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { // whitelisted, preventing any associated transaction from being dropped out of the pool // due to pricing constraints. func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err error) { + log.Debug("received tx", "gas", tx.Gas(), "gasprice", tx.GasPrice().Uint64()) // If the transaction is already known, discard it hash := tx.Hash() if pool.all.Get(hash) != nil { @@ -597,6 +598,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e knownTxMeter.Mark(1) return false, fmt.Errorf("known transaction: %x", hash) } + // If the transaction fails basic validation, discard it if err := pool.validateTx(tx, local); err != nil { log.Trace("Discarding invalid transaction", "hash", hash, "err", err) From bcbbfb49318a1d90806f69699c7aef70fdd6fdea Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 30 Mar 2021 09:51:12 +0300 Subject: [PATCH 19/23] fix(rollup_fee): normalize charged gas by the charged gas price --- core/rollup_fee.go | 2 +- internal/ethapi/api.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/rollup_fee.go b/core/rollup_fee.go index bee222ec0..7f0e6208c 100644 --- a/core/rollup_fee.go +++ b/core/rollup_fee.go @@ -7,7 +7,7 @@ import ( /// ROLLUP_BASE_TX_SIZE is the encoded rollup transaction's compressed size excluding /// the variable length data. /// Ref: https://github.com/ethereum-optimism/contracts/blob/409f190518b90301db20d0d4f53760021bc203a8/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol#L47 -var ROLLUP_BASE_TX_SIZE int = 96 +const ROLLUP_BASE_TX_SIZE int = 96 /// CalculateFee calculates the fee that must be paid to the Rollup sequencer, taking into /// account the cost of publishing data to L1. diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index b5faca348..a98711221 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1024,9 +1024,9 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash return 0, err } - // 3. calculate the fee - fee := core.CalculateRollupFee(*args.Data, uint64(gasUsed), dataPrice, executionPrice) - return (hexutil.Uint64)(fee.Uint64()), nil + // 3. calculate the fee and normalize by the default gas price + fee := core.CalculateRollupFee(*args.Data, uint64(gasUsed), dataPrice, executionPrice).Uint64() / defaultGasPrice + return (hexutil.Uint64)(fee), nil } func legacyDoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) { From 45b3516ca653abfa51bd2882da615ef67fa2c42d Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 5 Apr 2021 17:35:38 -0700 Subject: [PATCH 20/23] remove intrinsic gas checks --- core/tx_pool.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index 281da9fe6..d26dc00ec 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -571,14 +571,14 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { return ErrInsufficientFunds } } - // Ensure the transaction has more gas than the basic tx fee. - intrGas, err := IntrinsicGas(tx.Data(), tx.To() == nil, true, pool.istanbul) - if err != nil { - return err - } - if tx.Gas() < intrGas { - return ErrIntrinsicGas - } + // // Ensure the transaction has more gas than the basic tx fee. + // intrGas, err := IntrinsicGas(tx.Data(), tx.To() == nil, true, pool.istanbul) + // if err != nil { + // return err + // } + // if tx.Gas() < intrGas { + // return ErrIntrinsicGas + // } return nil } From 4b78e4ed0881cfa26e63cba275ec38c3941f6b0f Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 5 Apr 2021 17:50:28 -0700 Subject: [PATCH 21/23] move intrinsic gas check behind non-ovm codepath --- core/tx_pool.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/tx_pool.go b/core/tx_pool.go index d26dc00ec..6ad8c40d1 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -570,15 +570,15 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { if pool.currentState.GetBalance(from).Cmp(tx.Cost()) < 0 { return ErrInsufficientFunds } + // Ensure the transaction has more gas than the basic tx fee. + intrGas, err := IntrinsicGas(tx.Data(), tx.To() == nil, true, pool.istanbul) + if err != nil { + return err + } + if tx.Gas() < intrGas { + return ErrIntrinsicGas + } } - // // Ensure the transaction has more gas than the basic tx fee. - // intrGas, err := IntrinsicGas(tx.Data(), tx.To() == nil, true, pool.istanbul) - // if err != nil { - // return err - // } - // if tx.Gas() < intrGas { - // return ErrIntrinsicGas - // } return nil } From f5eb5349b04d36c810de64e6ccfe866fecc55804 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 6 Apr 2021 10:12:21 -0700 Subject: [PATCH 22/23] remove fudging code --- internal/ethapi/api.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a98711221..6c384955c 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1083,25 +1083,6 @@ func legacyDoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrO hi = mid } } - - // Fudging to account for gas required to verify signatures + pass around data. - // Specifically, this line accounts for the fact that there's a bit of computation performed in - // a "real" transaction that won't be covered by an eth_call: - // 1. Going into the OVM_SequencerEntrypoint. - // 2. Going into the OVM_ProxyEOA + OVM_ECDSAContractAccount. - // 3. Verify signatures in various places. - // eth_call skips all of this and therefore won't correctly estimate gas by default. We need to - // tweak the gas estimate to account for this discrepancy. Cost is quite high here because of - // the EVM limitation that CALL can only pass 63/64 of total gas available -- so most of this - // gas isn't actually spent during execution but needs to be provided to avoid a revert. - hi += 1000000 - if args.Data != nil { - hi += uint64(len([]byte(*args.Data))) * 128 - } - if hi > cap { - hi = cap - } - // Reject the transaction as invalid if it still fails at the highest allowance if hi == cap { if !executable(hi) { From 65f2d58b99540b068c9744f447d10866056e0273 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 6 Apr 2021 11:03:13 -0700 Subject: [PATCH 23/23] fix wrong gas limit --- core/state_processor.go | 2 +- core/state_transition_ovm.go | 4 ++-- eth/api_tracer.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/state_processor.go b/core/state_processor.go index 7d349f83a..21e5a02ae 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -90,7 +90,7 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo } } else { decompressor := config.StateDump.Accounts["OVM_SequencerEntrypoint"] - msg, err = AsOvmMessage(tx, types.MakeSigner(config, header.Number), decompressor.Address) + msg, err = AsOvmMessage(tx, types.MakeSigner(config, header.Number), decompressor.Address, header.GasLimit) if err != nil { return nil, err } diff --git a/core/state_transition_ovm.go b/core/state_transition_ovm.go index 7d21a2c8c..381fa3c3e 100644 --- a/core/state_transition_ovm.go +++ b/core/state_transition_ovm.go @@ -58,7 +58,7 @@ func toExecutionManagerRun(evm *vm.EVM, msg Message) (Message, error) { return outputmsg, nil } -func AsOvmMessage(tx *types.Transaction, signer types.Signer, decompressor common.Address) (Message, error) { +func AsOvmMessage(tx *types.Transaction, signer types.Signer, decompressor common.Address, gasLimit uint64) (Message, error) { msg, err := tx.AsMessage(signer) if err != nil { // This should only be allowed to pass if the transaction is in the ctc @@ -85,7 +85,7 @@ func AsOvmMessage(tx *types.Transaction, signer types.Signer, decompressor commo msg.From(), &decompressor, tx.GetMeta().RawTransaction, - msg.Gas(), + gasLimit, ) if err != nil { diff --git a/eth/api_tracer.go b/eth/api_tracer.go index 1186848b3..2c7576d73 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -809,7 +809,7 @@ func (api *PrivateDebugAPI) computeTxEnv(blockHash common.Hash, txIndex int, ree if !vm.UsingOVM { msg, _ = tx.AsMessage(signer) } else { - msg, err = core.AsOvmMessage(tx, signer, common.HexToAddress("0x4200000000000000000000000000000000000005")) + msg, err = core.AsOvmMessage(tx, signer, common.HexToAddress("0x4200000000000000000000000000000000000005"), block.Header().GasLimit) if err != nil { return nil, vm.Context{}, nil, err }