Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce a gasLimit in EOA wallet / eth_estimateGas #823

Closed
karlfloersch opened this issue May 9, 2021 · 23 comments · Fixed by #906
Closed

Reintroduce a gasLimit in EOA wallet / eth_estimateGas #823

karlfloersch opened this issue May 9, 2021 · 23 comments · Fixed by #906
Labels
C-feature Category: features

Comments

@karlfloersch
Copy link
Contributor

karlfloersch commented May 9, 2021

Is your feature request related to a problem? Please describe.
We have disabled the gasLimit check inside of the ECDSA wallet account here:

// TEMPORARY: Disable gas checks for mainnet.
// // Need to make sure that the gas is sufficient to execute the transaction.
// require(
// gasleft() >= SafeMath.add(decodedTx.gasLimit, EXECUTION_VALIDATION_GAS_OVERHEAD),
// "Gas is not sufficient to execute the transaction."
// );

We disabled this check because we changed the meaning of gasLimit signed in transactions to equal fee. This meant that the wallet contract no longer had access to the gas required for execution. Removing this check introduces a security vulnerability for user wallets as the sequencer can extract a fee even if the gas supplied to the call is lower than what is acceptable.

For more information on why we turned gasLimit into fee see this discussion (mirror).

Describe the solution you'd like
Currently,

gasLimit = gasUsed*executionPrice + transactionSizeInBytes*dataPrice
gasPrice = 1gwei

We propose to change this to:

gasLimit = Math.round((gasUsed*executionPrice + transactionSizeInBytes*dataPrice) / feeDivisor) + gasUsed/gasLimitGranularity
gasPrice = 0.001gwei

Where feeDivisor=10000000 and gasLimitGranularity=100000. Note this means that gasLimits can only be set in increments of 100k.

Next we modify the wallet contract to include the following check:

        gasLimit = (decodedTx.gasLimit % 1000) * 100,000;
        require(
           gasleft() >= SafeMath.add(gasLimit, EXECUTION_VALIDATION_GAS_OVERHEAD),
           "Gas is not sufficient to execute the transaction."
        );

This change also requires modifying L2Geth's estimateGas endpoint to return a gas value which also encodes the gasLimit as is done above.

Describe alternatives you've considered
Another option is require that the wallet's call does not revert. This has the adverse effect that the sequencer must execute all transactions before applying them. Otherwise they won't know that the transaction will pay them. This is a DOS vector,

@karlfloersch karlfloersch added A-contracts C-feature Category: features labels May 9, 2021
@tynes
Copy link
Contributor

tynes commented May 9, 2021

We disabled this check because we changed the meaning of gasLimit signed in transactions to equal fee. This meant that the wallet contract no longer had access to the gas required for execution.

Since the gasLimit was no longer encoding the gasLimit, information was being lost. The new scheme preserves the value of the gasLimit so that no information is lost

@karlfloersch
Copy link
Contributor Author

Updated this issue to specifically call out the change required in L2Geth

@samsondav
Copy link

Is there a reference implementation of this calculation that we can take a look at?

@tynes
Copy link
Contributor

tynes commented May 17, 2021

Note that transactionSizeInBytes*dataPrice does not take into account the gas cost differences between zero and non zero bytes in calldata. The implementation will change 4 gas for a zero byte and 16 bytes for a non zero byte, see https://eips.ethereum.org/EIPS/eip-2028

It now does

@K-Ho
Copy link
Contributor

K-Ho commented May 17, 2021

