From f7b384f8545af0070f5b077653ca53092f3b46c4 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Tue, 30 Mar 2021 20:20:52 +0300 Subject: [PATCH] Sequencer Fee Pricing Part 1, fix: revive the binary-search-enabled DoEstimateGas from geth (#276) * fix: add nil check when estimating gas to avoid panic during contract creation * fix: revert DoEstimateGas changes introduced as a temp fix in #22 * 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 Co-authored-by: Ben Jones Co-authored-by: smartcontracts Co-authored-by: Karl Floersch --- core/state_transition_ovm.go | 7 ++- internal/ethapi/api.go | 84 +++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/core/state_transition_ovm.go b/core/state_transition_ovm.go index ee4882c8897e..7d21a2c8c457 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(), } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index c26cf52aa503..0e4f04b29e19 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -997,13 +997,85 @@ 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 + } + } + + // 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) { + 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