L1 gas cost of a transaction is equal to:
L1GasPrice * ((16 * numNonZeroBytes + 4 * numZeroBytes) + perTxL1BatchCost + perStateRootL1BatchCost)
assuming 200 txs in a batch, perTxL1BatchCost will be 2687 gas
assuming 250 state roots in a batch perStateRootL1BatchCost is 369832/251 = 1473.4 gas. (See https://etherscan.io/tx/0x80100d2fdbe1d7d3a99f9569c25040e453ec2f3e173aadfbb8d1e672ee3194d4) -
L2 gas cost of a transaction is equal to:
L2ExecutionGasPrice * L2GasUsed

Total fee per tx (sum of the two):
1.5 * (L1GasPrice * ((16 * numNonZeroBytes + 4 * numZeroBytes) + perTxL1BatchCost + perStateRootL1BatchCost) + L2ExecutionGasPrice * L2GasUsed)

@K-Ho
Copy link
Contributor

K-Ho commented May 17, 2021

  [GAS BENCHMARK] OVM_CanonicalTransactionChain
    appendSequencerBatch [ @skip-on-coverage ]
Benchmark: 200 transactions in a single context.
Benchmark complete.
Gas used: 1646860
Fixed calldata cost: 1280000
Non-calldata overhead gas cost per transaction: 1834.3
      ✓ 200 transactions in a single context (533ms)
Benchmark: 200 transactions in 200 contexts.
Benchmark complete.
Gas used: 1817522
Fixed calldata cost: 1280000
Non-calldata overhead gas cost per transaction: 2687.61
      ✓ 200 transactions in 200 contexts (681ms)

@tynes
Copy link
Contributor

tynes commented May 17, 2021

edit: no longer being used

The formula used is:

overhead = 2688 + 1473
dataCost = 4 * zeroDataBytes + 16 * nonZeroDataBytes + overhead
dataFee =  dataCost * dataPrice
executionFee = executionPrice * gasUsed
scaledFee = dataFee + executionFee
normalizedFee = fee // feeDivisor
normalizedGasused = gasUsed // gasLimitGranularity
fee = normalizedFee + normalizedGasused
result = fee * 1.5

where rollupBatchOverhead is the sum of the cost of a single transaction in a transaction batch and the cost of a single state root in a state root batch

@K-Ho
Copy link
Contributor

K-Ho commented May 18, 2021

Example Transaction:

l1 gas price = 61 (gwei)
batchoverhead = 4161
4 *152 zero bytes + 16* 220 data bytes = 4128 gas
l1gasCost = 8289 gas * 61 gwei = 505629 gwei
Executionfee = .1 gwei executionPrice * 5,000,000 gasLimit = 500,000 gwei
egFee = floor(1.5 * (505,629 gwei + 500,000 gwei)) * 10,000,000 max gas + 5,000,000 gas limit
egFee = 12584435000000 
getGasPrice = 1/10000000 gwei = .0000001gwei
estimatedFee (in metamask) = 12584435000000 * .0000001gwei = $4.4045

@tynes

This comment has been minimized.

@tynes

This comment has been minimized.

@tynes
Copy link
Contributor

tynes commented May 19, 2021

This solution encodes the L2 gaslimit in the lower order bits of the fee, which is tx.gasLimit because tx.gasPrice should always be 1

#!/usr/bin/env python

def compute_data_cost(data):
    ones = 0
    zeros = 0
    for b in data:
        if b == 0:
            zeros += 1
        else:
            ones += 1
    ones_cost = ones * 16
    zeros_cost = zeros * 4
    return ones_cost + zeros_cost

def eth_estimate_gas(x):
    return x

def gwei_to_wei(x):
    return x * 10**9

def wei_to_gwei(x):
    return x * 10**-9

# The L1 gas price must satisfy the equation `x * (10**8)`
# Round to next highest number that satisfies this equation
def get_l1_gas_price(number):
    number += (10**8 - 2 if number % 10**8 < 2 else 10**8)
    return number - (number % 10**8)

# The L2 gas price must satisfy the equation `x * (10**8) + 1`
def get_l2_gas_price(number):
    number += (10**8 - 2 if number % 10**8 < 2 else 10**8)
    return number - (number % 10**8) + 1

if __name__ == '__main__':
    # example mainnet transaction
    tx = b'0xf86c01854349be0a0082520894a1d8d972560c2f8144af871db508f0b0b10a3fbf8803f60cc2863df0008026a06d0450652736e0b8f9b5b9d369498dcfd805bafc2099e02b5ecd1d8e9726d548a070718c36c22987a840869f87d650882554f2fa4faf85be061e355ceba930a270'

    # the overhead of submitting a transaction to L1 is ~4200 gas
    # this value is hardcoded
    overhead = 4200

    # fetch the L1 gas price from an L1 Provider
    # the return value must satisfy the equation
    # `y * 10**8` so round up to the nearest number
    l1_gas_price = get_l1_gas_price(gwei_to_wei(2000))
    # compute the L1 gas used to submit the transaction
    l1_gas_used = compute_data_cost(tx) + overhead

    # call the L2 gas oracle to get the current gas price
    # the return value must satisfy the equation
    # `x * (10**8) + 1`
    # round up to the nearest number
    l2_gas_price = get_l2_gas_price(gwei_to_wei(5000))
    # call estimate gas to determine how much gas is used by the transaction
    l2_gas_limit = eth_estimate_gas(437118)

    estimate_gas = l2_gas_price * l2_gas_limit + l1_gas_price * l1_gas_used
    gas_price = 1

    print(estimate_gas)
    recomputed_l2_gas_limit = estimate_gas % 10**8

    assert recomputed_l2_gas_limit == l2_gas_limit

@platocrat
Copy link
Contributor

platocrat commented May 20, 2021

do we have any code that's written in Python or is this just pseudocode? 👀

@tynes
Copy link
Contributor

tynes commented May 20, 2021

You can try running the python linked above

@platocrat
Copy link
Contributor

You can try running the python linked above

I was more so wondering why you were referring to the python code as a "solution" if you're not implementing it in python

@K-Ho
Copy link
Contributor

K-Ho commented May 20, 2021

I love that the overhead is 4200 gas 🤣

@K-Ho
Copy link
Contributor

K-Ho commented May 20, 2021

Documenting what we chatted about @tynes -

When a tx is received via RPC, we calculate the fee it should have been sent by:

  1. Extract the L2GasLimit from the lower-order bits of the tx.gasLimit
  2. Read the current L1GasPrice
  3. Read the current L2ExecutionPrice
  4. Calculate the ExpectedFee given the L2GasLimit, L2ExecutionPrice, Scalar, L1GasPrice, andtx.data
  5. If the tx.gasLimit specifies a fee that is less than ExpectedFee, then reject the tx

@karlfloersch
Copy link
Contributor Author

It turns out that setting a 1 wei gasPrice breaks metamask. This means we need to adjust the equation so I am reopening this issue.

@karlfloersch karlfloersch reopened this May 26, 2021
@karlfloersch
Copy link
Contributor Author

karlfloersch commented May 26, 2021

l1Fee = l1GasLimit * l1GasPrice
l2Fee = l2GasLimit * l2GasPrice
sum = l1Fee + l2Fee
scaled = sum / txGasPrice
remainder = scaled % 100_000_000
roundedScaled = scaled + 100_000_000 - remainder
txGasLimit = roundedScaled + l2GasLimit

fee = txGasLimit * txGasPrice

For this to work the following must be satisfied:

tx.gasPrice == 0 OR 10^tx.gasPrice // 0, 1, 10, 100, 1000, etc

To efficiently verify whether the tx.gasPrice is correct you can use:

x % 1 == 0 AND (x < 2 OR len(str(tx.GasPrice-1)) != len(str(tx.GasPrice)))

@K-Ho
Copy link
Contributor

K-Ho commented May 26, 2021

e.g. for a tx, with 1 wei gasPrice, we have a gasLimit of 45061500209015
if we increase this to 10000 wei gasPrice, we can have a gasLimit of: 4500209015

@K-Ho
Copy link
Contributor

K-Ho commented May 26, 2021

^ Actionably, we just divide the fee by 10000 before adding the l2GasLimit
rather than doing 45061500000000 wei + 209015 l2GasLimit = 45061500209015,
we do: (45061500000000 wei / 10000) + 209015 l2GasLimit = 4500209015

@K-Ho
Copy link
Contributor

K-Ho commented May 28, 2021

Proposal: Round up scaled to the nearest value where value%100_000_000==0

Formula

l1Fee = l1GasLimit * l1GasPrice
l2Fee = l2GasLimit * l2GasPrice
sum = l1Fee + l2Fee
scaled = sum / 1000
remainder = scaled %100_000_000
roundedScaled = scaled + 100_000_000 - remainder
result = roundedScaled + l2GasLimit

Example:

l1GasPrice = 8_000_000_000 // 8 gwei
l2GasPrice = 75 // 75 wei
l2GasLimit = 10_999_999
l1GasLimit = 4_219
l1Fee = 8_000_000_000 * 4_219 = 33_752_000_000_000
l2Fee =  75 * 10_999_999 = 824_999_925
sum = 33_752_000_000_000 + 824_999_925 = 33_752_824_999_925
scaled = 33_752_824_999_925 / 1000 = 33_752_824_999 // this does round down to nearest int. We're just rounding for this value anyways, so this shouldn't matter.
remainder = 33_752_824_999 % 100_000_000 = 52_824_999
roundedScaled = 33_752_824_999  + 100_000_000 - 52_824_999
roundedScaled = 33_800_000_000
result = 33_800_000_000 + 10_999_999 = 33_810_999_999

For fun on the above calc:

tx.gasLimit = 33_810_999_999
tx.gasPrice = 1000
fee = 33_810_999_999 * 1_000 wei = 0.000033811 ETH = ~$.09

@K-Ho
Copy link
Contributor

K-Ho commented Jun 1, 2021

Turns out Metamask rounds gasPrice down to gwei to 4 decimal places, which rounds a 1,000 Wei gasPrice to 0 :(. Here is a proposed revision to the above to allow a gasPrice of 10,000,000 wei or .01 gwei (can be scaled up to .015 gwei):

Proposal: Round up scaled to the nearest value where value%10,000==0 and scale l2GasLimit down to ceil(l2GasLimit / 10,000)

Formula

roundedL2GasLimit = ceilMod(l2GasLimit, 10_000)
l1Fee = l1GasLimit * l1GasPrice
l2Fee = roundedL2GasLimit * l2GasPrice
sum = l1Fee + l2Fee
scaledFee = sum / 10,000,000
remainder = scaled %10_000
roundedScaledFee = scaledFee + 10_000 - remainder
scaledRoundedL2GasLimit = roundedL2GasLimit / 10_000
result = roundedScaledFee + scaledRoundedL2GasLimit

Example:

l1GasPrice = 8_000_000_000 // 8 gwei
l2GasPrice = 75 // 75 wei
l2GasLimit = 10_999_999
l1GasLimit = 4_219
roundedL2GasLimit = ceilMod(10_999_999, 10_000) = 11_000_000
l1Fee = 8_000_000_000 * 4_219 = 33_752_000_000_000
l2Fee =  75 * 11_000_000 = 825_000_000
sum = 33_752_000_000_000 + 824_999_925 = 33_752_825_000_000
scaledFee = 33_752_824_999_925 / 10_000_000 = 3_375_282 # this does round down to nearest int. We're just rounding for this value anyways, so this shouldn't matter.
remainder =  3_375_282 % 10_000 = 5_282
roundedScaledFee = 3_375_282  + 10_000 - 5_282 = 3_380_000
roundedScaledL2GasLimit = 11_000_000/10_000 = 1_100
result = 3_380_000 +  1_100 = 3_381_100

For fun on the above calc:

tx.gasLimit =3_381_100 gas
tx.gasPrice = 10_000_000 wei
fee = 3_381_100 * 10_000_000 wei = 0.000033811 ETH = ~$.09

@smartcontracts
Copy link
Contributor

No longer required in OVM 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: features
Projects
None yet
6 participants